[abfab] Review of draft-ietf-abfab-aaa-saml-09

[tshields <[e-mail]>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

I don't have any major notes or concerns addressing the draft as a
whole as it was concise and easy to read through. Below are notes on
my questions and suggestions for improvement on some specific areas.


Section 2, Paragraph 5
> This document aspires to the guidelines stipulated by...

"Aspires" seems like a weird word choice, consider replacing with
"adheres".


Section 4, Paragraph 1

> The RADIUS SAML binding defined by this binding Section 5 uses two 
> attributes to convey SAML assertions and protocol messages 
> respectively [OASIS.saml-core-2.0-os] .

This sentence does not compute and can be reworded to be more
meaningful.


Section 4, Paragraph 1

> The Length of both of these attributes is >=5.

Greater than or equal to 5 what?


Section 5.2, Paragraph 2

> Support for fragmentation over UDP is not mandatory.

Is support for fragmentation over UDP RECOMMENDED or OPTIONAL? May be
beneficial to be more explicit here on what this draft's opinion is.
If the draft doesn't care at all, then consider removing this sentence
entirely.


Section 5.3.2, Paragraph 2 and Paragraph 4

> The following methods are sufficient:
> 
> o  NAS identity in trusted digitally signed request.
> 
> o  NAS identity in trusted SAML federation metadata.

> The following methods are sufficient:
> 
> o  RADIUS realm in trusted digitally signed request.
> 
> o  RADIUS realm in trusted SAML federation metadata.

The wording here makes it unclear whether one of these alone is
sufficient or whether both are required. Rewording to say something
like "Satisfying the following two conditions is sufficient:" is more
clear.


Section 5.3.3, Paragraph 1

> This bindings calls...

Correct to "This binding calls..."


Section 7, Paragraph 1

> The Relying Party, acting as a NAS, attempts to validate the 
> Principal's credentials against a RADIUS server acting the 
> Principal's Identity Provider.

Correct to "...Principal's credentials against a RADIUS server acting
as the Principal's Identity Provider."


Section 7.3.3, Paragraph 1

> If the ForceAuthn attribute on the <samlp:AuthnRequest> element
> (if sent by the requester) is present and true, the Identity
> Provider MUST freshly establish this identity rather than relying
> on any existing session state it may have with the Principal (for
> example, TLS state that may be used for session resumption).

Consider breaking this sentence up more and explaining why the
Identity Provider must freshly establish the identity.


Section 7.3.4, Paragraph 1

> The Identity Provider MUST conclude the authentication in a manner 
> consistent with the RADIUS authentication result, and MAY issue a 
> <samlp:Response> message to the Relying Party consisent with the 
> authentication result and as described in [OASIS.saml-core-2.0-os] 
> and delivered to the Relying Party using the SAML RADIUS binding.

Break this sentence up and fix the typo in "consistent".

Something like, "The Identity Provider MUST conclude the
authentication in a manner consistent with the RADIUS authentication
result. The Identity Provider MAY issue a <samlp:Response> message to
the Relying Party that is consistent with the authentication result,
as described in [OASIS.saml-core-2.0-os]."

(The last part of this sentence doesn't make sense to me, so reword
that as necessary).


Section 7.3.5, Paragraph 1

> Any subsequent use of the <saml:Assertion> elements is at the 
> discretion of the Relying Party, subject to any restrictions on
> use contained within the assertions themselves or previously 
> established out-of-band policy governing interactions between the 
> Identity Provider and the Relying Party.

Reword to something like: "Any subsequent use of the <saml:Assertion>
elements is at the discretion of the Relying Party, subject to any
restrictions contained within the assertions themselves or from any
previously established out-of-band policy that governs the interaction
between the Identity Provider and the Relying Party."


Section 7.4.2, Paragraph 2

> ... the <samlp:Response> element MUST conform to the following:
> 
> o  It MAY be signed.

Extremely minor, but I found it a little distracting/funny that the
first bullet in a list of MUSTs is a MAY. My recommendation is to
reword the introduction to this: "... the <samlp:Response> element is
subject to the following conditions."


Section 7.4.3, Bullet 3

> If a <saml:AuthnStatement> used to establish a security context for
> the Principal contains a SessionNotOnOrAfter attribute, the 
> security context SHOULD be discarded once this time is reached, 
> unless the service provider reestablishes the Principal's identity 
> by repeating the use of this profile.

Why is this a SHOULD and not a MUST? If it is a SHOULD merely to allow
for the situation described above (where the service provider
reestablishes the identity), then it actually should be a MUST. "The
security context MUST be discarded unless the service provider
reestablishes the Principal's identity" is a perfectly sound requirement.

- -- 
Troy Shields
Developer of Awesome | Jive Communications, Inc.
Jive.com | [e-mail]


-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.22 (Darwin)
Comment: GPGTools - https://gpgtools.org
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBCgAGBQJT6thtAAoJEPpHBvvPaXlQlToQAJatdEwvYJUFeaHp2/Es5cj8
WdKgqhBkgyBkVSHw4yezgoWJsWsduln8943zglxPnohg6N/ID/Z/IavBIakcbbju
PdtdxNUWl/NmV8TUQ2FRgublzUKfOkPaD+++ajCplx+lZmnti4rimCZhBAhd52iT
gzgWdjFVowCqzFNoffCJvm2sx2FHvlOsOT41mq/7NRZT6f8I4T1+Rv91ZrI83ERo
Pt7471Y+UcoqliQqNEs5KFZEad0Z8TqxDFiJWGuiwNIXzISrLsZmulGo0l/x33Zc
LUso8rAdDq4THRcrQCshI7qvcTyQZVap1FskuasoKvBERdk/gZmxve11JZF27u0S
JiuQPOA3Jt7ymyYqUCBI7kteSr5u17n/4xuxhwfE+cNsUl0oQd+k8pH86ZUk2QUX
EAcJVgbUiYyCEWVATzXBcoIjUlF/xtQ4eexfjMc7Ilco1t0PvU7HdHEjzuMweKH2
huYkYZXaVebqCNVWxaatMI4TvMaJ6sbdThI0BpPvO2z3QektRCxmDC8TRv5zz4Y7
RWBzaqe3XiYAOUP75ibcinbLI5pntUpfIb45pOmtgkoCpvoFHmUiRjedEiFq3JVW
bx+u1y2FDs7byciIpGeBOGK9Qg+HCPuXy5VDYxNRgoFaYiFNNl5CF2j1TU/UkF58
Yt8Bnp9oG7dopBGZkG1e
=ony5
-----END PGP SIGNATURE-----