Re: [apps-discuss] [APPS-REVIEW] review of draft-ietf-kitten-sasl-openid-07

[Alexey Melnikov <[e-mail]>]

Hi William,
Thank you for the review. I will reply to a couple of your points and I 
hope document editors will reply to the rest.

On 29/11/2011 06:24, William Mills wrote:
> I have been selected as the Applications Area Review Team reviewer for this draft (for background on apps-review, please seehttp://www.apps.ietf.org/content/applications-area-review-team).
>
> Please resolve these comments along with any other Last Call comments you may receive. Please wait for direction from your document shepherd or AD before posting a new version of the draft.
>
> Document: draft-ietf-kitten-sasl-openid-07
> Reviewer: William J. Mills
> Review Date: November 28, 2011
> IETF Last Call Date:  October 25, 2011
>
> Review Summary:
>
> This draft is almost ready for publication as a Proposed Standard, but should address the three major issues below before proceeding.  Some minor issues and nits are also noted.
>
> Document Summary:
>
> This document defines a pure SASL mechanism for OpenID, but it conforms to the new bridge between SASL and the GSS-API called GS2 [RFC5801], so it defines both a SASL and a GSS-API mechanism.
> .
>
> Major Issues:
>
> Section  1.2.  Applicability:  This section requires TLS but channel binding is not supported by the mechanism.  OpenID itself does not require TLS for client to relying party interactions, as integrity can be assured with a MAC signature and replayability is dealt with in the OpenID nonce.  Requiring TLS does not appear to be based on the underlying security profile of OpenID.  If TLS is ot be required channel binding should be supported.  If TLS is not required then there is the possibility of a DOS against the return_to entrypoint returned to the user, sending a false failure message.
>
>
> Section 3.2 Authentication Request:  In the second full paragraph defining transaction id, the language here probably isn't strong enough.    What it says now is
>
>     The form of this transaction is left to the RP to decide, but
>     SHOULD be large enough to be resistant to being guessed or attacked.
>
> (Nit: At the very least "transaction" needs to be "transaction id") I think it would be better if the current text is replaced with
>
>     The form of this transaction id is left to the implementer, but it
>     MUST be resistant to being guessed or attacked.
I agree with this, but the MUST by itself doesn't provide enough 
information about how to implement a conformant implementation, so ...
> I think MUST is justified here because the RP is possibly open to a DOS if the value is guessable.  A paragraph in the security considerations section might be warranted to talk about how to pick good unguessable values, although this has been done many times in many different specs.
... There is an RFC on generating good randomness: [RFC4086]. It should 
be cited here.
> Side comment: maybe we need an RFC just for this and then everyone can cite it.
>
> 3.3.  Server Response
>
> The problem I see here is that the result sent to the server that is "used to set state in the server accordingly" is not guaranteed to provide a username  that will be useful to the SASL endpoint.  The RP might get a full email address, or might get a bare username.  In the case of an IMAP server supporting multiple domains this may be significant.  The spec really should define how the SASL identities are determined from the response from the OP.
The authentication identity is specified in Section 3.1 (it is passed 
from the client to the server). However, two things are missing in the 
document:

1). A more precise definition of the authentication identity format. 
Section 3.1 says that it is a URI, but doesn't specify any specific URI 
scheme as mandatory to implement or even as recommended. Lack of 
specificity can lead to types of problem you point out above.

2). Section 3.3 should probably talk about whether any verification of 
the returned data can and should be performed to make sure that it 
corresponds to the identity requested. Unless editors can make a good 
argument why this doesn't belong in Section 3.3.
> It's possible that this could be solved by moving 6.1 and making it 3.3.1.  Identity mapping seems to fit better here than in security considerations.
>
>
> Minor issues:
>
> User confusion on names: The problem I see is one of confusion for the user of an OpenID enabled SASL client for Mail.  Some endpoint will need to be given usrename, some will be given an dOpenID endpoint.  Clarifying language might be useful to guide the client implementer.  Is there a disocvery method that the client can use ot go from a username/domain to the OpenID endpoint ot send to the RP?
>
> More examples:  I'd prefer to see an example of a failure flow included.  I tend to like examples though, and find them helpful in parsing the normative text.
>
> 3.3.  Server Response&  3.4.  Error Handling&  5. Example
>
> There's an inconsistency here I think could be better.  In the Exmaple we have a success case where the client returns and empty client message in order to prompt the server to finalize the SASL negotiation.  In the error handling case we have an explicit continuation from the client sending "=".   Is the "=" sign after the error return actually required or can this simply be an empty client message.  This means if the client knows the negotiation is complete and has not gotten a result it just always sends the empty message.
Personally I would appreciate an error case example.
> Nits:
>
> Section 1, 4th para, first sentence:  I would change "As currently envisioned, this mechanism is to allow" to "This mechanism allows".
>
> Section 1, 5th para, 2nd sentence: "will continued to be" change continued to continue.
Yeah, these make sense.

Best Regards,
Alexey, as the document shepherd.