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

Update cf-edhoc patch #92

Merged
merged 1 commit into from
Apr 27, 2024
Merged

Update cf-edhoc patch #92

merged 1 commit into from
Apr 27, 2024

Conversation

actyp
Copy link
Member

@actyp actyp commented Apr 27, 2024

Observation

The BigInteger#toByteArray() method returns the minimum number of bytes required to represent the BigInteger, which means that

byte[] bytes = new byte[] { 0, 1, 2 }
bytes /* [0, 1, 2] */  != new BigInteger(1, bytes).toByteArray() /* [1, 2] */

Specific Case

Focusing in the SharedSecretCalculation and specifically the methods recomputeEcdsa256YFromX and recomputeEcdsa384YFromX:

The input byte array publicKeyX should have 32 (resp. 48) byte length.

In the case that has a prefix of 0x00 bytes, the following would hold:

publicKeyX /* 32 bytes */ != new BigInteger(1, publicKeyX).toByteArray() /* < 32 bytes */

Now, because the right version of the above byte array was consequently used, the input would have an incorrect length, which is why the process would fail to find an available Y COSE key.

Partial Solution

One simple solution is to pad left the converted array in order to make it have the correct length.
At least, this solves the problematic cases of zero-prefix inputs.

@actyp actyp self-assigned this Apr 27, 2024
@actyp actyp requested a review from kostis April 27, 2024 18:00
Copy link
Member

@kostis kostis left a comment

Choose a reason for hiding this comment

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

Looks OK (and we should merge it), but wouldn't it be better if cf-edhoc was patched or properly fixed instead?

@actyp
Copy link
Member Author

actyp commented Apr 27, 2024

Yes, of course, we should report it.

@actyp actyp merged commit 19fe243 into main Apr 27, 2024
22 checks passed
@actyp actyp deleted the cf-edhoc-patch branch April 27, 2024 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants