dart-lang / tools

This repository is home to tooling related Dart packages.
BSD 3-Clause "New" or "Revised" License
30 stars 21 forks source link

Basic Auth encodes ascii characters #340

Open runesam opened 4 years ago

runesam commented 4 years ago

I will make this as simple as possible.

in method resourceOwnerPasswordGrant given basicAuth value is true by default (which is the right behavior) the method generates a basicAuth of the provided identifier and secret via a method called basicAuthHeader

given identifier is test and secret is test^ (^ is a valid ascii character) basicAuthHeader(identifier, secret)

expected result Basic dGVzdDp0ZXN0Xg== or Basic dGVzdDp0ZXN0Xg actual result Basic dGVzdDp0ZXN0JTVF

which causes an authentication failure.

here is what the method does

String basicAuthHeader(String identifier, String secret) {
  var userPass = Uri.encodeFull(identifier) + ':' + Uri.encodeFull(secret);
  return 'Basic ' + base64Encode(ascii.encode(userPass));
}

hayshi commented 3 years ago

Hello, I'm having the same problem, with other characters like | (pipe). The solution is to remove the encode from the 'secret'

jonasfj commented 3 years ago

https://tools.ietf.org/html/rfc6749#section-2.3.1

2.3.1. Client Password

Clients in possession of a client password MAY use the HTTP Basic authentication scheme as defined in [RFC2617] to authenticate with the authorization server. The client identifier is encoded using the "application/x-www-form-urlencoded" encoding algorithm per Appendix B, and the encoded value is used as the username; the client password is encoded using the same algorithm and used as the password. The authorization server MUST support the HTTP Basic authentication scheme for authenticating clients that were issued a client password.


So something as simple as https://github.com/dart-lang/oauth2/pull/97 probably won't solve this, because the basicAuthHeader function is used multiple places.

I didn't read the RFC cover-to-cover, so may be there is something special for the resourceOwnerPasswordGrant flow that I didn't see -- this is entirely possible.

But is this even a bug? Or is the correct behavior to URL encode client id and secret when using basic auth?

I'm going to label this "needs info" and close this issue, please reopen if the "unexpected behavior" contradicts the specification -- ideally include a citation and link to the relevant part of the specification :D

hayshi commented 3 years ago

@jonasfj the excerpt I believe to address this issue is: https://tools.ietf.org/html/rfc6749#appendix-B `At the time of publication of this specification, the "application/x-www-form-urlencoded" media type was defined in Section 17.13.4 of [W3C.REC-html401-19991224] but not registered in the IANA MIME Media Types registry (http://www.iana.org/assignments/media-types). Furthermore, that definition is incomplete, as it does not consider non-US-ASCII characters.

To address this shortcoming when generating payloads using this media type, names and values MUST be encoded using the UTF-8 character encoding scheme [RFC3629] first; the resulting octet sequence then needs to be further encoded using the escaping rules defined in [W3C.REC-html401-19991224].

When parsing data from a payload using this media type, the names and values resulting from reversing the name/value encoding consequently need to be treated as octet sequences, to be decoded using the UTF-8 character encoding scheme.

For example, the value consisting of the six Unicode code points (1) U+0020 (SPACE), (2) U+0025 (PERCENT SIGN), (3) U+0026 (AMPERSAND), (4) U+002B (PLUS SIGN), (5) U+00A3 (POUND SIGN), and (6) U+20AC (EURO SIGN) would be encoded into the octet sequence below (using hexadecimal notation):

 20 25 26 2B C2 A3 E2 82 AC

