Hi,

As promised, I have read this update now. Overall, this is certainly a useful 
(and, for me, educational! Thanks :) ) document - a good starting point, yet in 
my opinion it still needs some work.
My apologies for saying these things so late: I have read previous iterations 
of this document, but somehow the bigger issues below have never stood out to 
me as clearly as they do now. Sorry!


Here are my comments:

The main problem that I see is a missing "red thread” - e.g. when deriving the 
security features in section 4: as a reader, I would expect there to be a 1:1 
mapping from the protocol features in section 3 to the security features in 
section 4, yet there isn’t. For example: sec. 3.10, CurveCP, has, in sec. 
3.10.2, protocol features such as “1-RTT session bootstrapping” and 
“Sender-chosen explicit nonces, …”, and these are not listed in section 4 as 
such. I guess this is okay because “Protocol features” are internal details, 
not exposed to an application? But then this should be made clear in section 3, 
and the security features that we see listed in section 4 should be listed in 
sec. 3 per protocol with the same wording so that we can see a connection from 
section 3 to section 4.

Similarly, what is the rationale for making some features mandatory and some 
optional in section 4? Reasons for this choice should be explicitly laid out at 
the beginning of section 4. Maybe the reason can be derived from section 5… see 
below:

Once you’ve laid out the “interface” (I’ll come to that) as you do in section 
5, just from looking at this list, it’s easy to see that TLS gives us 
everything except:
- Authentication Delegation
- Explicit (as opposed to “direct”) pre-shared key import
- Transport mobility
… and none of these are mandatory according to section 4 (so maybe that could 
be seen as a reason to not make them mandatory?). TLS could be a general 
recommendation, combined with an explanation that, if you want one of these 
things listed above, you need to implement a protocol other than TLS, but then 
you need to be aware of the protocol dependencies that such a protocol choice 
brings with it.

Generally, as an implementer of a transport system, this document now tells me 
quite clearly which features I must offer (and it tells me which protocols I 
then must support). However, the trade-offs are not so clear. I see trade-offs 
in the protocol dependencies that you list in section 3; then there are 
Transport Dependencies listed in section 4, but only for the optional features, 
and it says “None” almost everywhere… yet, in section 3, there are tons of 
“Protocol Dependencies”, so I find this surprising.

Another missing “red thread”: why do the transport features have different 
names in sections 4 and 5? I would expect to find “Transport mobility” in 
section 4, but there it’s “Connection mobility”. Similarly, I’d expect to find 
“Session cache management” (from sec. 5.1) in section 4, but there it’s called 
“Session caching and management”. I think the whole document would look much 
more uniform and better structured with some clearer mappings - from section 3 
to the listed security features in section 4 and the “interfaces” in section 5.

This brings me to the next issue: I think there’s a terminology problem with 
this document. The terminology section, in line with all other TAPS documents, 
mentions “confidentiality” as an example of a “Transport Feature”. Further 
down, “Security Feature” is introduced. Given the description of Transport 
Feature, a Security Feature is a special case of a Transport Feature. Maybe the 
first sentence of the Security Feature definition could be changed to say “A 
Transport Feature that a network security layer provides to applications.”  
Section 3 then talks about “Protocol features”; these are maybe not offered to 
applications, so I think it’s fine to use this term, but you should also define 
it in the Terminology section. Then, Section 5 talks about Transport Security 
Protocol Interfaces - but that term isn’t defined anywhere. I think these 
“interfaces” are really Security Features, no?  Actually, given that they seem 
to be the same but with slightly different wording, I wonder if sections 4 and 
5 could be merged … or maybe they should be switched in order: section 5 should 
appear first, calling them Security Features, and then what is now Section 4 
could come after, deriving which ones of them are mandatory and optional, as 
well as list transport / application dependencies.


Some smaller things / nits:

I find it strange that this draft cites only RFC 8095 but no other TAPS 
documents.
In particular, because it complements draft-ietf-taps-minset in the process of 
TAPS, I think it should reference back to it.
(The minset draft says that it doesn’t discuss security because this is covered 
in draft-ietf-taps-transport-security, but then 
draft-ietf-taps-transport-security only says that it complements RFC 8095 - 
this gives readers following the link from minset a confusing context).

Section 3.2.3: Path MTU Discovery surprised me here. What does that mean? That 
DTLS relies on an application implementing PMTUD (as it’s over UDP), and then 
telling DTLS about it? Doesn’t DTLS do this on its own? Why not? Sorry for 
asking a DTLS question, surely it’s me… but just listing PMTUD as a dependency 
isn’t a good enough explanation, I think.

sec. 3.3: when I first read “A mapping for QUIC over TLS 1.3 has been specified 
to provide this handshake”, I was surprised. I mean, given that TLS runs over 
TCP, this sentence sounds like something really doesn’t match here. Only in 
sec. 3.3.3 you explain “TLS within QUIC relies on a reliable stream abstraction 
for its handshake”. Well that’s what I thought, but then this should probably 
be brought earlier - “mapping for QUIC over TLS 1.3” just sounds like QUIC uses 
TLS, which uses TCP, which is of course wrong. Maybe it’s just the word “over” 
- using “with” instead would already make this clearer.

sec. 3 sentence 1:
“…security protocols that _are_ currently used to …”

sec 3.3.1 1st sentence after bullet list:
“The QUIC transport layer support_S_ multiple streams…”
Same para, 3rd sentence: “The TLS handshake, along with further records, are 
sent over this stream.”  - did you mean to say “The TLS handshake messages” ?

sec 3.4, line 2:
“either as for creating tunnels (tunnel-mode)”  - I think the “as” should be 
removed here?

penultimate sentence of sec. 3.5.1:
“…DTLS-offered certificates match that which are offered over SIP” - is “that” 
correct here?  (should it be “the ones” or something like that?)

3.10.1 par 5:
This begins with “Unlike TCP, …”  - isn’t this strange to say, for being too 
obvious? Or did you mean to say tcpcrypt?
Well actually even TCP has cookies for some form of “source validation”, with 
TFO. Of course that’s a limited use, but …   maybe simply removing “Unlike TCP” 
here would make this clearer.

4.2, “Mutual authentication” and “Source validation”: the sentence describing 
these features says “must”, yet it’s in the section of optional features. While 
this doesn’t really matter, it makes for a slightly surprising read, and would 
IMO be better as a factual statement - e.g. “Transport security protocols allow 
each endpoint to authenticate the other …”  and “Source validation is provided 
to mitigate …”

Sec 5.1.: missing line break after:
- Session Cache Management
- Connection Termination
- Pre-Shared Key Export
… and before “Protocols” in the “Identity Validation” item.


That’s it - I hope this is helpful!

Cheers,
Michael

_______________________________________________
Taps mailing list
Taps@ietf.org
https://www.ietf.org/mailman/listinfo/taps

Reply via email to