Re: Gen-ART LC review of draft-ietf-abfab-aaa-saml-12

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dear Roni and chairs of the ABFAB WG,

thank you for the revision. Please, see my responses inline (specially the one related to point #2)

El 03/12/15 a las 15:31, Roni Even escribió:

I am the assigned Gen-ART reviewer for this draft. For background on Gen-ART, please see the FAQ at <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Please resolve these comments along with any other Last Call comments you may receive.

Document:  draft-ietf-abfab-aaa-saml-12

Reviewer: Roni Even

Review Date:2015–12-3

IETF LC End Date: 2015–12-4

IESG Telechat date:

 

Summary: This draft is almost ready for publication as an Informational RFC.

 

 

 

Major issues:

 

 

Minor issues:

1.       Why is the RADIUSNasIpAddress a string and not as specified in for example in RFC2865


The RADIUSNasIpAddress is a SAML metadata element, thus it has to comply with existing SAML types. The string type allows to encode the "display" value of these RADIUS attributes (e.g. "192.168.1.1", or "::1"). Note that current text specifies that the element contains an acceptable value for RADIUS NAS-IP-Address or RADIUS NAS-IPv6-Address attributes, so no arbitrary values are accepted nonetheless.

2.       In general I was wondering why this is an Informational document. It defines procedures and has normative language.


That sounds like kind of an unfortunate bug. For some reason, it changed from Standards Track to Informational between versions -00 and -01.
However, we want it standards-track with a normative downreference to radsec. Can it be done at this moment or does it require a more complex process?

3.       In the IANA consideration in section 11.1, as far as I understand the IANA attribute type registry you need to ask for values for TBD1 and TBD2 from the unassigned space (and not the reserved space)


I agree. I cannot find where we state otherwise, though. Could you point the specific text where we say it?

4.       In step 2 of figure 7 (section 7.2) the text says “In step 2, the Relying Party may optionally issue a <samlp:AuthnRequest> message to be delivered to the   Identity Provider using the SAML-Protocol RADIUS attribute.”  My reading is that the rest of the steps are when this message is sent, since it is  “may” what happens if the message is not sent?

 

 


If the <AuthnRequest> is not sent, the procedure follows the "unsolicited response" (explained in 7.4.4), where the IdP deliveres a <samlp:Assertion> element. I agree that step 4 needs to include this clarification. The new text should read as:


    4.  Identity Provider issues <samlp:Response> to Relying Party
       (Section 7.3.4).  In step 4, the Identity Provider issues a
       <samlp:Response> message to the Relying Party using the SAML
       RADIUS binding.  The response either indicates an error or
       includes a SAML Authentication Statement in exactly one SAML
       Assertion. If the RP did not send an <samlp:AuthnRequest>, the IdP

       issues an unsolicited <samlp:Assertion>, as described in section 7.4.4.


 

Nits/editorial comments:

  1. In  section 1 please expand ABFAB

Ok


  1. In section 7.2, the text says “To implement this scenario, a profile of the SAML Authentication   Request protocol is used in conjunction with the SAML RADIUS binding  defined in Section 4.” I think that the language should be more normative maybe it should say  “To implement this scenario, this profile of the SAML Authentication   Request protocol MUST Be (or SHOULD if there are other options) used in conjunction with the SAML RADIUS binding  defined in Section 4.”

Agree. I think "MUST be" is the one to be used.

Best regards,
Alejandro

[Index of Archives]     [IETF Annoucements]     [IETF]     [IP Storage]     [Yosemite News]     [Linux SCTP]     [Linux Newbies]     [Fedora Users]