nagyesta / lowkey-vault

Lowkey Vault is a small test double for Azure Key Vault. Developer feedback needed, please vote here: https://github.com/nagyesta/lowkey-vault/discussions/272
MIT License
55 stars 5 forks source link

Creating Elliptic Curve key sometimes provides invalid values #1024

Closed kntajus closed 4 months ago

kntajus commented 4 months ago

Describe the bug

When using the REST API to create an Elliptic Curve key, either the X or the Y value returned can sometimes be too long, in that the base64url-encoded value is 44 characters long (instead of 43).

To reproduce

Steps to reproduce the behavior:

  1. Spin up a Docker container exposing port 8443
  2. Make a POST request to https://127.0.0.1:8443/keys/sample/create?api-version=7.5
  3. Repeat Step 2 until the response shows an x or y value that is 44 characters long (it shouldn't take many attempts, I can get it to happen pretty consistently).

Expected behavior

The x and y values should both be 43 characters long, representing a 32 byte array when decoded.

Actual behavior

Sometimes the x or the y (or both!) values come back as 44 characters long, representing a 33 byte array, which then causes client code trying to use the key to fail.

image

Additional context

From my own observations, every time it comes back "too long", the value starts with a capital A. Digging a little further, if I then decode the faulty value, it looks like the 33 byte array has an extra 0-byte at the start that shouldn't be there.

nagyesta commented 4 months ago

Hi @kntajus , thank you for reporting this issue and thank you for using Lowkey Vault! I will look into it and fix it.

nagyesta commented 4 months ago

Hi @kntajus , I think your investigation was spot on!

When I serialized the X and Y values, the BigInteger was returning an additional zero at the beginning of the byte array. As far as I understood, this was the case because the integer was supposed to be unsigned and the first real byte happened to be a "negative" value in the array. This would have made the whole integer negative when deserialized by Java. Of course in this use-case we are importing it as a positive number anyway, so it made no difference as long as one have read back the value from the same app, but it was nonetheless broken for all other clients.

I have made a change to cut off the additional zero byte from the beginning if the byte array is longer than expected. Please feel free to try v2.4.42 and let me know whether it is working as expected!

Thank you in advance!

kntajus commented 4 months ago

Hi @nagyesta,

Everything seems to be working fine for me now. I have client code in both C# and Golang successfully reading a JWK and using it to verify a signature 🙌

Thank you so much for such a quick turnaround on the fix, and indeed for such a useful tool in the first place!

nagyesta commented 4 months ago

Hi @kntajus , thank you for your feedback! I am glad the fix worked! Best of luck!