Updating base64 to 0.12 #21
Conversation
|
Ok I had a look at the test failures and they only happen sometimes as rp sometimes returns base64 which does not conform to canonical encoding that requires flexible parsing to accept. I did create an issue upstream openid-certification/oidctest#227 and we may either wait for that to get in or use Config::decode_allow_trailing_bits to get rid of this error and make the base64 decoding a bit more forgiving (as it currently is) |
There was a problem hiding this comment.
thanks for looking into this! my assumption is that if the OIDC certification tests are producing invalid trailing bits, then other libraries and implementations probably will as well. to ensure compatibility, I think we should maintain the current permissive parsing behavior (i.e., decode_allow_trailing_bits with the new version). I don't think there are any security implications from doing that.
base64 removed the whitespace stripping. However the auther provides a code snippet on the RELEASE Page which fixes that [1] `filter(|b| !b" \n\t\r\x0b\x0c".contains(b)`. Unfortunatly that introduces an additional allocation which, however, also exited before but was inside base64. Furthermore forgiving base64 parsing was disabled upstream by default. Therefore it was necessary to reenable it
|
I added |
Codecov Report
@@ Coverage Diff @@
## master #21 +/- ##
==========================================
- Coverage 79.55% 79.55% -0.01%
==========================================
Files 16 16
Lines 3512 3531 +19
==========================================
+ Hits 2794 2809 +15
- Misses 718 722 +4
Continue to review full report at Codecov.
|
|
great work, thanks! |
I had to split them. I underestimated the changes made to base64.
base64 removed the whitespace stripping. However the
auther provides a code snippet on the RELEASE Page
which fixes that [1]
filter(|b| !b" \n\t\r\x0b\x0c".contains(b).Unfortunatly that introduces an additional allocation
which, however, also exited before but was inside
base64.
The change to the test is necessary as the error
detection was improved. Now it is necessary that the
invalid signature is made out of valid base64