Subject: [secdir] Secdir review of draft-ietf-oauth-saml2-bearer-21

From: Vincent Roca <[email-masked]>
Date: Tue, 14 Oct 2014 17:10:59 +0200

Hello,

I have reviewed this document as part of the security directorate's
ongoing effort to review all IETF documents being processed by the
IESG.  These comments were written primarily for the benefit of the
security area directors. Document editors and WG chairs should treat
these comments just like any other last call comments.

IMHO, the document is almost ready. I just have minor comments:

SAML and OAUTH are already covered by extensive, detailed security and privacy 
considerations sections.

I agree with the authors there is no need to duplicate text in the present document.
However I have two comments:

1- it is mentioned that replay attack protection is not mandatory, but there is no
justification. On the opposite, protection against replay attacks is mentioned at
several places in [OASIS.saml-sec-consider-2.0-os] (e.g., 6.1.2, 6.4.5, 6.5.2, 6.5.6,
7.1.1.4). I don’t know to what extent the situation differs, but I’m curious to know
why it is so.

2- [OASIS.saml-sec-consider-2.0-os] reference does not include any URL. It’s probably
worth to add it.
	http://docs.oasis-open.org/security/saml/v2.0/saml-sec-consider-2.0-os.pdf

Cheers,

  Vincent

---

From: Brian Campbell <[email-masked]>
Date: Wed, 15 Oct 2014 07:43:45 -0600

Thanks for your review and feedback, Vincent.  I'm adding the working group
to the thread so they’re aware of the comments. Replies are inline below...

From: Vincent Roca <[email-masked]>
> Date: Tue, Oct 14, 2014 at 9:10 AM
> Subject: Secdir review of draft-ietf-oauth-saml2-bearer-21
> To: IESG <[email-masked]>, [email-masked],
> [email-masked]
> Cc: Vincent Roca <[email-masked]>
>
>
> Hello,
>
> IMHO, the document is *almost ready*. I just have minor comments:
>

In the SAML 2.0 Profile for OAuth 2.0 Client Authentication and
Authorization Grants the client is not passive (like a web browser in the
case of many SAML profiles/bindings) but rather an active client which
either creates the assertion itself or obtains it from a trusted 3rd party
system like a security token service. In that sense, it's most analogous to
6.1.2 of OASIS.saml-sec-consider-2.0-os which discusses replay in the
context of the SOAP binding and says there "is little vulnerability to
replay attacks" and that "the best way to prevent replay attacks is to
prevent the message capture in the first place" going on to say that TLS
does this for point to point communication.

The general Assertion Framework for OAuth 2.0 Client Authentication and
Authorization Grants, which the SAML document is a concrete profile of,
discusses replay some in section 8.2 (
https://tools.ietf.org/html/draft-ietf-oauth-assertions-17#section-8.2 )
and says similar things.

In my own personal view, I think tracking assertion ids and rejecting those
that have been seen before does more harm to interoperability and
deployment at scale than good in mitigating a threat that's already
reasonably mitigated by other means. Others have wanted to have the option
in the documents, however, which is (as I recall) where the optionality of
it comes from.

> http://docs.oasis-open.org/security/saml/v2.0/saml-sec-consider-2.0-os.pdf

Perhaps you can help me with the tool usage here?

In the source XML I've got <?rfc include='
http://xml.resource.org/public/rfc/bibxml2/reference.OASIS.saml-sec-consider-2.0-os.xml'
?> in the references and
http://xml.resource.org/public/rfc/bibxml2/reference.OASIS.saml-sec-consider-2.0-os.xml
has that URL via <format type="PDF" target="
http://docs.oasis-open.org/security/saml/v2.0/saml-sec-consider-2.0-os.pdf"/>
but the URL doesn't show up in the rendered document.

The situation is the same for all the SAML references, FWIW.

---

From: Tina TSOU <[email-masked]>
Date: Thu, 16 Oct 2014 13:20:46 +0000

Dear all,

I have reviewed this document as part of the security directorate's ongoing effort to review all IETF documents being processed by the IESG.  These comments were written primarily for the benefit of the security area directors. Document editors and WG chairs should treat these comments just like any other last call comments.

** Technical **

* Page 6:
For IPv6, addition of the flow label [RFC6437] to network 5-tuples
results in network 6-tuples, but in practice, use of a flow label is
unlikely to result in a finer-grain traffic subset than the
corresponding network 5-tuple (e.g., the flow label is likely to
represent the combination of two ports with use of the UDP
protocol).

The flow label is different in each direction. Hence, if anything, you
should talk about 7-tuples rather than 6-tuples.

* Page 6, Section 2.2:

What about the drawbacks of multiplexing? -- You don't describe any.
What about Head of Line blocking?

* Page 13, Section 5.1:

When a protocol that provides reliable delivery receives a packet
other than the next expected packet, the protocol usually assumes
that the expected packet has been lost and respond with a
retransmission request for that packet.

Note: There is no "retransmission request" in TCP. Aditionally:
s/respond/responds/

* Page 13, Section 5.1:
This should not be done for current transport protocols within a
single network 5-tuple, with the exception of UDP and UDP-Lite.

This text is rather confusing. Maybe rephrase as "This should not be
done for transport protocol instances of current protocols, except of
UDP and UDP-Lite"?

* Security considerations section

Maybe Section 3.3 of RFC 6274 wuld be of use here?

** Nits/Editorial **

* Page 2:

The results of using multiple DSCPs to obtain different QoS
treatments within a single network 5-tuple (e.g., reordering) have
transport protocol interactions, particularly with congestion
control functionality.

Please "(e.g., reordering)" right after "treatments".

* Page 2:

s/requirments/requirements/

* Page 3:
Please add references for VP8 and H264

* Page 5:
RTP is usually carried over a datagram protocol, such as
UDP[RFC0768], UDP-Lite [RFC3828] or DCCP [RFC4340]; UDP is most
commonly used, but a non-datagram protocol (e.g., TCP) may also be
used.

Add missing space in "UDP[RFC0768]".

* Page 6:
When TURN selects use of TCP, the entire real-time communications
session is carried over a single TCP 5-tuple.

s/five-tuple/connection/

* Page 8, Section 3:
The DiffServ architecture[RFC2475] maintains distinctions among:

Please add the missing space

* Page 8:
s/loading/load/

* Page 11, Section 3.2:
Better results may be achieved when DSCPs are used to spread traffic
among a smaller number of DiffServ-based traffic subsets or
aggregates, see [I-D.geib-tsvwg-diffserv-intercon] for one proposal.

Please put the "see [I-D..]" within parenthesis.

* Page 13, Section 5.1:
Reordering also affects other forms of congestion control, such as
techniques for RTP congestion control that were under development
when this memo was published, see [I-D.ietf-rmcat-cc-requirements]
for requirements.

Please put the "see [I-D..." within parethesis -- i.e. "(see [I-D..)".

* Page 17, section 5.4:
SSRC

Please expand this acronym.

* Page 18, Section 6:
, see Section 5.2 above.

Please put this text within parenthesis.

Thank you,
Tina

---

From: "Black, David" <[email-masked]>
Date: Thu, 16 Oct 2014 17:45:16 +0000

Tina,

> IMHO, the document is almost ready. I just have minor comments:

Thank you for the review - the RFC 6274 pointer was particularly useful.

Comments inline @ "David>"

Thanks,
--David

From: Tina TSOU [mailto:[email-masked]]
Sent: Thursday, October 16, 2014 9:21 AM
To: [email-masked]
Cc: IESG; [email-masked]
Subject: Secdir review of draft-ietf-dart-dscp-rtp-07

Dear all,

** Technical **

* Page 6:

David> Yes, but many of the UDP flows involved are unidirectional.  I'll add a parenthetical "(or 7-tuples for bidirectional flows)."

* Page 6, Section 2.2:

David> The primary purpose of the section is to explain why one would want to do this multiplexing from an RTP
David> perspective - there are plenty of drawbacks described elsewhere in the draft, but if there's something specific
David> that belongs in this section, please suggest text ;-).

David> UDP does not exhibit TCP-like Head of Line blocking problems.

* Page 13, Section 5.1:

Note: There is no "retransmission request" in TCP. Additionally:
s/respond/responds/

David> Definitely a concern in general, and one that was also caught by another reviewer - thank you for noticing.  New wording:
David> "the protocol usually assumes that the expected packet has been lost and updates the peer, which often causes a retransmission."

* Page 13, Section 5.1:

David> The 5-tuple concept should not be dropped, as the resulting text is easy to mis-read.
David> Reorganized to: "This should not be done within a single network 5-tuple for current transport protocols, with the important exception of UDP and UDP-Lite.."

David> It's not as if the draft doesn't have enough references already :-).
David> Seriously, Section 3.3.2.1 of RFC 6274 is definitely relevant - I'll add a pointer to that and the RFC 6274 reference, thanks for the suggestion.

** Nits/Editorial **

* Page 2:

David> I prefer not to do that, as "within a single network 5-tuple" is an important modification to the scope of "different QoS  treatments."

* Page 2:

David> Fixed, thanks.

* Page 3:

David> Ok, I'll go look for these.

* Page 5:

David> Fixed, thanks.

* Page 6:

s/five-tuple/connection/

David> Good suggestion, changed to "TCP connection (i.e., 5-tuple)."

* Page 8, Section 3:

David> Fixed, thanks.  The way that XXE displays citations makes it easy to miss these ...

* Page 8:
s/loading/load/

David> "loading" is ok, prefer to leave as-is.

* Page 11, Section 3.2:

David> will change comma before "see" to a semicolon instead.

* Page 13, Section 5.1:

David> Same change as previous comment (',' -> ';').

* Page 17, section 5.4:

SSRC

David> Already done when acronym first used in Section 2.1.
David> The actual expansion is "Synchronization SouRCe," which seems unhelpful to provide in camel-case.

* Page 18, Section 6:

, see Section 5.2 above.

David> Done, and also for several other occurrences of similar text.

Thank you,
Tina