[Key Vault] Refine Security Domain tests to not require CLI use mid-execution #42089
Conversation
mccoyp
added
KeyVault
Client
mccoyp
force-pushed
the
sd-test-refinement
branch
from
77ba331 to
9fe0d95
Compare
There was a problem hiding this comment.
Pull Request Overview
This pull request modernizes and improves the Security Domain tests by eliminating the dependency on Azure CLI for security domain wrapping during test execution. The changes introduce local utilities to handle security domain encryption/wrapping operations directly in Python, making the tests more self-contained and reliable.
Key changes:
- Adds comprehensive security domain wrapping utilities (cryptographic operations, key derivation, JWE handling)
- Introduces consistent test certificates and keys stored in the resources directory
- Refactors test code to use local wrapping instead of requiring mid-test CLI execution
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| wrapping.py | New comprehensive utility implementing security domain cryptographic operations and wrapping logic |
| jwe.py | New JWE (JSON Web Encryption) implementation for handling encrypted key operations |
| utils.py | Refactored to use consistent resource paths and simplified certificate handling |
| test files | Updated to use new wrapping utilities instead of CLI dependency |
| resources/.cer/.pem | Added consistent test certificates and private keys |
| .gitignore/CredScanSuppression.json | Updated to allow test certificates while maintaining security |
Comments suppressed due to low confidence (4)
sdk/keyvault/azure-keyvault-securitydomain/tests/wrapping.py:288
- This condition uses chained comparison incorrectly. It evaluates as 'i < len(self.jwe_decode.auth_tag)' and then compares that boolean result with 32, which will never be True. Should be 'while i < len(self.jwe_decode.auth_tag) and len(self.jwe_decode.auth_tag) == 32:'
jwe_wrapped = JWE()
sdk/keyvault/azure-keyvault-securitydomain/tests/wrapping.py:25
- [nitpick] Class name 'ModMath' should follow PascalCase convention but is missing descriptive context. Consider 'ModularMath' or 'ModularArithmetic' for clarity.
class ModMath:
sdk/keyvault/azure-keyvault-securitydomain/tests/jwe.py:281
- Method name should follow snake_case convention. Should be 'aes256_hmac_sha512_decrypt'.
def Aes256HmacSha512Decrypt(self, cek):
sdk/keyvault/azure-keyvault-securitydomain/tests/jwe.py:306
- Method name should follow snake_case convention. Should be 'aes256_hmac_sha512_encrypt'.
def Aes256HmacSha512Encrypt(self, cek, plaintext):
| from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes | ||
|
|
||
| from utils import Utils | ||
|
|
There was a problem hiding this comment.
Would it be worth adding a header comment to describe and link to where this module is copied from and how it should be maintained? Or is that not really necessary, as the source we copied from isn't really a source of truth to be kept in sync with?
| jku=None, jwk=None, kid=None, x5u=None, x5c=None, x5t=None, | ||
| x5t_S256=None, typ=None, cty=None, crit=None): | ||
| """ | ||
| JWE header |
There was a problem hiding this comment.
| JWE header | |
| JWE header |
Strange indent.
|
|
||
| # At this point, use the Azure CLI to encrypt the security domain to prepare for upload | ||
| # `az keyvault security-domain restore-blob --sd-exchange-key <>-transfer-key.pem --sd-file <>-security-domain.json --sd-wrapping-keys <>-certificate0.key <>-certificate1.key <>-certificate2.key --sd-file-restore-blob <>-restore-blob.json` | ||
| poller = upload_client.begin_upload(security_domain=result) |
There was a problem hiding this comment.
How did this used to work by re-uploading the unwrapped result?
| This test requires two Managed HSMs to be set up, one for downloading and one for uploading. The URL of the | ||
| first Managed HSM is specified in the environment variable AZURE_MANAGEDHSM_URL, and the second is specified | ||
| in SECONDARY_MANAGEDHSM_URL. |
There was a problem hiding this comment.
From what I could see, the util code that read from these env vars was removed? Are they read again somewhere else?
Description
Security Domain tests until now have required the Azure CLI to run, since we relied on their utility to wrap a security domain with the appropriate keys. To streamline our process and avoid the (somewhat circular) dependency, this adds utilities to support this wrapping locally. These utilities are lifted directly from the CLI to avoid reinventing the wheel: https://github.com/Azure/azure-cli/tree/dev/src/azure-cli/azure/cli/command_modules/keyvault/security_domain
Additionally, consistent test certs/keys are added in this PR to simplify the testing process and make things more reliable.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines