Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added check to turn off CC backend for OS X version < 10.8 #744

Merged
merged 4 commits into from
Mar 8, 2014
Merged

Added check to turn off CC backend for OS X version < 10.8 #744

merged 4 commits into from
Mar 8, 2014

Conversation

Ayrx
Copy link
Contributor

@Ayrx Ayrx commented Mar 6, 2014

No description provided.

@public
Copy link
Member

public commented Mar 6, 2014

Is 10.8 the lowest version that actually works?

@jenkins-cryptography
Copy link

Test PASSed.
Refer to this link for build results: http://jenkins.cryptography.io/job/cryptography-pr-experimental/1001/

@Ayrx
Copy link
Contributor Author

Ayrx commented Mar 6, 2014

@public I think so and the guy that opened #740 says so as well. I can't find official support of this though, Apple's documentation is really difficult to search through.

@reaperhulk
Copy link
Member

Yes, I explicitly wrote it to support only 10.8+. There are several major missing features in older CC (like GCM).

@public
Copy link
Member

public commented Mar 6, 2014

Of course I have no idea how we get test coverage for this. You could re-write the if/else to be like
return version >= parse_version("10.8") instead of branching I suppose?

@reaperhulk
Copy link
Member

Have the whole thing invoke a function that you pass the version to, then you can pass it < 10.8 as a test.

@public
Copy link
Member

public commented Mar 6, 2014

That sounds ugly and doesn't really tell you very much?

@reaperhulk
Copy link
Member

Gets you coverage :) I prefer your solution provided coverage doesn't think there's an uncovered branch.

@askholme
Copy link

askholme commented Mar 6, 2014

Now checking my source(http://sourceforge.net/p/fink/mailman/fink-users/thread/[email protected]/) it might actually be that the KeyDeriviation file is available from 10.7 (but there is additional changes in 10.8)

On apple open source the file is copyright 2010, making inclusion in 10.7 likely
(http://www.opensource.apple.com/source/CommonCrypto/CommonCrypto-55010/CommonCrypto/CommonKeyDerivation.h) - Would be good to have someone confirm though

@reaperhulk
Copy link
Member

KDF is indeed in 10.7, but AES GCM and several other primitives are not available in 10.7 that were added in 10.8 so we made the decision to limit the CC backend to 10.8+ (at least for now).

@jenkins-cryptography
Copy link

Test PASSed.
Refer to this link for build results: http://jenkins.cryptography.io/job/cryptography-pr-experimental/1002/

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling d2fd08a on Ayrx:osx-version-check into 1e8aa9b on pyca:master.

@jenkins-cryptography
Copy link

Test PASSed.
Refer to this link for build results: http://jenkins.cryptography.io/job/cryptography-pr-experimental/1003/

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling e4880e6 on Ayrx:osx-version-check into 1e8aa9b on pyca:master.

@@ -46,4 +48,5 @@ def _ensure_ffi_initialized(cls):

@classmethod
def is_available(cls):
return sys.platform == "darwin"
version = parse_version(platform.mac_ver()[0])
return version >= parse_version("10.8")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Err, what does this do under a non-OS X OS?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It returns an empty tuple, which makes me uncomfortable since I don't know if that's a guarantee or just a coincidence. I'd prefer to do this using platform.uname() I think (Darwin >=12 is 10.8+).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If sys.platform =="Darwin" and platform.mac_ver() something something

On Mar 6, 2014, at 6:27 PM, Paul Kehrer [email protected] wrote:

In cryptography/hazmat/bindings/commoncrypto/binding.py:

@@ -46,4 +48,5 @@ def _ensure_ffi_initialized(cls):

 @classmethod
 def is_available(cls):
  •    return sys.platform == "darwin"
    
  •    version = parse_version(platform.mac_ver()[0])
    
  •    return version >= parse_version("10.8")
    
    It returns an empty tuple, which makes me uncomfortable since I don't know if that's a guarantee or just a coincidence. I'd prefer to do this using platform.uname() I think (Darwin >=12 is 10.8+).


Reply to this email directly or view it on GitHub.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@reaperhulk The docs state

Entries which cannot be determined are set to ''. All tuple entries are strings.

so I assumed that it is guaranteed to be an empty tuple for non OS X systems.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought you were going to keep the sys.platform check?

@jenkins-cryptography
Copy link

Test FAILed.
Refer to this link for build results: http://jenkins.cryptography.io/job/cryptography-pr-experimental/1042/

@jenkins-cryptography
Copy link

Test FAILed.
Refer to this link for build results: http://jenkins.cryptography.io/job/cryptography-pr-experimental/1043/

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 4dd97e5 on Ayrx:osx-version-check into c24003e on pyca:master.

@jenkins-cryptography
Copy link

Test PASSed.
Refer to this link for build results: http://jenkins.cryptography.io/job/cryptography-pr-experimental/1045/

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 3558fd0 on Ayrx:osx-version-check into c24003e on pyca:master.

reaperhulk added a commit that referenced this pull request Mar 8, 2014
Added check to turn off CC backend for OS X version < 10.8
@reaperhulk reaperhulk merged commit 78c2f2d into pyca:master Mar 8, 2014
@Ayrx Ayrx deleted the osx-version-check branch March 9, 2014 15:03
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

8 participants