and then represented in the payload as:

 +%25%26%2B%C2%A3%E2%82%AC`

the problem occurs in the encoding of the secret.

when we use special characters like | (pipe) for example, the secret :g86|;6^_0FZ8r7s should generate Omc4Nnw7Nl5fMEZaOHI3cw== and using Uri.encodeFull is generating Omc4NiU3Qzs2JTVFXzBGWjhyN3M= equivalent to :g86%7C;6%5E_0FZ8r7s

http.post already encodes "application / x-www-form-urlencoded" when there is a body.

jonasfj commented 3 years ago

http.post already encodes "application / x-www-form-urlencoded" when there is a body.

Are we not talking about the encoding of the basic auth header? And not the payload / body?

hayshi commented 3 years ago

it is a fact, there is a problem when using special characters in the secret, as in the example presented. Another suggestion would be to parameterize this in the method, but I don't see it as ideal

jonasfj commented 3 years ago

for example, the secret :g86|;6^_0FZ8r7s should generate Omc4Nnw7Nl5fMEZaOHI3cw==

@hayshi, how do you get that?

From RFC 6749 Appendix B:

To address this shortcoming when generating payloads using this media type, names and values MUST be encoded using the UTF-8 character encoding scheme [RFC3629] first; the resulting octet sequence then needs to be further encoded using the escaping rules defined in [W3C.REC-html401-19991224].

So apply UTF-8 encoding (which in your example does nothing), then apply W3C.REC-html401-19991224 section 17.13.4:

application/x-www-form-urlencoded

This is the default content type. Forms submitted with this content type must be encoded as follows:

  1. Control names and values are escaped. Space characters are replaced by '+', and then reserved characters are escaped as described in [RFC1738], section 2.2: Non-alphanumeric characters are replaced by '%HH', a percent sign and two hexadecimal digits representing the ASCII code of the character. Line breaks are represented as "CR LF" pairs (i.e., '%0D%0A').
  2. The control names/values are listed in the order they appear in the document. The name is separated from the value by '=' and name/value pairs are separated from each other by '&'.

So replace space by + and anything else non-alphanumeric with %HH (hex encoding). Is | not a non-alphanumeric character that must thus be hex encoded? Hmm, okay, I'm starting to wonder if I'm reading this wrong, or if perhaps ; should then also be encoded :see_no_evil:

Then you can move on to RFC 6749 section 2.3.1 which adds base64 encoding leaving you with Omc4NiU3Qzs2JTVFXzBGWjhyN3M=.


I see intricacies in appendix B, and I haven't checked if we encoding whitespace as + or %20, but unless I'm misreading something that's the only difference I can currently see.

jonasfj commented 3 years ago

Perhaps there is something investigate here, if someone is interested, so far RFC 6749 section 2.3.1 have not convinced me that simply removing the application/x-www-form-urlencoded encoding is correct behavior.

hayshi commented 3 years ago

@jonasfj I returned to this subject and analyzing the implementation of the spring framework, I believe it is a process error, Uri.encodeFull is not necessary.

  1. we transform characters into bytes (eliminating special characters)
  2. encode in Base64
  3. we send in the header using the "application / x-www-urlencoded"

In relation to spring, the only change would be coding from ascii to latin1, spring uses ISO-8859-1. As a reference you can use RFC7617 (The 'Basic' HTTP Authentication Scheme), which made RFC2617 obsolete Ref. https://tools.ietf.org/html/rfc7617#section-2 Page 4 `To receive authorization, the client

  1. obtains the user-id and password from the user,
  2. constructs the user-pass by concatenating the user-id, a single colon (":") character, and the password,
  3. encodes the user-pass into an octet sequence (see below for a discussion of character encoding schemes),
  4. and obtains the basic-credentials by encoding this octet sequence using Base64 ([RFC4648], Section 4) into a sequence of US-ASCII characters ([RFC0020]). `
jonasfj commented 3 years ago

Looking around it seems others also do this: https://github.com/lelylan/simple-oauth2/blob/a6d6aeb3610c354eabec3c3ca350f045694b96bd/lib/client/credentials-encoding.js#L49

I couldn't find the spring code (also my java is a bit rusty).

@hayshi

  1. we transform characters into bytes (eliminating special characters)
  2. encode in Base64
  3. we send in the header using the "application / x-www-urlencoded"

I don't understand the spec clearly says to do URL encoding first before concatenating clientid + password, see https://tools.ietf.org/html/rfc6749#section-2.3.1

I think you need to find an example somewhere credible to convince me that we have a bug here. I wouldn't be surprised if some other implementations don't do the URL encoding, but the spec says to do it, and while other implementations might offer options to not do so, and we could offer such options, lack of an option to deviate from the spec is a feature request, not a bug :D

hayshi commented 3 years ago

@jonasfj As I mentioned above, RFC6749 is referencing RFC2617, which is obsolete, being replaced by RFC7617. Reference from Spring Framework, see lines 1846 (encodeBasicAuth) and 750,769, 789 (setBasicAuth) https://github.com/spring-projects/spring-framework/blob/master/spring-web/src/main/java/org/springframework/http/HttpHeaders.java

jonasfj commented 3 years ago

As I read it, it is RFC6749 that mandates the application/x-www-form-urlencoded encoding, not RFC2617 so the fact that it was been replaced by RFC7617 is of no matter.

Indeed the changes doesn't say anything about application/x-www-form-urlencoded encoding, see: https://tools.ietf.org/html/rfc7617#appendix-A