[abfab] Comments on draft-ietf-abfab-aaa-saml-10

["Jim Schaad" <[e-mail]>]

I really want to see this document done as it is now the document holding up
the ABFAB cluster of documents in the RFC editor queue.

1.  Section 1 - Paragraph 3 - There is no longer a document
I-D.jones-diameter-abfab and there is currently no prospect of such a
document coming back into existence anytime soon.  The paragraph and the
reference should be killed.

2.  Section 4.2 - Please change the MAY back to a can 'this profile can
take'  There is no real protocol statement here.  If you wanted to make one
then it should be a lot higher than MAY as the odds are that SAML statements
are not going to be transportable in a large number of cases without either
this or the big packet size draft being implemented.

3.  Given the way that messages and assertions are being separated in
section 4.2, I think that we should consider renaming SAML-Message to be
SAML-Protocol.  This makes it clearer that protocol messages go here and
assertions go in SAML-Assertion.   It is a bit confusing to talk about SAML
protocol messages, SAML-Message and RADIUS Access-Request message in
proximity.  Making both of the SAML items be protocol I think would clarify
things.

4. Section 4.2 - s/Therefore, other arbitrary RADIUS attributes MAY be used
in either the request or response./Therefore, other arbitrary RADIUS
attributes will be used in requests and responses./

5. Section 4.3.2 - Last three paragraphs - If the SAML Issuer entityID is of
the form of an NAI and the realms match between the IdP and the SAML NAI,
are these statements still true or can the SAML policy still be applied or
is that just a match for what happens in section 4.3.1?

6.  Section 4.3.3 - Can we really say that XML signatures are outside of the
scope of the binding given that section 4.3.2 is saying that there are cases
where signatures are going to be needed?  I think we can just delete
everything following ", but this".

7.  Section 5 - I would like to add the following paragraph:

There are cases where the RP does not have a full NAI name or the IdP will
not release a full NAI name to the RP due to policy.  In these cases, the
domain only form of the NAI can be used.

8.  Section 6 - I am having a problem with the overview in this section.  It
is quite clear from the overview that this is designed to be a
query/response protocol.  It is not clear from the overview that any such
thing as an IdP spontaneously returning a SAML statement or responding with
a SAML statement that has nothing to do with the query is part of the
profile.  I will be honest, I would prefer that the binding be exactly what
is stated in the overview.  If the binding is to be adhered to, then the bad
behavior should not be permitted.

9.  Should section 6.1 Conformation Method identifiers also refer to the
ones in section 8?  I think so

10.  Section 6.2 - If one is using this binding, is it really optional to
issue the <samlp:AuthnRequest> message in step 2?  It would seem that if you
are using this binding one is going to do the SAML stuff.   If is not doing
the SAML stuff then one is not using this binding.

11.  Section 6.2 - Bindings - s/not require the use of/not use/  The use of
require seems to me to imply there are cases where HTTP based bindings could
be used.

12. Section 6.2 step 2 - s/may optionally issue/issues/

13.  Section 6.3.2  - s/MAY include/includes/  
        s/this RADIUS/the RADIUS/
        s/SAML RADIUS binding/SAML-Message RADIUS attribute/
	s/destination MAY be/destination may be/

14.  Section 6.3.3 - What is "RADIUS authentication"?  We have said that you
need to do EAP for ABFAB however no restrictions for non-ABFAB methods.  Do
we consider any RADIUS authentication method to be sufficient?

13.  Section 6.3.4 - I would prefer that this read
The IdP MUST issue a <samlp:Response> message to the RP that is consistent
with the authentication results, as described in [OASIS.saml-core-2.0-os].
This SAML response is delivered to the
   Relying Party using the SAML RADIUS binding described in Section 4.  If a
response message is not returned by the IdP, the RP MUST NOT assume that the
authentication was performed in a manner consistent with the request.

14.  Section 6.4.2 -  If the IdP cannot or will not satisfy the request, it
MUST either respond with a <samlp:Response> message containing an
appropriate error status code or codes and/or respond with a RADIUS access
denied message.

15.  Section 6.4.2 - Is the last bullet item normal for a SAML provider?
Specifically, that one can authenticate, but ignore any of the actual
requests from the RP.  If not, should we make some statements about telling
the RP that we did not follow the instructions?  Possibly by either an error
or no response (if the IdP does not support the binding).

16.  Section 6.4.4 - I think that this should become a completely separate
binding.   Let's make the spontaneous stuff be a single separate binding. 

17.  Section 7.3.1 - Bullet #1 - I am having problems with this because it
would potentially kill the machine confirmation type as a request.   I am
not even sure what it means for the identifiers to be different, you might
want to discuss that instead.   Do you really mean to say that these names
need to be, in some way, consistent instead?

18.  Neither in section 6 or section 7 is it made clear which RADIUS
attribute is to be used in sending the request and the response.  This
should be made explicit even if we don't create a new binding for the
spontaneous sending of SAML from the IdP to the RP.

19.  These are not RADIUS State confirmation methods - they are SAML Subject
Confirmation methods.  As it currently stands, it is not clear where these
go.

20.  I want to sit down with the authors in Dallas and talk about the change
in names that was made for revision -10.   I think that many of these
changes are potentially harmful for clarity of the document.

Jim