comments: sstc-saml-holder-of-key-browser-sso-draft-02

["Tom Scavo" <trscavo@gmail.com>]

Here are some comments on the "Holder-of-Key Web Browser SSO Profile"
(sstc-saml-holder-of-key-browser-sso-draft-02):

- [lines 125--128] In the opening sentence to section 1, why must the
"service provider and desired resource [be] understood or implicit"?
Holder-of-key makes that rather unimportant, does it not?

- [line 132] s/its choice/its choosing

- [line 136] s/should rely/relies/

- [line 169] The Document ID is incorrect.

- [line 180] Link broken (literally)

- [lines 183--184] Link broken (literally)

- [section 1.3.1] Specifically, what sections are normative?  Ideally,
what sections apply to the IdP and what sections apply to the SP?

- [lines 210--213] These sentences may be put in section 2.7, I think.

- [lines 216--218]  Why is discovery out of scope?  This is a golden
opportunity to specify a method of IdP Discovery that actually works!
I don't mean to imply that a conformant SP MUST support a certain
certificate extension for the purposes of discovery, but if there
where a well-defined method of discovery specified in this document,
deployments could choose to support it or not as the case may be.  At
least a standard approach to discovery in X.509 certificates would be
an available option.

- [lines 232--236]  This step is out of scope, right?  Not sure why
discovery deserves its own step in the overview if it's out of scope.

- [lines 240--241] Delete this sentence (since it's irrelevant in an overview).

- [line 242]  You skipped a step, namely, the all-important request by
the HTTP UA (which must be made over SSL/TLS).  In fact, too little
attention is paid to the HTTP UA in these steps.  Break out the
requests made by the UA to the SP and the IdP.

- [line 248] I thought mutually authenticated TLS was mandated in this profile?

- [lines 252--253] s/message may indicate an error or will
include/message includes/

- [lines 256--258] Concentrate on non-error behavior in this step.

- [lines 263--266] This paragraph needs to be rewritten since it
doesn't mention the subsections in section 2.5 at all.  In fact, if
you think about it, this paragraph contains the seeds of a new
conformance section that describes the conformance requirements of the
IdP and SP separately.

- [lines 273--279] This paragraph is related to the comment above re
discovery.  I strongly recommend you specify a method of discovery
based on X.509 certificates in this profile.

- [lines 277--279] Delete this sentence.

- [lines 289--293] Does this paragraph belong in section 2.4.4?

- [lines 294--295] This sentence doesn't make sense since it uses the
word "required" inappropriately.  Is it required that the request
issuer be authenticated?  No, I don't think so.  So the use of
normative SHOULD is not at all clear.

- [line 318] s/Issues <samlp:Response> to Service Provider/Issues
<samlp:Response>/

- [line 319] s/Regardless of the success or failure of the
<samlp:AuthnRequest>, the/The/

- [line 319] s/identity provider SHOULD/identity provider MUST/   ?

- [lines 322--232] Join these lines.

- [lines 327--331] I think these lines belong in section 2.4.6.

- [lines 337--339] What if the IdP is unable to honor the SP's
request?  Also, this sentence is irrelevant in the case of IdP-first.

- [line 341] s/information included/information to be included/

- [line 342] s/as well as maintain/as well as to maintain/

- [lines 343--344]  Alternatively, why can't the Response be signed?

- [lines 345--346] This sentence belongs in the next section, I think.

- [section 2.4.6] There is no mention of TLS or subject confirmation
in this section, which is an oversight, I think.

- [lines 354--355] In what sense is this profile "based upon" the Web
Browser SSO Profile?  Earlier you suggested it was an "alternative" to
Web Browser SSO.

- [line 356] Which document are you referring to when you say "that document"?

- [line 365] s/this message is signed/the <samlp:AuthnRequest> element
is to be signed/

- [lines 365--369] This entire paragraph needs to be rewritten.  If
the initial request from the UA was made over TLS, then a
<saml:Subject> element MAY be included in the <samlp:AuthnRequest>
element.  If a <saml:Subject> element is included, it MUST include a
<saml:SubjectConfirmation> element and MUST NOT include a
<saml:NameID> element.  In this case, the <samlp:AuthnRequest> element
MUST be signed.

- [lines 365--369] That said, I don't see why this requirement is
important.  Who cares if the authn request is presented to the IdP
with a different key than that used at the SP?  Moreover, you
apparently don't require the SP to check the key later on, so I don't
see the point.

- [lines 369--371] This sentence belongs in section 2.5.2, I think.

- [line 387] What does "it" refer to?

- [lines 400--401] Is "one and only one" or "at least one"
<saml:AuthnStatement> element required?

- [line 402] s/a <saml:AuthnStatement>/the <saml:AuthnStatement>/   ?

- [lines 404--408] This is too vague.  Can you elaborate on the
choices and perhaps be specific about the content of the
<saml:SubjectConfirmationData> element?

- [lines 415--417] This only applies to SP-first.

- [lines 417--418] I disagree.  If you give the IdP the option to
ignore the <md:RequestedAttribute> elements in metadata, then what's
the point of including an AttributeConsumingServiceIndex in the first
place?

- [section 2.5.4] Where does it say that the key presented to the SP
earlier must match the key presented now?  I don't think this should
be a requirement, I'm just pointing out the inconsistency.

- [line 433] What "other respects" are you referring to?

- [lines 437--438] Discuss in section 2.7 how mutual authentication,
integrity, and confidentiality might be achieved.  What are the
recommendations?

- [lines 442--443] Move these lines to the paragraph at lines 437--438.

- [lines 445--446] Can the <samlp:Response? be signed instead?  (Yes.)

- [lines 460--462] Other authentication methods have this characteristic, right?

- [lines 463--468] This paragraph assumes the use of a long-lived
X.509 certificate.  This should be made explicit (since a short-lived,
opaque certificate won't have any of these limitations).

- Does this profile allow SSL 3.0 in addition to TLS 1.0?

- Is it true that holder-of-key SSO assertions can have longer
validity periods (compared to bearer assertions) without loss of
security properties?  If so, this should be discussed in section 2.7.

Tom Scavo
NCSA