-----Original Message-----
From: oauth-boun...@ietf.org [mailto:oauth-boun...@ietf.org] On Behalf
Of Stephen Farrell
Sent: Thursday, October 13, 2011 10:13 AM
List 1 - Fairly sure these need changes:
----------------------------------------
(1) In 2.3.1 MUST the AS support both HTTP basic and the client_id and
client_secret in the request schemes? You say it MAY "allow"
credentials in the request body, but that's different from what the AS coder
has to implement. Saying an AS that issues passwords MUST support "basic
over TLS" and MAY support including credentials in the body would seem
right here.
Changed 'MAY allow' to 'MAY support'. Clarified the TLS requirement for sending
passwords.
(2) In 2.3.1 (and more generally) what does "MUST require the use of a
transport-layer security mechanism" really mean? I think you need to say
explicitly what this means in terms of TLS and which version of TLS and which
cipher suites etc. Doing that once is fine and you could then have a defined
phrase for it, but it needs to be specified for each place where TLS is used.
The text at the end of
p15 is almost exactly what's needed, and would be if you said which cipher
suites are MUSTs. I think you might need to pick cipher suites for each
version if you want some combination of tls1.0 and tls1.2 and need server
auth at least.
Replaced ' transport-layer' with TLS with reference to the new section.
(3) Having a MUST for TLS1.0 and a SHOULD for TLS1.2 is going to generate a
DISCUSS or two. I think you really need to justify that in the Document and
PROTO write up if you want to stick with the current choices. I personally
would prefer if those were swapped myself, that is have a MUST for the
latest version of TLS (TLS1.2) and a MAY/SHOULD for TLS 1.0. In addition to
taking care of process concerns, there is also the recent TLS chosen plaintext
attack where TLS1.2 has no problem but TLS1.0 does. (This differs from (2)
above, since the former is almost editorial, but I guess this one needs to go
to the WG.)
This has been resolved by the chair and adjusted as such.
(4) The response_type description in 3.1.1 is unclear. You say it MUST be
"one of" various values, but then that it can be a space separated list of
values. When>1 value is provided, it doesn't say what that means, it only
says that the order is irrelevant. (It could mean "any of these" or "all of
these" for example, both are order independent, but are not the same.) I
suggest adding a sentence to say that "code token" means "please send
both" if that's what it means.
Removed the space-delimited text from the end of the response_type parameter
definition and added the following immediate after:
Extension response types MAY contain a space-delimited (%x20) list
of values, where the
order of values does not matter (e.g. response type"a b" is
the same as "b a"). The meaning of such composite response
types is defined by their respective specifications.
(5) How does a client implement the MUST in the last sentence of 3.1.2.3? I
don't see anything here or in 10.15 that says how to do this. I guess ideally,
you'd just need a reference to somewhere else in the doc or to something
else, but I do think you need something since you've made this a "MUST
ensure" rule for clients.
Adding a sentence/short paragraph here or in 10.15 with some typical/good
ways to ensure this would be fine.
I took that last paragraph out. Client mitigation isn't really practical here.
(6) In 7.1 what does it mean to say that the client MUST NOT use the access
token if it doesn't trust the token type? I don't know what code I'd write
there in a client. Maybe s/trust/support/ or s/trust/understand/ would fix
this.
Took 'trust' out.
(7) Doesn't 7.1 need to say which token types are MTI so that we get
interop? I think I'd like to see mac being a MUST and bearer being a MAY but
regardless of my preference, I don't think you can be silent on this. One way
to do this would be to mandate that the client MUST support mac and MAY
support bearer but to allow the server to choose which token types they
support. And as a consequence one or both of the mac/bearer drafts need
to end up as normative I think.
No change as resolved by the chair.
(8) 10.3 says access tokens MUST be kept confidential in transit.
Does that not mean that non-anonymous TLS is a MUST? If so, then saying
that clearly here would be good. If not, then saying what's really meant
clearly is needed. (Same point for refresh tokens in
10.4.)
Added to each section:
[Access token credentials | Refresh tokens] MUST only be transmitted
using TLS as described
in<xref target='tls' /> with server authentication as defined by<xref
target='RFC2818' />.
(9) 10.5 says "effort should be made" to keep codes confidential, but I expect
that'll generate AD comments - that's just a bit too vague - what do you really
mean there?
Took that sentence out. Added no value.
(10) 10.10 has an impossible requirement - you cannot stop/prevent
attackers guessing, but you can ensure that such guessing is futile. Can you
not be a bit more specific about a "reasonable"
level of entropy here? I'd say 10 bits is not enough, 128 is, and there may be a
good level to at least RECOMMEND (e.g. maybe>N bits if rate limiting can
effectively prevent 2^(N/2) guesses going undetected? I'm not
recommending an "N" myself here, but rather that the WG consider picking
one or else justify not picking one.
(The same comment applies to the term "non-guessable" as used in
10.12 and maybe elsewhere as well.)
Per the requirement in the thread-model document, replaced with the following:
Generated tokens and other credentials not intended for handling by
end-users MUST be
constructed from a cryptographically strong random or pseudo-random
number sequence
(<xref target='RFC1750' />) generated by the authorization server.
The probability of any
two values being identical MUST be less than or equal to 2^(-128)
and SHOULD be less than
or equal to 2^(-160).
Added a reference to 10.10 from 10.12.
(11) Section 11 says a couple of times that the apps ADs are the appeal chain -
shouldn't that be the SEC ADs now?
Fixed.
List 2 - Questions: (not sure whether change needed or not)
----------
(1) p12 - 2.1 says that the client developer declares the type of the client,
but
then says that the definition is up to the authorization server really, so, in
principle one client might have different types for different authorization
servers. I think you could make this clearer by adding a sentence saying that
one client could have>1 type according to different authorization servers or
that the client type could change over time, e.g. as some vulnerability in an
OS gets discovered or if a new OS feature is added. I'm not sure if there's
any action that could/should be taken if the client type changes, perhaps re-
registration with a SHOULD for using a new redirection URL and invalidating
all outstanding tokens or something? Maybe this sounds like something that
some other document (and RFC or not) can tackle later.
Added to section 2.1:
A client application consisting of multiple components, each with
its own client type
(e.g. a distributed client with both a confidential server-based
component and a public
browser-based component), MUST register each component separately as
a different client
to ensure proper handling by the authorization server. The
authorization server MAY
provider tools to manage such complex clients through a single
administration interface.
(2) 3.1 - I think you may need more about how passwords are handled, esp.
when they contain odd characters. Is there really no problem with x-www-
form-urlencoded when such a password is in a form? Have you checked with
someone who really cares about such things?
Not sure I understand this, but how passwords are handled is outside the scope.
Basically, the user-agent is sent to the endpoint and the server shows some
kind of form. How that form is submitted is not part of OAuth is not part of
the form encoding described. The form encoding mentioned is just about inputs
send to the server before any login prompt.
If this is not clear, please let me know.
(3) Generally, the specification is a bit unclear about what each component
MUST do, it's hard to figure that out with the style of describing the 2119
rules on a per endpoint basis. Not sure what to suggest that'd not be loads of
work, but I wondered if there's evidence that someone not involved in the
WG or Oauth1 would find it easy or very hard to implement and get interop?
I know that there've been some people who've implemented from the
drafts and I'm told some of those were not involved in the WG at all. If you
know of any such implementers, it'd be good to note that in the PROTO
write-up since it'd answer this question, which I'd expect to be posed by
some AD.
We've got feedback from people who just read the spec without being involved
here (most of it in the past couple of months) and it did not raise this issue.
(4) Is it clear that a client always knows when a redirection request will
result
in sensitive data being sent, thus triggering the SHOULD for use of TLS in
3.1.2.1? It may well be the case but it wasn't clear to me from reading
whether I could write code that'd know when it's ok to not use TLS.
Changed to:
The redirection endpoint SHOULD require the use of TLS as
described in
<xref target='tls' /> when the requested response type is
"code" or "token", or when the redirection request will result
in the transmission of sensitive credentials
over an open network.
(5) 3.1.2.2 says AS'es MAY support>1 redirection URI. Why not make that a
MUST? If a client needed it, then it couldn't interop with an AS that only
supports 1, and it ought be easy for an AS to support>1 I guess.
Because this is not an interop issue. The number of supported redirection URIs
is a matter of configuration. Until we have an spec for interop on that side,
we don't need to prescribe it. Most 1.0 sites today support only one.
(6) What's the case where the AS is ok to redirect to an unregistered or
untrusted URI that corresponds to the 2nd para of 3.1.2.4?
I assume you are asking why isn't this a MUST NOT?
That sentence is a little bit tongue-in-cheek... In 3.1.2.2 we say that
unregistered endpoints SHOULD NOT be allowed and this one is a way of saying
why they are bad in a twisted way. After reading it again, I took that line out
and added this to 3.1.2.2:
Lack of a redirection URI registration requirement can enable an
attacker to use
the authorization endpoint as open redirector as described in
<xref target='open-redirect' />.
(7) Can a client really ensure that other scripts don't go before its own as
required in 3.1.2.5?
Yes, for example, by using multiple endpoints chained in the right order.
(8) What is the impact of the "MUST be taken into consideration" at the end
of 3.2.1? I can't see how to implement that nor is it clear what checks an AS
could do at registration time.
Dropped that line. 10.1 already covers this.
(9) Does the recent list discussion about scope encoding require any changes
to 3.3?
Yep. Been applied.
(10) Is there anything that needs saying if the AS ignores the client's
requested scope but doesn't indicate that in the response?
Put another way - why is that SHOULD not a MUST? (Just asking that you
explain, not change.)
No reason. I originally put it in as a SHOULD because scope was more of an nice
idea than an important feature. This should probably be changed to MUST. Not
going to make this change now, but will if the WG would like to. Will bring up
in another thread.
(11) State and scope variables probably should not contain anything sensitive
for the client or user, since the AS and UA both get to see them, is that
stated somewhere? (Maybe with a hint that if you need something sensitive
in the state, then just generate a symmetric key and encrypt it for yourself.)
An example of what not to include would be a banking client including a
user's bank a/c number as the state variable.
Added to 10.8:
The "state" and "scope" parameters
SHOULD NOT include sensitive client or resource owner information in
plain text as they
can be transmitted over insecure channels or stored insecurely.
(12) 4.4.3 says a refresh token SHOULD NOT be included. Seems like the AS
actions for good requests are fairly variable, which is likely to cause
developers to do the wrong thing - is there no way to make the AS actions
more consistent? (Could be that they are consistent though, but I'm just
getting lost in the text;-)
Yeah...
This specification sucks when it comes to prescribing interoperable behavior.
At this point the biggest reason people hate OAuth 1.0 is not the crypto but
that every server implementation is different. Wait until they experience
2.0... But unfortunately this is as far as this WG was able to agree. WG
participants didn't want to come together and agree but to make sure they will
be able to implement their own unique use cases (and screw interop).
I think the only hope moving forward is for someone to publish a profile that
is tight and restrictive and see if it gains traction.
Yeah, I'm bitter about the quality of this work after investing 4 years into
it, but that's life.
(13) 10.9 says that the client MUST verify the server's cert which is fine.
However, does that need a reference to e.g. rfc 6125?
Also, do you want to be explicit here about the TLS server cert and thereby
possibly rule out using DANE with the non PKI options that that WG (may)
produce?
Don't know anything about this. Will bring up in another thread.
(To be continued...)
EHL
