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

[BUG] SignResult keyId is missing the version information of the key used #43451

Open
3 tasks done
S1M0NM opened this issue Dec 17, 2024 · 1 comment
Open
3 tasks done
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. KeyVault needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@S1M0NM
Copy link

S1M0NM commented Dec 17, 2024

Describe the bug
In the .NET implementation of the Cryptography client, the SignResult for the signData action returns the full key used to create the signature as the keyId, as this information is relevant to verifying the signature on the other side against the correct key.

However, if you use the same call in the Java implementation of the Cryptography client, you only ever get back the versionless key that was used to create the Cryptography client.

Currently, it is not possible to use keys with key rotation due to the missing version specification, as the recipient cannot be informed which key was used for the signature.
If the recipient also uses the versionless key ID, the verify call returns false

Exception or Stack Trace
No Exception

To Reproduce

  1. Create an Azure Key Vault with an EC P-256 Key and assign "Key Vault Crypto User" to yourself or a service principal
  2. Use the code snippets below and to sign data against an key within an Azure KeyVault
  3. Check the returned SignResult where you can notice the difference for the results keyId-value

Code Snippet

public SignResult sign(string signatureKeyUri, byte[] data, string signatureAlgorithm) 
{
    KeyVaultKeyIdentifier identifier = new KeyVaultIdentifier(signatureKeyUri);
    TokenCredential credential = new DefaultAzureCredential();
    
    KeyClient keyClient = new KeyClient(identifier.VaultUri, credential);
    CryptographyClient cryptographyClient = keyClient.GetCryptographyClient(identifier.Name, identifier.Version);

    return cryptographyClient.SignData(signatureAlgorithm, data);
}
public SignResult sign(String signatureKeyUri, byte[] data, String signatureAlgorithm) {
    KeyVaultKeyIdentifier identifier = new KeyVaultKeyIdentifier(signatureKeyUri);

    KeyClient keyClient = new KeyClientBuilder()
            .vaultUrl(identifier.getVaultUrl())
            .credential(new DefaultAzureCredentialBuilder().build())
            .buildClient();

    CryptographyClient cryptographyClient = keyClient.getCryptographyClient(identifier.getName(), identifier.getVersion());

    return cryptographyClient.signData(SignatureAlgorithm.fromString(signatureAlgorithm), data);
}

Expected behavior
Both implementations should always return the full KeyId, as this is required to verify the signature correctly.
If key rotation is configured, the signature will otherwise be recognized as invalid if the verification is performed against a KeyUri without a version.

Screenshots
If applicable, add screenshots to help explain your problem.

Setup (please complete the following information):

  • OS: Windows
  • IDE: IntelliJ IDEA Ultimate
  • Library/Libraries: com.azure:azure-security-keyvault-keys:4.9.1
  • Java version: 17
  • App Server/Environment: -
  • Frameworks: -

If you suspect a dependency version mismatch (e.g. you see NoClassDefFoundError, NoSuchMethodError or similar), please check out Troubleshoot dependency version conflict article first. If it doesn't provide solution for the problem, please provide:

  • verbose dependency tree (mvn dependency:tree -Dverbose)
  • exception message, full stack trace, and any available logs

Additional context
According to my research so far, the problem could be solved quite simply at this point in the code

The KeyOperationResult received back here contains the correct and complete kid including version.

Therefore, instead of return new SignResult(result.getResult(), algorithm, keyId); actually return new SignResult(result.getResult(), algorithm, result.getKid()); should be used in order to be able to use key rotation for sign and verify actions within the java implementation

This could also apply to the return values for methods like decrypt, verify, wrapKey and unwrapKey since they als use keyId within the return value.

Information Checklist
Kindly make sure that you have added all the following information above and checkoff the required fields otherwise we will treat the issuer as an incomplete report

  • Bug Description Added
  • Repro Steps Added
  • Setup information Added
@github-actions github-actions bot added Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. KeyVault needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Dec 17, 2024
Copy link

Thank you for your feedback. Tagging and routing to the team member best able to assist.

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. customer-reported Issues that are reported by GitHub users external to the Azure organization. KeyVault needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
Status: Untriaged
Development

No branches or pull requests

2 participants