Azure / azure-cli

Azure Command-Line Interface
MIT License
3.94k stars 2.92k forks source link

az keyvault key sign rejects correctly encoded inputs for ES256 #28027

Open achamayou opened 8 months ago

achamayou commented 8 months ago

Describe the bug

az keyvault key sign rejects SHA256 input, encoded as base64url, because of an incorrect size check applied before decoding.

Related command

az keyvault key sign

Errors

(BadParameter) Invalid length of 'value': 43 bytes. ES256 requires 32 bytes, encoded with base64url. Code: BadParameter Message: Invalid length of 'value': 43 bytes. ES256 requires 32 bytes, encoded with base64url.

Issue script & Debug output

Step 1: produce a SHA256, encode it to base64url:

$ cat README.md | openssl dgst -sha256 -binary | openssl base64 | sed 's/+/-/g; s,/,_,g; s,=,,g' X_bj29D-XEwEt8VLg69uT9FNoCDWaUSSiTvVAMjn114

Step 2: try to sign:

$ az keyvault key sign --name $KEY_NAME --vault-name $AKV_NAME --algorithm ES256 --digest X_bj29D-XEwEt8VLg69uT9FNoCDWaUSSiTvVAMjn114 (BadParameter) Invalid length of 'value': 43 bytes. ES256 requires 32 bytes, encoded with base64url. Code: BadParameter Message: Invalid length of 'value': 43 bytes. ES256 requires 32 bytes, encoded with base64url.

Expected behavior

I should obtain a signature.

The binary representation of a SHA-256 is 32 bytes, but its base64 representation is necessarily longer, regardless of padding (not generally required for base64url). The size check is incorrectly applied to the base64 representation.

Environment Summary

$ az --version azure-cli 2.55.0

core 2.55.0 telemetry 1.1.0

Extensions: aks-preview 0.5.172 confcom 0.3.1

Dependencies: msal 1.24.0b2 azure-mgmt-resource 23.1.0b2

Python location '/opt/az/bin/python3' Extensions directory '/home/amchamay/.azure/cliextensions'

Python (Linux) 3.11.5 (main, Nov 29 2023, 03:42:00) [GCC 9.4.0]

Legal docs and information: aka.ms/AzureCliLegal

Your CLI is up-to-date.

Additional context

No response

yonzhan commented 8 months ago

Thank you for opening this issue, we will look into it.

achamayou commented 8 months ago

Please note that unlike #27631, this is not a feature request to support raw binary input, instead this is a bug when using the only supported input format (base64url).

achamayou commented 8 months ago

@yonzhan can you explain why this is not a bug please? The CLI is failing on an input that's 1. the right size, and 2. the right format.

SecuriFresh commented 7 months ago

+1

achamayou commented 7 months ago

@yonzhan @evelyn-ys is it possible to have an update on this please? As far as I can tell, key sign is completely broken at the moment for ES256.

evelyn-ys commented 7 months ago

This needs service team to take a look. @jlichwa for awareness

microsoft-github-policy-service[bot] commented 7 months ago

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @RandalliLama, @schaabs, @jlichwa.

sburton84 commented 6 months ago

Please note that unlike #27631, this is not a feature request to support raw binary input, instead this is a bug when using the only supported input format (base64url).

I believe this is actually related to #27631, as the error saying to use base64url encoding is coming from the Azure REST API, which accepts base64, not from the CLI itself, which for some reason doesn't actually accept base64 at all, it only accepts raw bytes. It is taking the 43 characters you supplied and, despite them already being base64 encoded, interpreting them as raw bytes, then base64-encoding them again to send to the API, so when it decodes them it has 43 bytes instead of 32.

achamayou commented 6 months ago

@sburton84 I agree they are related, I did not claim otherwise. #27631 reports that passing arbitrary input does not work though, which is not something the documentation or the error message suggest is supported.

This issue specifically reports that passing a valid base64url input of the correct size (43 bytes for this particular 32 bytes digest), which the documentation implies and the error message specifically states (ES256 requires 32 bytes, encoded with base64url) should work, doesn't. That is clearly a gap between the stated intent, and the outcome.

I suspect you are right about what's going wrong here, but I have not looked at the code in detail. I do wish this was prioritised, because as far as I can tell, the command is entirely unusable in its current state!

meum commented 4 months ago

This is not only happening with ES256. I've tried RS256, RS384 and RS512 and I'm having the same issue.

Sifungurux commented 1 month ago

Having this error as well. Is there an updated? Are you still working on this and is there an workaround?