openid-certification / oidctest

THE CERTIFICATION TEST SUITE HAS BEEN MIGRATED TO A NEW SERVICE https://www.certificatinon.openid.net
Other
49 stars 15 forks source link

rp-id_token-bad-sig-{hs,rs}256 returns somewhat invalid base64 #227

Open mettke opened 4 years ago

mettke commented 4 years ago

Hey folks,

I'm currently updating openidconnect-rs to the newest base64 version. However I'm having difficulties as the tests for rp-id_token-bad-sig-rs256 and rp-id_token-bad-sig-hs256 fail on decoding base64.

After a bit of digging I found that the new base64 parser contains better error checking thus failing as the returned base64 for the signature is invalid. The description of the test, however, says that the test should fail on token validation failure and not on base64 decoding error.

Therefore I propose changing that response to contain valid base64 using canoncial encoding containing an invalid signature as described in [1]

Further Info

The base64 that is returned from the endpoint is:

1234ABCDDDILNNRRRUWZ__aaaccdefghijkklosvwxx

If we base64 decode that we get this (bytes in hex representation):

d7 6d f8 00 10 83 0c 32 0b 34 d4 51 45 45 99 ff f6 9a 69 c7 1d 79 f8 21 8a 39 24 96 8b 2f c3 1c

Returning that back to base64 we get the following:

1234ABCDDDILNNRRRUWZ__aaaccdefghijkklosvwxw

Now lets have a look at the last character of both base64 text (x and w) in binary:

x: 01111000 w: 01110111

With the w all padding bits are 1 to match Canonical Encoding from [1]

References

zandbelt commented 4 years ago

without digging in, you refer to base64 encoding but OIDC actually requires baes64url encoding (which implies stripped padding characters) see https://tools.ietf.org/html/rfc4648#section-3.2 and https://tools.ietf.org/html/rfc7515#appendix-C

mettke commented 4 years ago

Yes padding bytes (or characters) are not present in baes64url. However I'm talking about padding bits. Base64 transforms 6 bits to 8. Thus if we transform 1 character (8bits) we transform it to 16bits using 4 padding bits.

plain: 01100001 (letter a) baes64url: 01011001 01010001

Process:

011000 010000
|-------||--|
letter a  padding bits

011000 becomes 01011001 and 010000 becomes 01010001 as described in the RFC

Now to our situation:

Our plain data ends with c3 1c (the last base64 block to transform). That should be transformed to (keep note of the last 2 padding bits which are zero):

plain:        11000011 00011100
6-bit blocks: 110000 110001 1100(00)
base64url:    wxw

Now lets have a look at the padding bits of the base64url that is actualy returned from the endpoint:

base64url: wxx
6-bit blocks: 110000 110001 1100(01)
plain: 11000011 00011100 (01)

Instead of being 0 the last padding bit is 1. Thus is does not conform to Canonical Encoding.

I hope this makes it a bit more understandable. One cannot randomly generate character inside of the base64url alphabet as it may not reassemble canonical base64.

zandbelt commented 4 years ago

ah, I see: you're observing that the test suite whilst corrupting the signature for the purpose of this bad-sig tests, creates an invalid base64 sequence, right?

mettke commented 4 years ago

Yes exactly. Thus it fails while trying to decode the base64 instead of failing due to the invalid signature