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

[WIP] CommonCrypto Initial #344

Closed
wants to merge 21 commits into from

Conversation

reaperhulk
Copy link
Member

This PR is the base structure + hash support for CommonCrypto, an OS X cryptographic API.

  • Basic backend structure
  • Hash support
  • Basic docs
  • Determine how to skip basic cipher/hmac/hash tests if the backend doesn't support those operations. Ideally this won't require us to add a generator + separate function for each test?
  • Rebase to handle split backend/bindings
  • Determine how best to skip commoncrypto on unsupported platforms

"object": "CC_MD5_CTX *",
"init": self.lib.CC_MD5_Init,
"update": self.lib.CC_MD5_Update,
"final": self.lib.CC_MD5_Final},
Copy link
Member

Choose a reason for hiding this comment

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

These shouldn't be dicts, make them named tuples or something like that.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 2c5596c on reaperhulk:common-crypto-base into 2a710fd on pyca:master.

)
ctx = self._backend.ffi.new(mapping["object"])
# init/update/final ALWAYS return 1
mapping["init"](ctx)
Copy link
Member

Choose a reason for hiding this comment

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

No error checking? If they're supposed to always return 1 we should have an assert res == 1

@jenkins-cryptography
Copy link

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

@jenkins-cryptography
Copy link

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

@jenkins-cryptography
Copy link

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

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 3190ebe on reaperhulk:common-crypto-base into 0865a8b on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.88%) when pulling 5922ac3 on reaperhulk:common-crypto-base into 0865a8b on pyca:master.

@jenkins-cryptography
Copy link

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

* master: (28 commits)
  move macro from functions into macros where it belongs
  convert all functions without args from () to (void)
  update style guide docs to reflect change to (void)
  re-add some removed generators to simplify patch
  Add myself to AUTHORS
  change typeerror to valueerror
  expand the explanation for this workaround and switch XXX to TODO
  A handful of style fixes in the cffi bindings
  Use uintptr_t to get sufficiently wide storage for these types even on 32 bit Windows.
  Shorten long lines
  remove parameter names, fix some spacing
  Remove verify from Hash.
  Documentation clarity and grammer fixes.
  Update documentation on interface as well.
  refactor all tests to use mark instead of generator skips
  rename the method to be less horribly named
  add mark that allows us to do skip tests on backends via decorators
  remove SSL_OP_MSIE_SSLV2_RSA_PADDING
  fix a style error
  fix last warning (macro returns ASN1_ITEM_EXP *)
  ...

Conflicts:
	pytest.ini
	tests/conftest.py
@jenkins-cryptography
Copy link

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

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.8%) when pulling 17eac24 on reaperhulk:common-crypto-base into f82a03a on pyca:master.

* master: (41 commits)
  Correct spelling, fix phrasing, line wrap.
  Revert "Travis now has an up to date pypy"
  Make capitalization in the glossary consistent and sort it
  Make lib and ffi be private on backend
  Split OpenSSL binding
  Fixed test for earlier exceptino
  Rearrange
  Move GCM tag size/value validation farther forward -- this makes it easier by not requiring future backends to implement the same checks
  expose num_locks and {get,set}_{id,locking}_callback
  Make the PyPy tox job consistent with the main one.
  THis should be a seperate PR
  Typo fix
  This page has been subsumed by the index
  Bump the copyright year
  Document compiling OpenSSL to avoid conflicts
  missed some param names (C style fix)
  add the rest of the engine methods
  fix bogus merge
  Switch to long for these stand-in constants as suggested by reaperhulk to avoid compiler warnings
  Restore exposing a symbol when it's available. Refs pyca#351
  ...

hashtuple = namedtuple('hash', 'object init update final')
self.hash_mappings = namedtuple('HashMapping',
'md5 sha1 sha224 sha256 sha384 sha512')
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be a namedtuple, it should be a dict mapping hash algorithm classes to the tuples.

self._ffi = self._binding.ffi
self._lib = self._binding.lib

hashtuple = namedtuple('hash', 'object init update final')
Copy link
Member

Choose a reason for hiding this comment

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

This should be created at module level, also the naming could use a bit of level. Also use the "list" form of the constructor, not the str (i.e. namedtuple("Class", ["foo", "bar"]))

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling a17d834 on reaperhulk:common-crypto-base into adbea0d on pyca:master.

@jenkins-cryptography
Copy link

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

"""

FUNCTIONS = """
extern int CC_MD5_Init(CC_MD5_CTX *);
Copy link
Member

Choose a reason for hiding this comment

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

Why are these all extern?

* master:
  backend support check now lists which backend caused the skip
  add to author list
  Also use an absolute path for the about file
  Include a long description in our setup.py
  Explanatory comment
  So the tests don't all explode
  fixed typo
  flake8
  test for ctr
  Tests for OFB and CFB
  Implement this for CTR
  Validate the IV/nonce length for a given algorithm.
* master:
  improve docstring
  fix indentation mistake
  import build_ffi directly
  remove verify_kwargs and replace with pre_include/post_include/libraries
  refactor bindings to reduce code duplication with multiple backends
  instantiate hash objects for hmac checks too
  Instantiate our hash objects used for supported checks
@alex
Copy link
Member

alex commented Jan 4, 2014

You may hate me for this, what do you think about splitting up the bindings and the backend bits into seperate PRs?

@reaperhulk
Copy link
Member Author

So put the cryptography.hazmat.bindings.commoncrypto stuff into a PR and then build the backend PR on that? That's a pretty simple one to split out so I can do that. I'll hate you later ;)

@alex
Copy link
Member

alex commented Jan 4, 2014

Sounds good. Mostly I want to avoid re-reviewing bits of the backend stuff,
when we need to get the bindings stuff done first.

On Sat, Jan 4, 2014 at 11:30 AM, Paul Kehrer notifications@github.comwrote:

So put the cryptography.hazmat.bindings.commoncrypto stuff into a PR and
then build the backend PR on that? That's a pretty simple one to split out
so I can do that. I'll hate you later ;)


Reply to this email directly or view it on GitHubhttps://github.com//pull/344#issuecomment-31586449
.

"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero
GPG Key fingerprint: 125F 5C67 DFE9 4084

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling c8afb31 on reaperhulk:common-crypto-base into d68fd37 on pyca:master.

@jenkins-cryptography
Copy link

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

@reaperhulk reaperhulk closed this Jan 10, 2014
@reaperhulk reaperhulk deleted the common-crypto-base branch March 28, 2014 19:38
joerichter-stash pushed a commit to kiwigrid/cryptography that referenced this pull request Nov 15, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

None yet

5 participants