Skip to content
This repository has been archived by the owner on Feb 27, 2023. It is now read-only.

Invalid JWK exponent encoding for RSA ? #40

Closed
r0ro opened this issue Jul 23, 2015 · 5 comments
Closed

Invalid JWK exponent encoding for RSA ? #40

r0ro opened this issue Jul 23, 2015 · 5 comments

Comments

@r0ro
Copy link

r0ro commented Jul 23, 2015

I believe there is an issue regarding exponent ("e") encoding in JWK.
In jwk_test.go the TestMarshalNonPointer JWK encoding test looks strange to me :

    keyJson := []byte(`{
        "e": "AQAB",
        "kty": "RSA",
        "n": "vd7rZIoTLEe-z1_8G1FcXSw9CQFEJgV4g9V277sER7yx5Qjz_Pkf2YVth6wwwFJEmzc0hoKY-MMYFNwBE4hQHw"
    }`)

[ ... ]

expected := "{\"Key\":{\"kty\":\"RSA\",\"n\":\"vd7rZIoTLEe-z1_8G1FcXSw9CQFEJgV4g9V277sER7yx5Qjz_Pkf2YVth6wwwFJEmzc0hoKY-MMYFNwBE4hQHw\",\"e\":\"AAEAAQ\"}}"

Why isn't "e" encoded as "AQAB" as in input ?
as per JWA RFC7518 e should be encoded in Base64urlUInt
AQAB represent 0x010001 => 65537
AAEAAQ represent 0x01000100 => 16777472

@csstaub
Copy link
Collaborator

csstaub commented Jul 23, 2015

It's big-endian, so it decodes to the same number: AQAB decodes to three bytes (01 00 01) and AAEAAQ decodes to four bytes (00 01 00 01). But you are right, there is some extraneous zero padding there that we should probably get rid of for cleanliness.

@r0ro
Copy link
Author

r0ro commented Jul 23, 2015

yes you are right, I got the endianess wrong when printing. The issue is not trimming the extra 0.
I've tested with a public key with a 3 exponent :

-----BEGIN PUBLIC KEY-----
MIIBIDANBgkqhkiG9w0BAQEFAAOCAQ0AMIIBCAKCAQEAspbGSo1vhexIC/bTdYDW
eUodxH5VFBfOvtaRk650giucOFbAM6ZUZXalWxPJn/6XmXmxlTk6GKMJmAUdG/Gr
LxzSTvhbYWSRV1ViYtpJv7FTfH2oY038jtp33tSoXa0wdryxh3iVwct6DcE+WzVJ
oFxmba8wIzXIpRAnIKEHtFIEZAIhFAULLHoQCAQCQmzP4b9+LQVqFQiCA/vgD2Jo
pqSgrY9Fouah+vhoLR63NU1Mi7JgZlyGl36FV73aXto8/nuli8VdjRQcMFRJihSK
X9qV2MrgqIFsPEfIESo19mB9Tjg06xAEkFP2uLN/8rvkja6tBJh/vPgP8R3bgLLc
KwIBAw==
-----END PUBLIC KEY-----

and I'm getting

{"kty":"RSA","n":"spbGSo1vhexIC_bTdYDWeUodxH5VFBfOvtaRk650giucOFbAM6ZUZXalWxPJn_6XmXmxlTk6GKMJmAUdG_GrLxzSTvhbYWSRV1ViYtpJv7FTfH2oY038jtp33tSoXa0wdryxh3iVwct6DcE-WzVJoFxmba8wIzXIpRAnIKEHtFIEZAIhFAULLHoQCAQCQmzP4b9-LQVqFQiCA_vgD2JopqSgrY9Fouah-vhoLR63NU1Mi7JgZlyGl36FV73aXto8_nuli8VdjRQcMFRJihSKX9qV2MrgqIFsPEfIESo19mB9Tjg06xAEkFP2uLN_8rvkja6tBJh_vPgP8R3bgLLcKw","e":"AAAAAw"}

and AAAAAw reprents 00 00 00 03, again it should only be 03

Right now it's invalid according to the spec:

The octet sequence MUST utilize the minimum number of octets needed to represent the value.

I assume it should be easy to fix but I'm not fluent in Go, and it would probably be better to add tests to.

@csstaub
Copy link
Collaborator

csstaub commented Jul 23, 2015

Opened #41 with a fix. Thanks for catching this and reporting it to us!

@r0ro
Copy link
Author

r0ro commented Jul 23, 2015

Thanks for this quick fix. I haven't checked but there may be other values that suffer from the same encoding issue.

Le 23 juil. 2015 à 21:20, Cedric Staub notifications@github.com a écrit :

Opened #41 with a fix. Thanks for catching this and reporting it to us!


Reply to this email directly or view it on GitHub.

csstaub added a commit that referenced this issue Jul 23, 2015
Fixes issue #40: extraneous zero padding on serialized exponent
@csstaub
Copy link
Collaborator

csstaub commented Jul 23, 2015

Merged fix now. Thanks again!

@csstaub csstaub closed this as completed Jul 23, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants