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

Support custom key pair #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

sodre
Copy link

@sodre sodre commented Mar 30, 2021

@wbond

This PR uses a different approach to break the dependency from oscrypto. The user just needs to pass a class to build that has the algorithm property and the sign callable.

I added a UnitTest to show how one can use it the construct.

  A different approach where the user can pass a CustomKeyPair class to the build callable.

from asn1crypto import x509, keys, csr, pem
from oscrypto import asymmetric
Copy link
Author

Choose a reason for hiding this comment

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

We delay loading oscrypto until absolutely necessary.

  1. In case the public key is not asn1crypto.keys.PublicKeyInfo
  2. In case the private key does not conform to the duck typing

Comment on lines 165 to 177
if not isinstance(value, keys.PublicKeyInfo):
from oscrypto import asymmetric
if isinstance(value, asymmetric.PublicKey):
value = value.asn1
else:
raise TypeError(_pretty_message(
'''
subject_public_key must be an instance of
asn1crypto.keys.PublicKeyInfo or oscrypto.asymmetric.PublicKey,
not %s
''',
_type_name(value)
))
Copy link
Author

Choose a reason for hiding this comment

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

This change is simply inverting the logic, to first look for an asn1crypto.PublicKeyInfo, only if that fails should we import oscrypto.asymmetric.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant