Subject: [Gen-art] Gen-ART LC review of draft-ietf-abfab-aaa-saml-12

From: "Roni Even" <[email-masked]>
Date: Thu, 3 Dec 2015 16:31:10 +0200

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

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

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)

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?

Nits/editorial comments:

1.	In  section 1 please expand ABFAB
2.	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."

---

From: Alejandro Pérez Méndez <[email-masked]>
Date: Thu, 10 Dec 2015 08:32:02 +0100

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ó:
> Please resolve these comments along with any other Last Call comments 
> you may receive.
>
> Review Date:2015–12-3
> IETF LC End Date: 2015–12-4
>
> IESG Telechat date:
>
> 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.

>  1. In section 1 please expand ABFAB

Ok

> 1.
>
>
>  2. 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

---

From: Stephen Farrell <[email-masked]>
Date: Thu, 10 Dec 2015 12:59:08 +0000

Hiya,

> radsec. Can it be done at this moment or does it require a more complex

Hmm. The shepherd write-up says informational is correct. If the WG
chairs want to, we can re-spin the IETF LC. But this has been so
long in the process and has slowly so I'd prefer to not do that
unless someone really cares, and it makes a difference.

For now, I've kept this on the Dec17 IESG telechat as informational
but if needed we can push it into the new year.

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

I'll add the above as an RFC editor note.

Cheers,
S.

---

From: Scott Bradner <[email-masked]>
Date: Thu, 10 Dec 2015 08:02:00 -0500

> but if needed we can push it into the new year.

fwiw - 2026 requeres a new LC if there is to be a increase in the status (info to studs track) 

Scott

---

From: "Klaas Wierenga (kwiereng)" <[email-masked]>
Date: Fri, 11 Dec 2015 16:16:34 +0000

> On 10 Dec 2015, at 14:02, Scott Bradner <[email-masked]> wrote:
> 
> 
>>> 

Yes, that is understood. I (document shepherd) actually consulted RFC2026 on this because I was in doubt, and since there is mentioning of "Specifications that have been prepared outside of the Internet
 community” and part of this spec is OASIS’ domain, I found informational defensible. As it turns out the change to informational was accidental. I’ll discuss with the authors.

Klaas

---

From: "Klaas Wierenga (kwiereng)" <[email-masked]>
Date: Fri, 11 Dec 2015 18:10:42 +0000

Hi Stephen,

> but if needed we can push it into the new year.

The authors unanimously prefer standards track, so I'm afraid a new last call is required.

Klaas

---

From: Stephen Farrell <[email-masked]>
Date: Fri, 11 Dec 2015 18:29:59 +0000

> The authors unanimously prefer standards track, so I'm afraid a new

Fair enough. Will you check quickly with the WG then I'll start that?

Ta,
S.

---

From: Sam Hartman <[email-masked]>
Date: Mon, 14 Dec 2015 05:04:46 -0500

>>>>> "Klaas" == Klaas Wierenga (kwiereng) <[email-masked]> writes:

It's also important that the new LC make a note about the normative
down-ref to RADIUS over TLS.

---

From: Stephen Farrell <[email-masked]>
Date: Mon, 14 Dec 2015 10:50:04 +0000

> down-ref to RADIUS over TLS.

Good catch - thanks! ID-nits (which I'd not re-run until you reminded
me;-) didn't object to it funnily enough but I've added it to the LC
text anyway.

S.