Comments on Auth 2.0 - Pre-Draft 11
Johnny Bufu
johnny at sxip.com
Tue Dec 12 00:41:01 UTC 2006
Hi Johannes,
Josh and I went through the remaining issues, so I have addressed and/
or commented on some of them below.
For easier tracking I've inserted [josh] after the ones that Josh
agreed to handle.
Thanks again for the feedback! The spec looks definitely better now
than a few days ago.
Johnny
On 11-Dec-06, at 9:41 AM, Johannes Ernst wrote:
> Commenting just on the remaining items ...
>
>>> 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.
>
> I don't understand what you are saying. Are you saying you are
> making a change to the document (if so, which?) or not, in which
> case -- are you saying that entering a private identifier at a site
> is or isn't prohibited? (I am arguing that it should not be
> prohibited)
During IIW we removed the "Public Identifier" from the definitions
section, because it was used in only one other place in the document.
The latest version in SVN allows any type of identifiers (private /
public / OP-) to be entered at the RP.
>>> 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
>
> I understand that is one possible fix. What about we define one of
> the possible fixes as the "canonical" fix for text data, otherwise
> different implementors will implement different fixes (base64, C-
> style \n, URL-style %0D%0a, ... ) and interop will suffer.
[josh]
>>> 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.
>
> Is there a proposed solution to this issue or does it remain open?
We've made is a bit more strict; it now reads:
> When a message is sent as a POST, OpenID parameters MUST only be
> sent in and processed from the POST body.
>>> 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.
>
> My point is that you "assume" and I "assume" and it would be good
> if we all assumed the same. Which is why I'm proposing to at least
> put a SHOULD in there for that assuming.
Josh and I agreed that the language should not be specified, and be
left to RPs / OPs to choose the one that is most useful for their
users. We have edited it and it now reads:
> A human-readable message indicating the cause of the error.
which describes the destination of message better than the previous
term ('unstructured').
>>> 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
>
> Proposed resolution?
Rephrased a bit; the hint about the state of the market was felt
necessary to explain why the default is different than the
RECOMMENDation. It now reads:
> HMAC-SHA1 is the default for authentication requests.
> If supported, the use of HMAC-SHA256 is RECOMMENDED.
>>> 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.
>
> Proposed resolution?
[josh]
>>> 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.
>
> I am referring to par 1 sentence 2, where is says "needs to"
> instead of "MUST ... according to the following algorithm".
> Then to the entire paragraph 2, which says SHOULD.
>
> I'd like an algorithm that produces the same normalized identifier
> from the same entered text string, regardless of site.
[josh]
>>> 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?
>
> I'd prefer a MANDATORY in every place where something is mandatory
> -- less likely to be misread that way.
Section 4.1 now specifies that:
> Throughout this document, OpenID message parameters are REQUIRED,
> unless specifically marked as OPTIONAL.
>>> 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.
Done, spec updated with your text above.
>>> 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.
>
> In which case I misunderstood the current wording. Maybe your
> sentence here "openid.claimed_id and openid.identity are either
> both present or both absent" could be added.
Done, it now reads:
> "openid.claimed_id" and "openid.identity" SHALL be either both
> present or both absent.
>>>> 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.
>
> So I modify my request to modify the wording in a way that it is
> easier to read.
[josh]
>>> --
>>>
>>> 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).
Rephrased a bit, so that the intent is clearer; it now reads:
> The realm SHOULD be used by OPs to uniquely identify Relying
> Parties. For example, OPs MAY use the realm to allow the end user
> to automate approval of authentication requests.
>>> Also, this is the place where to say that OPs cannot prevent RPs
>>> from doing something else than the realm they give.
>
> Resolution?
Not sure what exactly you meant by "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.
[josh]
>>> 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.
Done, changed to "there are Identifiers for which the user is
authorized to issue authentication responses".
>>> 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.
>
> Sorry for being unclear. Second sentence in 10 says "... should
> send a positive assertion ..."
Done.
> What about my other questions here? Resolution?
>
>>> 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?
>
> It constrains the implementation space to something that people
> know how to deal with. For example, how long do I need to make the
> database field? Will somebody send \01?
Done, used the same restriction as for the association handles
(33-126 range).
>>> 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.
>
> Resolution?
I'll try to rephrase the three paragraphs in there. In the meantime,
it would help if you could point what you found the most confusing.
>>> 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.
>
> If there is supposed to be a mechanism (convention?) for specifying
> multiple values for the same parameter, shouldn't that be defined a
> bit more prominently? For example, it appears that the comma
> suddenly has turned into a meta-character, while previously only
> \0d was a meta-character.
>
> [I stand by my opposition to multiple-valued parameters that I
> expressed earlier. But if everybody else thinks it is a good idea,
> then at the very least, define a consistent way of using them.]
We have removed the RECOMMENDation and left it to the extensions to
deal with this individually:
> Extensions MUST NOT define multiple parameters with the same name.
> Extensions that need to send multiple values for the same parameter
> name must define their own conventions for doing so.
>>> 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.
>
> Resolution?
[josh]
>>> 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?
>
> Instead of:
> "An eavesdropper could also intercept a successful authentication
> assertion and re-use it, if the nonce is not checked."
>
> what about"
> "Nonces MUST be checked in order to prevent an eavesdropper from
> intercepting and re-using a successful authentication."
Rephrased to:
> If the nonce were not checked, an eavesdropper could also
> intercept a successful authentication assertion and re-use it.
More information about the specs
mailing list