[kitten] [APPS-REVIEW] review of draft-ietf-kitten-sasl-openid-07

William Mills <wmills@yahoo-inc.com> Tue, 29 November 2011 06:24 UTC

DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=ginc1024; d=yahoo-inc.com; h=X-YMail-OSG:Received:X-RocketYMMF:X-Mailer:Message-ID:Date:From:Reply-To:Subject:To:Cc:MIME-Version:Content-Type:Content-Transfer-Encoding; b=nx/Kffth5avxdJsmMyr+VKofeEBmvSnvj66TCah7scFGKHbQ9b7lCYT8PBUioLNi0eYHfjiS8sK5NKsSQlVzTLx1waSq3uWfekgo4TiFoHTZI9ZXNVIZd6zF43+C+7XPPOZar0Hmqp3mij7MgBKhCK9HxLqof0FNFfWxWIqd9sM=;
Message-ID: <1322547891.26139.YahooMailNeo@web31812.mail.mud.yahoo.com>
Date: Mon, 28 Nov 2011 22:24:51 -0800
From: William Mills <wmills@yahoo-inc.com>
To: "apps-discuss@ietf.org" <apps-discuss@ietf.org>, "draft-ietf-kitten-sasl-openid.all@tools.ietf.org" <draft-ietf-kitten-sasl-openid.all@tools.ietf.org>
MIME-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Cc: "kitten@ietf.org" <kitten@ietf.org>, "iesg@ietf.org" <iesg@ietf.org>
Subject: [kitten] [APPS-REVIEW] review of draft-ietf-kitten-sasl-openid-07
Precedence: list
Reply-To: William Mills <wmills@yahoo-inc.com>

I have been selected as the Applications Area Review Team reviewer for this draft (for background on apps-review, please see http://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 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.  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.

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.


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.

[kitten] [APPS-REVIEW] review of draft-ietf-kitte… William Mills
Re: [kitten] [APPS-REVIEW] review of draft-ietf-k… Alexey Melnikov
Re: [kitten] [APPS-REVIEW] review of draft-ietf-k… William Mills