Re: [OAUTH-WG] comments on draft-hammer-oauth-03

[Peter Saint-Andre <[e-mail]>]

On 11/15/09 3:29 PM, Eran Hammer-Lahav wrote:
> Thanks Peter. This is very helpful.
> 
> The following changes have been submitted as a -04 revision.

Thanks for the fast turnaround! A few comments inline.

>> -----Original Message----- From: [e-mail]
>> [mailto:[e-mail]] On Behalf Of Peter Saint-Andre 
>> Sent: Monday, November 09, 2009 2:14 PM
> 
>> We might also want to point to RFC 4949 regarding terminology and
>> check that our usage of key terms like "authorization" is
>> consistent with that document.
> 
> If someone wants to give it a try, I am open to it.

I think this is more important for OAuth 1.1 so let's fold it into that
effort. In any case I don't think it's a lot of work.

>> 3. Before jumping right into authenticated requests in section 3, I
>>  think it would be helpful to provide a bit more context about the 
>> overall protocol flow (similar to Section 6 of the community
>> edition).
> 
> I flipped between section 3 and 4. This might be enough.

I think that does it, yes.

>> 4. In Section 3.1, we might want to say explicitly which parameters
>> are required and which are not. For example, "Protocol parameters
>> MUST NOT appear more than once per request. The following
>> parameters are REQUIRED unless otherwise noted." Then say that, for
>> example, oauth_version is RECOMMENDED instead of REQUIRED (or
>> whatever).
> 
> In the new section 4.1 I added:
> 
> Each of the protocol parameters listed in Section 4.3 except for 
> "oauth_signature" is assigned a value based on its definition. All
> the parameters MUST be included, except for "oauth_version" which MAY
> be omitted, and "oauth_token" which MUST be omitted when making a
> temporary credentials request (Section 3.1).

That's better, thanks.

>> 5. In Section 3.2, it's not completely clear that the timestamp
>> value must be numerical instead of, say, in ISO 8601 format. It's
>> also not clear how the server would specify that the timestamp is
>> something other than "the number of seconds since January 1, 1970
>> 00:00:00 GMT".
> 
> I made a protocol change to this section, simplifying the text and
> remove the 'equal or greater' requirement (which is not really
> practical when used across multiple machines):
> 
> The timestamp value MUST be a positive integer. Unless otherwise
> specified by the server's documentation, the timestamp is expressed
> in the number of seconds since January 1, 1970 00:00:00 GMT.

+1

>> 8. In Section 3.3.1.1, we might want to be more explicit about
>> which parameters are included in the "specific set of request
>> parameters" (see recent discussion on the community list).
> 
> Entire section rewritten. Review requested.

That's clearer now.

>> 10. In Section 3.3.1.1, the sentence
>> 
>> The "oauth_signature" parameter MUST be excluded if present.
>> might be taken to imply that all other oauth_* parameters MUST be 
>> included (if found in the request). Let's be explicit about that.
> 
> Please check new language.

Works for me.

>> 11. In Section 3.3.4, we say that PLAINTEXT method SHOULD be used
>> with SSL/TLS. Does that deserve to be a MUST? At least it might be
>> worth a mention in the Security Considerations.
> 
> MUST is asking for too much. I added a pointer to the relevant
> security section.

OK.

>> 12. In Section 3.6, we say that "Text values are first encoded as
>> UTF- 8" but it's not fully clear to me which values this applies to
>> or exactly where in the protocol the encoding rules are applied.
> 
> I added an introduction to the section. This has always been a sore
> point and underspecified. I am happy to consider clarifications.

Now it's clear that these rules apply to the signature base string. I
think that it would be even clearer as follows:

   This specification defines the following method for percent-
   encoding of parameter values that are used as input to construction
   of the signature base string:

(I say this because it's not 100% clear which strings are meant in the
current text -- i.e., does the method apply only to the values in the
name/value pairs?)

>> 13. In Section 4, I think that a brief flow diagram would be
>> helpful to illustrate the authorization process (e.g., this would
>> provide context for why the client obtains a set of temporary
>> credentials, which might not otherwise be obivous to first-time
>> readers of Section 4.1).
> 
> I am going to dig Jeff's diagrams and see if I can make them work for
> this.

OK.

>> 14. In Section 4.2 (or in a security consideration pointed to from 
>> Section 4.2), it might be helpful to describe why oauth_callback is
>>  important and the nature of the session fixation attach that it
>> helps to prevent.
> 
> If someone wants to offer text, I am happy to consider. I don't write
> security text.

Perhaps we could borrow and adjust some text from your blog post at
http://hueniverse.com/2009/04/explaining-the-oauth-session-fixation-attack/
:)

>> 15. In Section 6.8, I think there is a non-sequitur. Just because a
>>  client is a freely available desktop application does not mean
>> that an attacker will be able to recover the client credentials.
>> Perhaps one step in the chain of reasoning is not spelled out here?
>> 
> 
> If you have access to the client bits, you can reverse engineer it to
> get the secret. See DVD encryption key...

Again, I must be missing something. Just because I use Thunderbird or
Pidgin or some other free software doesn't mean you can hack into my
email or IM password. How is the case any different here?

>> 17. In Appendix A.1 the phrase "HTTPS POST" is used, but I think
>> that's more accurately an HTTP POST to an SSL-protected host.
> 
> I think it is easier to read this way.

OK. Not a big deal either way.

> Thanks again. Please review -04.

Done.

Peter

-- 
Peter Saint-Andre
https://stpeter.im/