Comments on Auth 2.0 - Pre-Draft 11

[Johnny Bufu]

Johannes,

On 8-Dec-06, at 11:08 AM, Johannes Ernst wrote:
> Dear Authentication 2.0 editors,
>
> I know you are going to hate me ("more changes!"), but I hope the  
> attached comments are useful as you construct the final version of  
> the OpenID 2.0 Authentication document.

Not at all! As an implementer of the spec, I can assure you that more  
clarity is always desired.

Thanks for all the patches. I've already checked-in some of them, and  
on others I have commented below.


Johnny


> Section 2 - Terminology:
>
> It says:
>> OP Endpoint URL:
>>     The URL which accepts OpenID Authentication requests,  
>> discovered by dereferencing the end user's Identifier. This value  
>> MUST be an absolute URL.
>
> "Deferencing" is a term that's unclear in this context.
>
> Better:
>> OP Endpoint URL:
>>     The URL which accepts OpenID Authentication requests, found by  
>> performing discovery on the the end user's Identifier. This value  
>> MUST be an absolute URL.
>

Done.

> --
>
> The terminology section (between "User-supplied Identifier" and  
> "Public Identifier") implies that I MUST NOT ever enter a Private  
> Identifier at a Relying Party. While I understand that this might  
> not be the usual case, I don't think it should be prohibited at all.
>
> Better:
>> User-supplied Identifier
>>     An Identifier that was presented by the end user to the  
>> Relying Party. During the initiation phase of the protocol, an end  
>> user may enter either a Public Identifier, a Private Identifier or  
>> an OP Identifier. If an OP Identifier is used, the OP may then  
>> assist the end user in selecting either a Public Identifier or a  
>> Private Identifier to share with the Relying Party.
>

The Public-Identifier was removed from the terminology section, as it  
was used in a single other place in the spec. The user-supplied  
identifier can be an identifier that the user owns (private or  
public), or an OP Identifier.

> --
>
> Section 4.1.1 - Key-Value Form Encoding
>
> If in the key-value form, I wish to transmit a value that includes  
> a '\n', what am I supposed to do?

Encode it such that it doesn't have a '\n' in it, e.g using base64.  
If  '\n' was allowed, the protocol would permit the kind of attack  
described in this thread:
http://openid.net/pipermail/specs/2006-November/000901.html

> --
>
> Section 4.1.2 HTTP Encoding
>
> Second paragraph currently says:
>> All of the keys in the request message MUST be prefixed with  
>> "openid.". This prefix prevents interference with other parameters  
>> that are passed along with the OpenID Authentication message. When  
>> a message is sent as a POST, the application processing the HTTP  
>> request MUST only use the values in the POST body and MUST ignore  
>> any GET parameters.
>
> I think I pointed out earlier that this is more restrictive than  
> necessary, and prevents certain implementations that make sense,  
> such as using a service endpoint URL like
>     http://example.com/endpoint?bizmodel=free
>     http://example.com/endpoint?bizmodel=premium
> because it says that those parameters must be dropped.
>
> Further, are you guys sure that there is such a thing as a "GET  
> Parameter" in the appropriate URI / HTTP standards? If so, I wonder  
> where that is defined, because I can't find it.
>
> Better:
>>  All of the keys in the request message MUST be prefixed with  
>> "openid.". This prefix prevents interference with other parameters  
>> that are passed along with the OpenID Authentication message. When  
>> a message is sent as a POST, the application processing the HTTP  
>> request MUST ignore those values provided as GET parameters for  
>> which identically-named POST parameters exist in the same request.
>
> --
>
> 4.2 Integer representation
>
> I love your use of "henceforth". It has always been one of my  
> favorite words in the English language. ;-)
>
> --
>
> 5.1.2.2 Error Responses, and also
> 5.2.3 Indirect Error Responses
>
> Please clarify which language is supposed to be used for the  
> "error" field, and what a party should do that receives such an  
> error string, such as:
>
>> #  error
>>     Value: Unstructured text error message that SHOULD use the  
>> English language. This error message is intended to be used by  
>> technically-savvy personnel to debug problems. It is not intended  
>> to be shown to the end user.
>

My opinion is that specifying a language is out of scope for the  
spec; it's up to the RP/OP to choose it. Being unstructured text, I  
assume it's intended to be passed (eventually) to a human behind the  
party receiving it.

> --
>
> 5.2.2 HTML FORM Redirection
>
> I don't think you need to restrict this to JavaScript. There are  
> other languages, too, and there is no need to create a dependency  
> on JavaScript here. Better:
>>  A mapping of keys to values can be transferred by returning an  
>> HTML page to the User-Agent that contains an HTML form element.  
>> Form submission MAY be automated, such as by using JavaScript.
>

Done - your wording is clearer, but your reasoning and conclusion  
about the previous wording weren't exactly right: specifying that  
form submission "MAY be automated using JavaScript" doesn't imply  
that it cannot / must not be automated using something else.

> --
>
> 6.3 Signature Algorithms
>
> Everything after "RECOMMENDED" is unnecessary because it expresses  
> an opinion about the state of the market, which has no role in this  
> kind of document
>
> --
>
> 7.1 Initiation
>
> Given recent discussions on logo and User Experience, this needs to  
> be different. Instead of:
>
>>  To initiate OpenID Authentication, the Relying Party SHOULD  
>> present the end user with a form that has a field for entering an  
>> Identifier.
>>
>> It is RECOMMENDED that a Relying Party place the OpenID logo at  
>> the beginning of the form field where the end user enters their  
>> Identifier. This aides in end user recognition that they can use  
>> an OpenID enabled Identifier at the Relying Party.
>
> Better:
>
>> This document does not define a user experience. It is RECOMMENDED  
>> that implementors follow the OpenID user experience if and when  
>> such an OpenID user experience has been defined in a separate  
>> document.
>
> --
>
> 7.2 Normalization
>
> I'm not sure that this -- all of which is OPTIONAL -- should be in  
> this document. I would suggest to either make it MANDATORY -- or to  
> take it out of this document and refer to a User Experience  
> document instead.
>
> The problem is that if the user can type in something incomplete at  
> site A, and then types in the same incomplete thing at site B, it  
> may work at A but differently at B, which is no good. So either  
> make these rules MANDATORY, or delegate them into the user experience.

Normalization is not optional at all - not sure why you understood it  
was. Maybe that section needs to be clarified, if you can point it to  
us.

> --
>
> 7.3.1 Discovered Information
>
> Typo. Instead of:
>
>> OP Endpoint URL:
>>     The absolute URL on the OP that accepts authentication requests.
>
> it should be:
>> OP Endpoint URL:
>>     The absolute URL of the OP that accepts authentication requests.
>

Done.

> Also, does this section miss a comment about what to do if multiple  
> endpoints are discovered? (Eg because the XRDS file contained  
> multiple OpenID Auth Service Types, or because one of them  
> specified multiple end point URIs?)
>
> So instead of:
>>  Upon successful completion of discovery, the Relying Party will  
>> have the following information (see the Terminology section for  
>> definitions):
>
> I would write:
>>  Upon successful completion of discovery, the Relying Party will  
>> have one or more sets of the following information (see the  
>> Terminology section for definitions). If more than one set of the  
>> following information has been discovered, the precedence rules  
>> defined in [[Yadis]] are to be applied.
>

Done, but I believe it's [XRI Resolution] the one that defines the XRDS.

> --
>
> 8.1.1 Common Request Parameters, and
> 8.2.1 Common Response Parameters, and
> 8.2.2, and
> 8.2.3, and
> 8.2.4, and
> 9.1 (some of them)
>
> It is unclear which of those parameters are MANDATORY and which are  
> OPTIONAL
>

All parameters that are not marked as optional are mandatory. Maybe  
we should include this generic statement somewhere?


> --
>
> 9.1. Request Parameters
>
> The thing about extensions in there is a great big kludgey hack.  
> I'm sure you guys are aware of it and have good reasons for it, so  
> I won't complain, but it's a kludge anyway.
>
> also:
>
>> If the RP needs to ensure that query parameters are not modified  
>> by outside parties, it must prevent this through an out-of-band  
>> method.
>
> I'm not sure the term "out-of-band" should be used. Better:
>
>> This document does not define a mechanism by which the RP can  
>> ensure that query parameters are not modified by outside parties;  
>> such a mechanism can be defined by the RP itself.
>
> also:
>
> The language about claimed_id and identity, and which is optional  
> and which not under which circumstances is very confusing. This  
> needs better copy writing with a clear structure, such as:
>
>> #  openid.claimed_id
>>
>>     Value: The Claimed Identifier. If, and only if,  
>> openid.identity is present, the value is MANDATORY. In all other  
>> cases, the value is OPTIONAL.
>>

Hmm.. your wording doesn't convey the same meaning. openid.claimed_id  
and openid.identity are either both present or both absent, which I  
think the current wording makes clear.

Your text would allow for the openid.identity to be absent and the  
openid.claimed_id to be present.

>>     Note: If an OP-SPecific Identifier is not supplied, the  
>> Claimed Identifier is considered to have the same as the OP- 
>> Specific Identifier. If neither value is present, the assertion is  
>> not about an identifier, and will contain other information in its  
>> payload, using extensions (Extensions).

This doesn't seem right; I read your text like this:

> "If an OP-Specific Identifier is not supplied"
and therefore openid.identity = "http://openid.net/identifier_select/ 
2.0"
> "the Claimed Identifier is considered to have the same as the OP- 
> Specific Identifier."
openid.claimed_id = "http://openid.net/identifier_select/2.0"

Which is fine, but doesn't cover the remaining cases, i.e. when  
Claimed Identifiers / OP-Specific Identifiers *are* supplied.

