Subject: Re: [kitten] AD review of draft-ietf-kitten-sasl-saml

From: Stephen Farrell <[email-masked]>
Date: Thu, 15 Dec 2011 12:52:10 +0000

Folks,

Any response to those questions of mine? This has
been sitting a while...

Or, have I missed the response and dropped the ball?

Cheers,
S.

On 11/23/2011 12:37 AM, Stephen Farrell wrote:
>
> Hi,
>
> I've reviewed this and its ready or nearly ready for
> IETF LC - I've a few questions though so I'm not sure.
> (That's the first, numbered group in the attached.)
>
> Let's see if there's changes needed or not and then
> proceed. (IESG comments on the openid spec might also
> be worth a look with respect to this when we get them
> next week.)
>
> Cheers,
> S.
>
>
> _______________________________________________
> Kitten mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/kitten

---

From: Klaas Wierenga <[email-masked]>
Date: Thu, 15 Dec 2011 16:23:22 +0100

Stephen,

No, nothing dropped on your side. The action is on me, I was on holidays and will respond before Xmas.

Klaas

Sent from my iPad

---

From: Klaas Wierenga <[email-masked]>
Date: Mon, 19 Dec 2011 11:14:11 +0100

On Nov 23, 2011, at 1:37 AM, Stephen Farrell wrote:

Hi Stephen,

> (That's the first, numbered group in the attached.)

Thanks for your detailed review! 

> be worth a look with respect to this when we get them

questions:
----------

> #1, 1.2 - what does "or similar integrity protected and
> authenticated channels" mean? If you mean IPsec, then saying that
> seems right.  The problem though is that people do say things like
> this when they mean "use it in clear on the intranet" which is not
> desirable here presumably. (I realise now I let this pass for the
> openid spec but we could still fix that too;-) I'm sure we can come
> up with a better wording that captures the WG's desired meaning but
> doesn't leave open the option to just run this in clear.

Hmm, IPsec is indeed the most obvious alternative, but I didn't want to enumerate, acknowledging that is conceivable that other means to provide integrity and authentication can be developed or used. I would argue that an Intranet is NOT an "integrity protected and
authenticated channel". How about just calling that out:

"Because this mechanism transports information that
   should not be controlled by an attacker, the SAML mechanism MUST only
   be used over channels protected by TLS, and the client MUST
   successfully validate the server certificate, or similar integrity
   protected and authenticated channels, like through the use of IPsec.  [
RFC5280][RFC6125]

note: An Intranet does not constitute such an integrity protected and authenticated channel"

> #2, s2, what does "sends a domain" mean near the end of p5?

ehm, I assume you mean end of p4. It's a domain name, the assumption is that one RP can serve multiple IdPs, the domain name is used by the RP to construct the appropriate SAML request. In earlier versions we used the SAML entity-id (a URL or URN) but given that it is likely that an end-user may need to enter this into their client WG consensus was that we needed something simpler, hence the domain name, assuming that it is easy to either construct the request from that or do a simple lookup in table that holds domain to entity-id mapping. See also 3.2

> #3, s3.2 says that the SASL server "transmits" stuff to the IdP,
> but there's no line for that in figure 2 - what's up there? Is it
> sent via the client really? Be less confusing to say so, if so.  If
> not, then I think something needs to change.

ack, this line is multi interpretable, it is supposed to say that a redirect uri (to the IdP) is sent to the client, not that something is transmitted to the IdP. Proposed change:

"The SASL Server transmits to the SASL client a redirect URI to the IdP (corresponding
   to the domain the user provided), with a SAML authentication request
   as one of the parameters."

> #4, s.3, says the client uses a "HTTP GET" - shouldn't that be
> HTTPS?

yes, or rather HTTP(S)

> #5, s3.2, the client "MUST handle" auth with the IdP - that seems
> ambiguous - how's I test for it? Maybe s/MUST handle/handles/ since
> that's really SAML, right?

yes, I agree, this is indeed outside the scope of this spec

> #6, s3.2, what does it mean to say the client "relays the response
> to the RP via HTTP(S)"? That makes the client sound like an HTTP
> proxy which, I think, misleading.

It is meant to indicate that the response is again transmitted by means of a redirect of the browser, how about:

"After all authentication has been completed by the IdP, the IdP will send a redirect message to the client in the form of an HTTP(S) URI corresponding to the Relying Party as specified  in the authentication request ("AssertionConsumerServiceURL") and with the SAML response as one of the parameters."

> #7 s4, the first para needs changes as were done with openid, e.g.
> NORMATIVE isn't 2119 language etc.

ack

> minor, can be handled alongside IETF LC, or earlier, or not at all:
> -------------------------------------------------------------------
> 
> general: you sometimes talk about the "SASL server" and other times
> use "Relying Party" or RP, I think it'd be good to say those are
> basically the same (or whatever is the case). That's nearly all
> there, at the end of 3.2, but I think it might be better to just
> say that those are the same thing near the front and then just use
> one of the terms all the time after that. Should be clearer for
> coders I'd hope.

yes, that makes sense
 
> s2, How is this about "non-HTTP Use Cases"? It seems much broader.
> Suggest renaming the section, maybe to "Authentication Flow." 

ack
 
> s2, "some sort of cookie" is a bit vague - can't you do better?

yes we can ;-)

> s2, Why "must" the RP "remain untouched"? That may be resonable but
> it doesn't seem like its a must - you could have chosen to modify
> the IdP if you'd liked. Maybe s/must remain/is better/?

ehm, (assuming s/RP/IdP/) I suggest s/must remain/can remain/ (as this is one of the distinguishing features from sasl-saml-ec)

> 
> s2, is 3986 really a useful reference for the URI sent at step 3 on
> p5? Isn't there some saml reference that'd be better?

hmm, yes that appears a bit generic, I'll dig up something better

> s3.1, it might be useful to label the abnf as such (xml2rfc can
> help with that a bit if you make it a figure with artwork of type
> abnf).

ack
 
> s3.x, it'd be good to tie the sections here back to figure 2, e.g.
> put stuff like "(message 2 in Figure 2)" in as parenthethic notes.

ack
 
> s6.1, is the encoding shown for steps 4, 5 and others obvious for
> implementers? Maybe it is, but I was surprised.

ehm, I'd like to think so ;-) What in particular surprises you? 

> 
> nits:
> -----
> 
> p3, s/We want to point out that the/The/
> p3, s/is optional/is OPTIONAL/
> p3, s/that uses/that use/
> p3, s/to a maximum extent/to the maximum extent/
> p3, s/The mechamsisms assumes/The mechanism assumes/
> p3, s/will continued to/will continue to/
> p4, s/Applicability Because/Because/
> p5, s/RP now has/The RP now has/

ack

Thanks again,

Klaas

---

From: Stephen Farrell <[email-masked]>
Date: Mon, 19 Dec 2011 13:19:51 +0000

Hi Klaas,

> Thanks for your detailed review!

 From the answers below I guess a revised I-D is going
to help here so I've marked it thusly in the tracker.
Let me know if that's wrong.

>> be worth a look with respect to this when we get them

I can live with that if the WG want to say it
that way.

> ehm, I assume you mean end of p4.

No, p5:

"   2.  The client initiates a SASL authentication with SAML20 and sends
        a domain"

 > It's a domain name, the assumption is that one RP can serve multiple 
IdPs, the domain name is used by the RP to construct the appropriate 
SAML request. In earlier versions we used the SAML entity-id (a URL or 
URN) but given that it is likely that an end-user may need to enter this 
into their client WG consensus was that we needed something simpler, 
hence the domain name, assuming that it is easy to either construct the 
request from that or do a simple lookup in table that holds domain to 
entity-id mapping. See also 3.2

So should it say "a domain name that allows the RP to contact the IdP"
or something like that?

>     as one of the parameters."

That helps. Maybe "redirect URI to the IdP" is a bit oddly
phrased though. Perhaps "a URI that (re)directs the SASL client to
the IdP" is better?

A couple of follow on questions arising from that. Is the client
expected to sanity check that URI in any way? What bad stuff
might happen if the client doesn't do any checks?

>> #4, s.3, says the client uses a "HTTP GET" - shouldn't that be

Or maybe "an HTTP GET (sent over a server-authenticated TLS
channel)" is what's really meant?

> yes, I agree, this is indeed outside the scope of this spec

ok

> "After all authentication has been completed by the IdP, the IdP will send a redirect message to the client in the form of an HTTP(S) URI corresponding to the Relying Party as specified  in the aut…

Better. But is "HTTP(S)" right here or "https" which the
right scheme?

>> NORMATIVE isn't 2119 language etc.

Ok, as are the rest except the last one.

> ehm, I'd like to think so ;-) What in particular surprises you?

I was surprised by the volume of base64 and the lack of detail
as to how its generated.

If you tell me its obvious to implementers then fine.
(Has someone coded this up from the spec who's not an author?)

Cheers,
S

---

From: Klaas Wierenga <[email-masked]>
Date: Mon, 19 Dec 2011 14:45:46 +0100

On Dec 19, 2011, at 2:19 PM, Stephen Farrell wrote:

Hi,

> Let me know if that's wrong.

No, I think that is right.

> > It's a domain name, the assumption is that one RP can serve multiple IdPs, the domain name is used by the RP to construct the appropriate SAML request. In earlier versions we used the SAML entity-id (a URL or URN) but given that it is likely that an end-user may need to enter this into their client WG consensus was that we needed something simpler, hence the domain name, assuming that it is easy to either construct the request from that or do a simple lookup in table that holds domain to entity-id mapping. See also 3.2
> 

Ehm, I would prefer to say "a domain name that allows the RP to determine the appropriate IdP"

> phrased though. Perhaps "a URI that (re)directs the SASL client to

a gladly accept your native speaker formulation

> channel)" is what's really meant?

Ehm, is there a difference, apart from the fact that you (rightfully) call out that the server needs to be authenticated? Or is this to make it more generic for all integrity protected channels with server authentication? Maybe I just miss your point.

