comments: sstc-saml-x509-authn-attrib-profile-draft-11

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

Document identifier: sstc-saml-x509-authn-attrib-profile-draft-11

Errata:

[line 76] s/section /section 3/

[line 77] s/section /section 4/

[line 138] Delete this empty line.

[line 144] s/Based/In step 4, based/

[line 146] s/sequence steps/sequence of steps/

[line 146] s/Sections/sections/

[line 166, 168, 175, 178, 179, 229, 260] s/Section/section/

[lines 171--175] Decrease the indentation of these lines one level.

[line 174] s/isurn/is urn/

[line 178, 229] s/defined/specified/

[line 202] s/the Basic Mode/Basic Mode/

[line 245] s/Optionally, the/The/

[line 252, 277, 300] s/assertions and protocols/Assertions and Protocols/

[line 274] s/assertions/encrypted assertions/

[line 279] s/Encrypted mode/Encrypted Mode/

[line 287] s/Optionally, and if supported by the service provider, the/The/

[line 287] s/Optionally, and if supported by the service provider, and if/If/

[line 310] Replace the curved quotes with straight quotes.

[line 326] s^username/password^credential (most likely a username/password)^

[line 329] s/attribute requester/attribute requester (i.e., the
service provider)/

[line 332] s/service provider/attribute requester/

[line 342] s/situations/deployment scenarios/

[line 384] s/SAML Metadata Extension/Metadata Extension/

[line 385] s/OASIS/OASIS Draft/

[line 391] Remove the extraneous indentation.

[lines 399--402] Remove the bold text.

Specific Comments:

[line 113, 117, 129, 135, 137, 151, 172, 204] What precisely is meant
by the term "X.509v3 certificate"?  In section 1, the term "X.509
certificate" is defined, but "X.509v3 certificate" is used throughout.
 I would suggest either to use the term "X.509 certificate" throughout
or modify the definition in section 1.  If you choose to do the
latter, note that term "X.509 v3 certificate" is used in RFC3280, not
"X.509v3 certificate".

[lines 130--132] I would suggest deleting these lines and replacing
them with this sentence: "The details of this step are out of scope
for this profile."

[line 160, 214] Should the URNs contain the string "2.0"?  If not, how
do we distinguish between this profile and a similar profile bases on
SAML V1.1?

[lines 181--182] These lines are mostly redundant considering the
previous four lines.  Can you combine lines 177--182 into a single
paragraph?

[lines 184--193] Just a reminder that these lines were discussed in
detail on the mailing list.

[lines 188--189] This requirement assumes that the IdP is able to
authenticate the SP, but nowhere in this section is client
authentication required.

[section 3] Why did you delete section 3.3.2 (Error Processing) from
draft-10?  (Not a big deal, I guess---just curious.)

[section 3] What are the security requirements of Basic Mode?  This is
not clear from reading this section.

[line 218] Is Encrypted Mode more properly thought of as an extension
of Basic Mode?  Indeed, section 3 is referenced repeatedly in section
4, so this would seem to be the case.  I think this line should be
modified to reflect this.

[line 244, 286] I suggest you delete this line.  It can't be enforced
so I don't see the point of making such a vacuous requirement.

[lines 287--291] In effect, this key becomes a "previously established
symmetric key."  How long does this key remain a previously
established symmetric key?  In other words, should the IdP cache this
symmetric key, or should it be discarded immediately after use?

[lines 287--296] These two options are mutually exclusive.  This
should be indicated as such.

[lines 303--306] The <Assertion> signature is discussed, but what
about the <Response>?  Must it, too, satisfy FIPS 140-2 Security
Requirements?

[lines 338--339] I totally agree with this sentence, but this is not
mentioned in section 3.

[section 6] Much of the implementation guidance from draft-10 was
omitted from draft-11.  Although the material is only informative, and
therefore expendable, I'm curious why you chose to omit it?

[section 7] Note that much of the text in this section is blue.  Only
the URLs should be blue, I think.

[lines 369--370] This is reference is totally incorrect.  See draft-10
for a precise reference for RFC 3280.

General Comments:

Shouldn't this spec be cast as a "deployment profile"?  I may be
mistaken, but I thought it was agreed that this spec was to be
formulated as a deployment profile.

The diff is evidently against CD-02, but I believe it should be
against draft-10, right?

Can you include a detailed Revision History (from the old, deprecated draft-11)?

The default namespace prefix is ambiguous.  Unqualified elements from
namespaces urn:oasis:names:tc:SAML:2.0:assertion and
urn:oasis:names:tc:SAML:2.0:protocol are used throughout the document.

----------------------
Tom Scavo
NCSA/University of Illinois