don't use '=' padding in base64 URL encoding of S256 code challenge method, be strict about any additional characters
dmitshur opened this issue · 1 comments
(Hello! It's really cool to find a Go implementation for the IndieAuth spec! I've also implemented one; see shurcooL/home#34 and shurcooL/home#43. When trying to sign in to your site which I understand uses this package, I found a problem and wanted to report it.)
I believe there's a small bug in the PKCE verification as currently implemented in the latest version of this package, to do with padding used in base64 encoding.
The IndieAuth spec defers to RFC 7636 for PKCE details:
All IndieAuth clients MUST use PKCE ([RFC7636]) to protect against authorization code injection and CSRF attacks. A non-canonical description of the PKCE mechanism is described below, but implementers should refer to [RFC7636] for details.
(Source: https://indieauth.spec.indieweb.org/#authorization-request.)
RFC 7636 section 4.6 (https://datatracker.ietf.org/doc/html/rfc7636#section-4.6) details how the code challenge method verification is done:
If the "code_challenge_method" from Section 4.3 was "S256", the received "code_verifier" is hashed by SHA-256, base64url-encoded, and then compared to the "code_challenge", i.e.:
BASE64URL-ENCODE(SHA256(ASCII(code_verifier))) == code_challenge[...] If the values are not equal, an error response indicating "invalid_grant" as described in Section 5.2 of [RFC6749] MUST be returned.
In https://datatracker.ietf.org/doc/html/rfc7636#section-3 it is defined that "Base64url Encoding" refers to URL encoding without padding:
Base64 encoding using the URL- and filename-safe character set defined in Section 5 of [RFC4648], with all trailing '=' characters omitted ([...]) and without the inclusion of any line breaks, whitespace, or other additional characters. (See Appendix A for notes on implementing base64url encoding without padding.)
(Emphasis mine.)
Go's base64.RawURLEncoding implements URL base64 encoding without padding, and can be used instead of base64.URLEncoding.
I'll send a PR that fixes this issue in case you find it helpful.