The original / current wording does cover these cases, albeit I admit  
it is not very easy to read.

> --
>
> 9.2. Realm
>
> Remove last sentence in first paragraph, because it is unclear what  
> this is needed for. (Or, alternatively, explain why an OP needs to  
> uniquely identify RPs).
>
> Also, this is the place where to say that OPs cannot prevent RPs  
> from doing something else than the realm they give.
>
> --
>
> 10 Responding to Authentication Requests
>
> First sentence:
>> When an authentication request comes from the User-Agent via  
>> indirect communication (Indirect Communication), the OP SHOULD  
>> identify the User-Agent, and determine whether the end user wishes  
>> to complete the authentication.
>
> I have no idea what the term "identify" means here. Do you mean:
>> When an authentication request comes from the User-Agent via  
>> indirect communication (Indirect Communication), the OP SHOULD  
>> determine the validity of the current session of the User-Agent  
>> with the OP, and -- with or without direct interaction with the  
>> user, this is left to implementors -- determine whether the end  
>> user wishes to complete the authentication with this particular RP.
>
> Also: I don't know what "there are Identifiers in the control of  
> the end user" means either:
>>  If the relying party requested OP-driven identifier selection by  
>> setting "openid.identity" to "http://openid.net/identifier_select/ 
>> 2.0" and there are Identifiers in the control of the end user, the  
>> OP SHOULD allow the end user to choose which Identifier to use.
>
> There also seems to be an "uppercase missing" problem.
>
> Do you mean:
>> If the Relying Party requested OP-driven identifier selection by  
>> setting "openid.identity" to "http://openid.net/identifier_select/ 
>> 2.0", and the OP provides facilities to manage more than one  
>> Identifier on behalf of the same user, the OP SHOULD allow the end  
>> user to choose which Identifier to use.

Not sure where the uppercase is missed; the SHOULD above appears as  
uppercase in the spec.

> --
>
> 10.1 Positive Assertions
>
> Re response_nonce: it would be good to better define "additional  
> characters". What about
>> The nonce MUST start with the current time on the server, and MAY  
>> contain up to 16 additional characters at the end as necessary to  
>> make each response unique. Any such additional character MUST be  
>> taken from the set of printable ASCII characters.

Not sure how this would help?

> --
>
> 10.2.2 In Response to Non-Immediate Requests
>
> Last sentence -- what about saying instead:
>
>> Often, if the user does not wish or cannot perform authentication,  
>> the sequence of steps to authenticate the use will be aborted, and  
>> the Relying Party will not get a cancel mode response. For  
>> example, the end user may press the back button in their User- 
>> Agent instead of continuing. If a RP receives the "cancel"  
>> response, authentication was unsuccessful and the RP MUST treat  
>> the user as non-authenticated.
>

Done.

> --
>
> 11.2.2.2 Response Parameters
>
> Not clear which values MUST be present and which not.
>
> Also:
>
> the language in this section is confusing. I don't quite understand  
> it. Not sure I can make a suggestion how to explain it better,  
> because so far I don' tunderstand it.
>
> --
>
> 12.1 Updated Initiation and Discovery
>
> Bullet three: missing a "the". Should read:
>> #  When URLs are used as Identifiers, they are normalized  
>> according to the guidelines in [RFC3986] (Berners-Lee, T.,  
>> “Uniform Resource Identifiers (URI): Generic Syntax,” .), for  
>> better compatibility with the existing Web infrastructure.
>

Done.

> --
>
> 13. Extensions
>
> Last paragraph:
>>  Extensions MUST NOT define parameters with the same name. It is  
>> RECOMMENDED that commas are used as value delimiters, though other  
>> characters may be better suited in certain situations. Another  
>> approach is to append a numeric value to each key to differentiate  
>> between each value.
>
> What's that business about the comma? Does this relate to any other  
> part of this document, or is this a left-over from a previous  
> revision?

It offers an alternative to specifying multiple parameters with the  
same name, which forbidden by section 4.1.

>
> --
>
> 14. Discovering OpenID Relying Parties
>
> Can I ask to append the following sentence?
>> The <xrd:URI> element is OPTIONAL for this use of the  
>> <xrd:Service> element. If the <xrd:URI> element is not given, it  
>> is assumed to be the URI on which Yadis discovery was performed to  
>> lead to the XRDS document.
>
> This would be a useful default that would come very handy. And  
> right now, the way it is written, this is underdefined in any case.
>
> --
>
> 15.1.1 Eavesdropping Attacks
>
> I do not understand the phrase "if the nonce is not checked"  
> because above, it says that checking nonces is mandatory.
>

I read it as a reinforcement. Is there a better wording for it?

> --
>
> 15.6 Protocol Variants
>
> SOme of the sentences in the second column need ? at the end, not  
> periods.
>

Done.

> --
>
> Appendix A.2 XRDS
>
> The last use of "http://www.example.com/" needs a trailing slash.
>

Done.