Review of draft-ietf-radext-radsec

[Bernard Aboba]

This is a review of "TLS Encryption for RADIUS" draft-ietf-radext-radsec. 

Overall, this draft was a pleasant to read, and it is clear that a lot of 
thought as well as implementation experience has gone into it. 

Kudos to the authors. 

General Issues

There is a considerable amount of text relating to dynamic discovery in this 
document, yet
there is only an informational reference to it. 

Since inserting a normative reference to dynamic discovery could delay the 
publication of
this document unnecessarily, my recommendation is to consolidate some of the 
dynamic
discovery material into a single section in which you can discuss the 
implications, while
clearly indicating the status of the dynamic discovery work (e.g. still under 
development, optional to
implement along with RADSEC, etc.). 

For example, you might consider consolidating the following text from Sections 
3.1 and 6 
and placing it prior to the current Section 3.1:

Section 3.X:  Implications of Dynamic Peer Discovery


   One mechanism to discover RADIUS over TLS peers dynamically via DNS 

   is specified in [I-D.ietf-radext-dynamic-discovery].  While this mechanism

   is still under development and therefore is not a normative dependency of

   RADIUS/TLS, the use of dynamic discovery has potential future implications 
that are

   important to understand. 

   If dynamic peer discovery as per
   [I-D.ietf-radext-dynamic-discovery] is used, peer authentication
   alone is not sufficient; the peer must also be authorised to perform
   user authentications.  In these cases, the trust fabric cannot depend
   on peer authentication methods like DNSSEC to identify RADIUS/TLS
   nodes.  The nodes also need to be properly authorised.  Typically,
   this can be achieved by adding appropriate authorisation fields into
   a X.509 certificate.  Such fields include SRV authority [RFC4985],
   subjectAltNames, or a defined list of certificate fingerprints.
   Operators of a RADIUS/TLS infrastructure should define their own
   authorisation trust model and apply this model to the certificates.
   The checks enumerated in Section 2.3 provide sufficient flexibility
   for the implementation of authorisation trust models.

[BA] I think you need to be more prescriptive here.  Are there specific
fields that a RADSEC TLS certificate should contain?  Having individual
implementations/deployments defining their own authorization schemes seems
like a bad idea.  

   In the case of dynamic peer discovery as per
   [I-D.ietf-radext-dynamic-discovery], a RADIUS/TLS node needs to be
   able to accept connections from a large, not previously known, group
   of hosts, possibly the whole internet.  In this case, the server's
   RADIUS/TLS port can not be protected from unauthorised connection
   attempts with measures on the network layer, i.e. access lists and
   firewalls.  This opens more attack vectors for Distributed Denial of
   Service attacks, just like any other service that is supposed to
   serve arbitrary clients (like for example web servers).

   In the case of dynamic peer discovery as per
   [I-D.ietf-radext-dynamic-discovery], X.509 certificates are the only
   proof of authorisation for a connecting RADIUS/TLS nodes.  Special
   care needs to be taken that certificates get verified properly
   according to the chosen trust model (particularly: consulting CRLs,
   checking critical extensions, checking subjectAltNames etc.) to
   prevent unauthorised connections.

Other comments

Section 1

   One mechanism to discover RADIUS over TLS peers with DNS is specified in
   [I-D.ietf-radext-dynamic-discovery].

[BA] Recommend moving this to a section devoted to dynamic discovery. 

Section 2.1

   See
   section Section 3.3 (4) and (5) for considerations regarding
   separation of authentication, accounting and dynauth traffic.

[BA] Recommend changing to:

   "See Section 3.3 for considerations regarding separation of
authentication, accounting and dynamic authorisation traffic."

Section 2.3

   4.  start exchanging RADIUS datagrams.  Note Section 3.3 (1) ).  The
   shared secret to compute the (obsolete) MD5 integrity checks and
   attribute encryption MUST be "radsec" (see Section 3.3 (2) ).

Section 3.1

   (3) If dynamic peer discovery as per
   [I-D.ietf-radext-dynamic-discovery] is used, peer authentication
   alone is not sufficient; the peer must also be authorised to perform
   user authentications.  In these cases, the trust fabric cannot depend
   on peer authentication methods like DNSSEC to identify RADIUS/TLS
   nodes.  The nodes also need to be properly authorised.  Typically,
   this can be achieved by adding appropriate authorisation fields into
   a X.509 certificate.  Such fields include SRV authority [RFC4985],
   subjectAltNames, or a defined list of certificate fingerprints.
   Operators of a RADIUS/TLS infrastructure should define their own
   authorisation trust model and apply this model to the certificates.
   T

Re: Protocol Definition

[Joe Touch]

On 1/3/2012 5:53 PM, Kaushal Shriyan wrote:

Hi,

Can someone please explain me the term "Protocol". Does it also mean it
has some software code underlying beneath it. Please help me understand.


My 2 cents:

A protocol is a defined set of rules that function at multiple locations 
(communication endpoints) that enable shared state (communication), and 
is composed of:


a defined set of messages ("on the wire" part)
a defined set of states (localized endpoint state)
a set of rules that relate message arrival, message departure,
time (optionally), and state transition

It's a lot like computation where the input is the received message 
sequence and the output is the transmitted message sequence.


A protocol is to communications as an algorithm is to computation.

Code can be used to implement an algorithm.

Code can be used to implement a protocol on an endpoint.

I.e., a protocol is just a special kind of algorithm - one that is used 
to support communication.


A running algorithm is called a process or thread (typically). Since a 
protocol *is* an algorithm, a running protocol is just called a 
communicating process or thread, IMO.


A "session" or "connection" defines an association between multiple 
parties running the same protocol and thus communicating.


Joe
___
Ietf mailing list
Ietf@ietf.org
https://www.ietf.org/mailman/listinfo/ietf

Re: Gen-ART Telechat Review of draft-ietf-kitten-sasl-saml-08

[Klaas Wierenga]

Hi Ben,

On Jan 13, 2012, at 11:00 PM, Ben Campbell wrote:

> 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 wait for direction from your document shepherd
> or AD before posting a new version of the draft.
> 
> Document: draft-ietf-kitten-sasl-saml-08
> Reviewer: Ben Campbell
> Review Date: 2012-01-13
> IESG Telechat date: 2012-01-19
> 
> Summary: This draft is almost ready for publication as a proposed standard. 
> There are a few minor issues that should be considered first.
> 
> Note: This is incremental to my review of version 06 at last call. Version 08 
> is considerably improved and resolved most of my comments. But a few still 
> remain. Quoted text below is from that previous review.

Yes, should have informed you about those changes before. I was going to wait 
until I had also incorporated Simon's additions, but I realize that may have 
been confusing.

> 
> Major issues:
> 
> None
> 
> 
> Minor issues:
> 
>> -- section 3.2, last paragraph:  "… needs to correlate the TCP session from 
>> the SASL client with the SAML authentication."
>> 
>> Please elaborate on this correlation
> 
> The author added text, but the new text is about correlating response with 
> request. The text I mentioned was about correlating a TCP connection to a 
> SAML authentication.

ah ok, but the intention of the text really is to talk about correlating the 
session that the SASL client maintains with the SASL server and the SAML 
session that the SAML client has with the SAML server. Would you be ok with 
changing the wording to that extent? So:

"Also, the SASL server needs to correlate the
   session it has with the SASL client and the SAML authentication by
   comparing the ID of the SAML authentication request it has issued with the 
one it receives in the SAML authentication statement"

> 
>> -- section 4, 3rd and 4th paragraph (paragraph a and b)
>> 
>> These seem like protocol affecting differences. If so, they need 
>> elaboration, such as normative statements and formal definitions, or 
>> references to such.
> 
> I did not see a response to this comment.

see Simon's comments

> 
>> -- section 5, general:
>> 
>> The section seems to need further elaboration or references
> 
> I did not see a response to this comment.
> 

idem

>> Also, this section compresses the interaction with the identity provider. 
>> Why not show the details for those steps like the others? (If you mean them 
>> to be out of scope, then why give as much detail as you do?)
>> 
> 
> I did not see a response to this comment.

I want to give enough details for implementers to understand the full flow, 
even though those steps are out-of-scope for this specification. I thought the 
[ ] brackets would convey that, do you think it is clearer to leave that part 
out altogether?

> 
> Nits/editorial comments:
> 
>> -- Pagination is strange throughout the document. (Mostly blank pages, etc.) 
>> It's worse in the PDF version, but still not right in the text version.
> 
> Pagination is still strange. I see a few mostly blank pages, diagrams split 
> across page breaks, etc.

hmm, strange, I removed some empty lines and in my version it nicely broke on 
session headings, but I'll double check all generated versions next iteration. 

> 
>> -- section 3, 1st paragraph: "Recall section 5 of [RFC4422] for what needs 
>> to be described here."
>> 
>> That reads like an author's "to do" note to himself. Has the needed 
>> description been completed, or does it still need to be described?
> 
> Partially fixed. I suggest s/"needs to be"/"is"

ok, will do

> 
>> -- section 3.1, first paragraph:
>> 
>> Is "authorization identifier" the same as "IdP-Identifier"?
> 
> The paragraph has been updated, but I still have the question. 

It is not, I will add text on that

> 
>> -- section 3.3, 2nd paragraph:  "and SHALL be used to set state in the 
>> server accordingly, and it shall be used by the server"
>> 
>> Is this new normative language, or a repeat of language from the SAML spec?
>> 
> 
> The author changed SHALL to MUST, which leads me to believe my comment was 
> not clear. I did not object to SHALL. My concern was, with the reference to 
> RFC4422, it was not clear if the text was introduction a new normative 
> requirement, or just restating requirements from 4422. If the second, then 
> it's important to make sure the reader knows which doc is authoritative. You 
> can do that by keeping the language descriptive, or by explicitly (and 
> strongly) attributing the language with something like 'RFC4433 says, "…. 
> SHALL…."'
> 
> If, on the other hand, this is truly a new normative statement, then no 
> change is needed.

right, now I get it. It is indeed intended in the 4422 sense, so I will take 
your suggestion

> 
>> -- section 4, 1st paragraph:
>> 
>> I have difficulty parsing this.
> 
> The text is updated, but I still hav

Re: Last Call: (A URN Namespace For The ucode) to Informational RFC

[Julian Reschke]

On 2012-01-17 19:27, Mykyta Yevstifeyev wrote:

Hello,

Having reviewed this document, I think there is no problem with its
publication.  Several tiny comments:

1) "RFID" in the Introduction needs expanding at first use.

2)   ucode-value = 32hex-decimal
hex-decimal = "0" / "1" / "2" / "3" / "4" / "5" / "6" / "7" /
  "8" / "9" / "A" / "B" / "C" / "D" / "E" / "F"

may be changed to:

ucode-value = 32HEXDIG

and that's RFC 5234 which defines  so you may refer to it later.

3)

Rules for lexical equivalence:

The entire UCODE-URN is case-sensitive.

This is at least wrong for the first three chars in URN (the URI
scheme name "urn"), that is case-insensitive as per RFC 3986.  What's
the reason for setting such regulation?
...


What's case-sensitive depends on context; for instance, when comparing 
XML namespace URIs or Atom IDs, everything is case-sensitive. See also 
.


Best regards, Julian
___
Ietf mailing list
Ietf@ietf.org
https://www.ietf.org/mailman/listinfo/ietf

Re: Gen-ART Telechat Review of draft-ietf-kitten-sasl-saml-08

[Simon Josefsson]

Ben Campbell  writes:

>> -- section 7 
>> 
>> Does the GSS-API description introduce security considerations? If
>> not, please say so.
>> 
>
> I did not see a response to this comment.

I missed this in my last e-mail.  I propose we add another sub-section
of the security considerations like this:

7.5. GSS-API specific security considerations

   Security issues inherent in GSS-API (RFC 2743) and GS2 (RFC 5801)
   apply to the SAML GSS-API mechanism defined in this document.
   Further, and as discussed in section 4, proper TLS server identity
   verification is critical to the security of the mechanism.

I believe this should cover the relevant security considerations.  Of
course, having more implementation experience with the SAML mechanism
used as a GSS-API mechanism may help to identify further security
considerations for the GSS-API mechanism.  However, I don't believe that
is a show-stopper that prevent publication now.

/Simon
___
Ietf mailing list
Ietf@ietf.org
https://www.ietf.org/mailman/listinfo/ietf

Re: Gen-ART LC Review of draft-ietf-kitten-sasl-saml-06

[Simon Josefsson]

Ben Campbell  writes:

> -- section 4, 3rd and 4th paragraph (paragraph a and b)
>
> These seem like protocol affecting differences. If so, they need
> elaboration, such as normative statements and formal definitions, or
> references to such.
>
> -- section 5, general:
>
> The section seems to need further elaboration or references

Hello Ben.  Thank you for your review.  Klaas pointed me at this part
and I will try to work this out with you.

Re section 4 I believe your comment is based on a misunderstanding.  Let
me try to explain what the intention is, and we can work out how to fix
the text if needed.  What is described in paragraph a) and b) are the
protocol requirements that (implicitely) follow from GS2.  There is
nothing particular to this protocol in there.  Let's take a step back:

The purpose of GS2 is to map any GSS-API mechanism into a SASL
mechanism.  However, SASL-SAML is defined as a SASL mechanism (because
SASL implementers wants it that way).  The point of section 4 is to turn
this SASL mechanism into a GSS-API mechanism that, after the SAML
GSS-API mechanism is used via GS2, becomes identical to the SAML
mechanism defined in the rest of the SASL-SAML document.  This is a bit
convuleted to describe, but this is the gist of it.

(It would have been simpler to specify a SAML GSS-API mechanism and then
let GS2 convert it to SASL automatically, but some SASL people doesn't
want anything to do with GSS-API so this is a compromise to allow them
to implement SAML-SASL without touching GSS-API.)

I'm thinking that perhaps the above explanation would be useful to have
in the document, to give some background.  If you agree, I propose to
resolve your section 4 comment by making this change:

OLD:
  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

NEW:
  This section specify a GSS-API mechanism that when used via the GS2
  bridge to SASL behave like the SASL mechanism defined in this
  document.  Thus, it can loosely be said that the SAML SASL mechanism
  is also a GSS-API mechanism.

  The SAML user takes the role of the GSS-API Initiator and the SAML

Does this resolve your concern re section 4?

Re your section 5 comment, I tend to agree.  The section is quite brief
because it was ripped out of section 3.1.  I propose that we simply
collapse this section back into 3.1 where it makes more sense.  Thus:

OLD:
  - See Section 5 for the channel binding "gs2-cb-flag" field.

NEW:
  - The "gs2-cb-flag" MUST be set to "n" because channel binding
data cannot be integrity protected by the SAML negotiation.
(Note: In theory channel binding data could be inserted in the
SAML flow by the client and verified by the server, but that is
currently not supported in SAML.)

And then remove section 5 completely.  Section 3 and in particular
section 3.1 already contains the relevant references and provides the
context where the statements make sense.

What do you think?

Thanks,
/Simon
___
Ietf mailing list
Ietf@ietf.org
https://www.ietf.org/mailman/listinfo/ietf