> Better. But is "HTTP(S)" right here or "https" which the

I guess this goes to the previous question

> I was surprised by the volume of base64 and the lack of detail

ah ok, I am not particularly happy about the double base64 encoding myself, but this is how I understand the SASL and SAML specs

> (Has someone coded this up from the spec who's not an author?)

Well, I had a student implement from the spec, but admittedly, I was around to answer questions

Klaas

---

From: Stephen Farrell <[email-masked]>
Date: Mon, 19 Dec 2011 13:54:24 +0000

Hiya,

I guess the only point below is how to phrase
"HTTP(S)" throughout. I guess I'm not that
fond of "HTTP(S)" since to me it means "either
HTTP or HTTP/TLS" and that's not really what
you mean here. Or maybe I'm getting stuff wrong
again;-)

S

---

From: Klaas Wierenga <[email-masked]>
Date: Mon, 19 Dec 2011 15:18:36 +0100

On Dec 19, 2011, at 2:54 PM, Stephen Farrell wrote:

Hi,

> you mean here. Or maybe I'm getting stuff wrong

ah ok, let me try to rephrase 

Klaas

---

From: Nico Williams <[email-masked]>
Date: Mon, 19 Dec 2011 17:39:27 -0600

> authenticated channel". How about just calling that out:

Note that use of IPsec as an alternative to TLS is fraught with
dangers, and one must a) use IPsec channels (see RFC5660, which has
not yet been implemented near as I can tell), b) use non-existent APIs
by which to query the ID of the peer (the server, in this case).
Alternatively one can skip TLS and expect that IPsec is properly
configured, or one could use IPsec APIs (non-standard) by which to
configure IPsec such that a channel effectively exists with a strongly
authenticated peer.

It really is not worthwhile to attempt to write text for running SASL
applications over IPsec.  It is OK to say something about similar
channels, but don't bother with IPsec, IMO,

Nico
--

---

From: Klaas Wierenga <[email-masked]>
Date: Wed, 21 Dec 2011 14:47:17 +0100

Hi,

I have updated the draft to version 6 and resubmitted, I think I have addressed all your comments, with the exception of calling out IPSec as an alternative channel cf. Nico's arguments.
I believe all the changes are cosmetic, but encourage the WG to verify.

Klaas

---

From: Stephen Farrell <[email-masked]>
Date: Thu, 22 Dec 2011 14:56:24 +0000

Hi Klaas,

Those changes look good to me and I hope the WG.

I've asked for IETF LC to start, if there are any
WG comments they can be handled as part of that
I guess.

S.

On 12/21/2011 01:47 PM, Klaas Wierenga wrote:
>> On Mon, Dec 19, 2011 at 4:14 AM, Klaas Wierenga<[email-masked]>  wrote:

---

From: Klaas Wierenga <[email-masked]>
Date: Thu, 22 Dec 2011 16:03:26 +0100

On Dec 22, 2011, at 3:56 PM, Stephen Farrell wrote:

Hi Stephen,

> WG comments they can be handled as part of that

Thanks, and happy holidays!

Klaas

---

From: Alexey Melnikov <[email-masked]>
Date: Sun, 25 Dec 2011 13:50:24 +0000

Additional comments (mostly nits) on the latest version:

In Section 3.2:

Note: While the SASL client MAY sanity check the URI it received,
         ultimately it is the SAML IdP that will be validated by the SAML
         client out-of-scope for this document..

2 dots at the end. I also think "which is" is missing before "out-of-scope.

In Section 3.3:

  The SAML SASL mechanism is actually also a GSS-API mechanism.  The
  SAML user takes the role of the GSS-API Initiator and the SAML
  Relying Party takes the role of the GSS-API Acceptor.  The SAML
  Idenity Provider does not have a role in GSS-API, and is considered

typo: Identity

  an internal matter for the OpenID mechanism.The messages are the

Typo: SAML20

  same, but

It doesn't look like the reference to [RFC4648] needs to be Normative. 
In fact the current text is confusing. base64 encoding is only needed 
because you have an XMPP example and not for any other reason.

---

From: Alexey Melnikov <[email-masked]>
Date: Sun, 25 Dec 2011 13:58:20 +0000

> Additional comments (mostly nits) on the latest version:
One more thing:

  Step 6: Client sends a BASE64 encoded empty response to the
    challenge:

    <response xmlns='urn:ietf:params:xml:ns:xmpp-sasl'>
     =
    </response>

Is this actually correct, or is this how XMPP sends empty response?
Either way, I think the text should read:

    Client sends the empty response to the challenge encoded as a single =.

Because base64 certainly doesn't allow for that.

---

From: Alexey Melnikov <[email-masked]>
Date: Sun, 25 Dec 2011 14:01:09 +0000

One more thing:

  [RFC2818]  Rescorla, E., "HTTP Over TLS", RFC 2818, May 2000.

This is likely to be need to be a Normative Reference. It is Ok that it 
is Informational, it is already in the Downref registry.