Skip to content

[Key Vault] Implement abstract RSA key interfaces from cryptography#31996

Merged
laiapat merged 30 commits into
Azure:mainfrom
laiapat:kv-rsaprivatekey
Oct 11, 2023
Merged

[Key Vault] Implement abstract RSA key interfaces from cryptography#31996
laiapat merged 30 commits into
Azure:mainfrom
laiapat:kv-rsaprivatekey

Conversation

@laiapat
Copy link
Copy Markdown
Member

@laiapat laiapat commented Sep 8, 2023

Description

Resolves #31877. This PR adds an implementation of the cryptography library's abstract RSAPrivateKey and RSAPublicKey interfaces.

The key implementations accept a cryptography client and key material, but are meant to be created with create_rsa_private_key and create_rsa_public_key methods on CryptographyClient. The cryptography client is used to perform cryptographic operations (decryption and signing for private, encryption and verification for public). Byte serialization and RSAPublicKey.recover_data_from_signature use cryptography's own implementations. Deprecated interface methods are left unimplemented.

There were initial limitations to the operations we could perform because of opaque types in cryptography. The library maintainers helped me get a fix merged, which should be available in the next release. Since we know that there will be public properties for these attributes, we can safely access private attributes that have always been present as a backup. This can be seen in the code where we inspect .algorithm and .mgf attributes.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

@laiapat laiapat added KeyVault Client This issue points to a problem in the data-plane of the library. labels Sep 8, 2023
@azure-sdk
Copy link
Copy Markdown
Collaborator

azure-sdk commented Sep 8, 2023

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-keyvault-keys

@heaths
Copy link
Copy Markdown
Member

heaths commented Sep 12, 2023

private_numbers and private_bytes are left unimplemented since the private portion of the key is managed by Key Vault.

Why not attempt to download them like I do for .NET's and Go's (unofficial) CryptographyClient? Don't fail - simply send to the service - if you can't, though. But it's more of a proxying optimization.

Comment thread sdk/keyvault/azure-keyvault-keys/azure/keyvault/keys/crypto/_models.py Outdated
Comment thread sdk/keyvault/azure-keyvault-keys/azure/keyvault/keys/crypto/_models.py Outdated
Comment thread sdk/keyvault/azure-keyvault-keys/azure/keyvault/keys/crypto/_models.py Outdated
Comment thread sdk/keyvault/azure-keyvault-keys/azure/keyvault/keys/crypto/_models.py Outdated
Comment thread sdk/keyvault/azure-keyvault-keys/azure/keyvault/keys/crypto/_models.py Outdated
Comment thread sdk/keyvault/azure-keyvault-keys/azure/keyvault/keys/crypto/_models.py Outdated
Comment thread sdk/keyvault/azure-keyvault-keys/azure/keyvault/keys/crypto/_client.py Outdated
Comment thread sdk/keyvault/azure-keyvault-keys/azure/keyvault/keys/crypto/_client.py Outdated
@laiapat laiapat marked this pull request as ready for review September 26, 2023 17:55
@laiapat laiapat requested a review from schaabs as a code owner September 26, 2023 17:55
@laiapat laiapat requested review from annatisch and heaths September 26, 2023 17:56
Copy link
Copy Markdown
Member

@heaths heaths left a comment

Choose a reason for hiding this comment

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

Barring a more thorough Pythonic review, LGTM.

@laiapat laiapat changed the title [Key Vault] Implement abstract private key interface from cryptography [Key Vault] Implement abstract RSA key interfaces from cryptography Sep 27, 2023
Comment thread sdk/keyvault/azure-keyvault-keys/azure/keyvault/keys/crypto/_models.py Outdated
Comment thread sdk/keyvault/azure-keyvault-keys/azure/keyvault/keys/crypto/_models.py Outdated
@laiapat laiapat requested a review from annatisch October 5, 2023 01:01
@laiapat laiapat requested a review from schaabs October 6, 2023 19:00
Comment thread sdk/keyvault/azure-keyvault-keys/azure/keyvault/keys/crypto/_models.py Outdated
@laiapat laiapat merged commit 0b86105 into Azure:main Oct 11, 2023
@laiapat laiapat deleted the kv-rsaprivatekey branch October 11, 2023 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Client This issue points to a problem in the data-plane of the library. KeyVault

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Implement abstract private key interfaces from "cryptography" library

5 participants