Subject: [tcpinc] Editorial Review Comments: draft-bittau-tcpinc-tcpcrypt-00

From: Bob Briscoe <[email-masked]>
Date: Mon, 27 Oct 2014 22:59:03 +0000

tcpcrypt authors (Andrea, Dan, Mike, Mark, David, Quinn),

I've just made technical comments in a separate email. This one is 
confined to editorial comments, e.g. improvements to clarity, nits etc.

2. Intro

Suggest reword the parenthesis at the end of the 3rd bullet to turn 
it into a positive feature.
CURRENT:
       if there was no tampering or
       eavesdropping, the IDs will match
SUGGESTED:
         This allows a mechanism that compares session IDs to be 
built over unauthenticated encryption so that active modification of 
the communication can be reliably detected.

Suggest reword 4th bullet to avoid implying extension will be needed:
         "Independent of IP address, so it will be possible to 
authenticate resumption of TCP connections when either end changes IP address."

GAP:
Nowhere in the intro does it introduce the idea of opportunistic 
encryption. It needs to explain that tcpcrypt can be enabled 
(disabled) by default at the machine level, and disabled (enabled) 
per-socket by the app. It can refer to Sec.6 for more details on the 
API but it needs this much overview at least.

3.1 Stages

It would be worth explaining that the states represent the state of 
either endpoint, not the whole connection.

3.1.1. The setup phase; 2nd para:
CURRENT:
         The setup phase uses the Data portion of TCP segments to 
exchange cryptographic keys.
SUGGESTED:
         After establishing that both endpoints support tcpcrypt, the 
protocol uses the data portion to exchange cryptographic keys and 
parameters that would otherwise be too large to fit in the limited 
space available in the TCP header.
RATIONALE:
         This sentence led me to completely misunderstand the 
protocol, until I got to the detail of each header.

3.4 Key exchange protocol

s/The TAG values/
  /The CONST values/
Rationale: now that you have renamed these variables from TAG_* to 
CONST_*, the reference back to the name TAG in the general definition 
of a CPRF earlier is not so clear.

The key subscript notation is misleading. I think you use the 
subscript 'i' twice: for a series, then for another series nested 
within the first series.
* I think you mean that each session secret ss[i] can be used to 
generate a series of master keys mk[i,j].
* Instead you've silently renamed ss[i] to ss, then used ss to 
generate a series mk[i].

I suggest the following changes:
CURRENT:
                   mk[0] := CPRF (ss, CONST_REKEY | flags, K_LEN)
                   mk[i] := CPRF (mk[i-1], CONST_REKEY, K_LEN)
SUGGESTED:
                   mk[i,0] := CPRF (ss[i], CONST_REKEY | flags, K_LEN)
                   mk[i,j] := CPRF (mk[i,j-1], CONST_REKEY, K_LEN)

CURRENT:
                    k_cs := CPRF (mk, CONST_KEY_C, ae_len)
                    k_sc := CPRF (mk, CONST_KEY_S, ae_len)
SUGGESTED:
                    k_cs[i,j] := CPRF (mk[i,j], CONST_KEY_C, ae_len)
                    k_sc[i,j] := CPRF (mk[i,j], CONST_KEY_S, ae_len)

    For brevity, where the context is clear, the subscript [i,j] will 
be suppressed.

3.8 Session caching

s/After using ss[i] to compute mk[0]/
  /After using ss[i] to compute mk[i,0]/
         or
s/After using ss[i] to compute mk[0]/
  /After using ss to compute mk[0]/

Even if you don't accept my [i,j] notation above, you need to clarify 
this phrase, which uses the two nested subscript spaces in the same 
sentence as if they were one and the same.

s/the only cost is the additional ten bytes to send NEXTK1 for the 
next connection./
  /the cost is the additional ten bytes to send NEXTK1 for the next 
connection and an extra round trip./

4.1 Protocol States

A number of normative statements apply solely to 'a host in the setup 
phase'. However, I don't see the start and end of the setup phase 
precisely defined anywhere (one would expect to see it in Sec.3.1.1 
about the Setup Phase).

4.3 The TCP CRYPT option

s/OPT1/CRYPT/
I don't think the TCP option needs to be called both OPT1 and CRYPT.

s/MAY also choose reply/
  /MAY also choose to reply/

4.3.3. The NEXTK1 and NEXTK2 suboptions

"   If the passive opener recognizes SID[i] and knows ss[i], it SHOULD
    respond with a segment containing the dataless NEXTK2 suboption.
...
    If the passive opener does not recognize SID[i], or SID[i] is not
    valid or has already been used, the passive opener SHOULD respond
    with a PKCONF or HELLO option and continue key negotiation as usual.
"
Why are these two SHOULDs not MUSTs?

"  In the event that two hosts
    simultaneously send SYN segments to each other with the same SID[i],
    but the two segments are not part of a simultaneous open
"
I don't understand how two simultaneous SYNs could not be a simultaneous open.

s/implementations MAY chose/
  /implementations MAY choose/

4.3.5. The UNKNOWN suboption

"This suboption is sent in response to an unknown suboption"
"each containing unknown options"
"If a host receives an unknown option,"
"as a result of the unknown option,"
"in the unknown opcode list"

Suggest s/unknown/unrecognised/ to make sure these are not 
misunderstood as UNKNOWN in caps.

4.4 The TCP MAC option

s/OPT2/MAC/
(as for OPT1 vs CRYPT above)

CURRENT:
    options
          These are bytes 20-off
SUGGESTED:
    options
          These are bytes 20 to (off*4 - 1)
RATIONALE:
    off is in units of 4-octet words. Then, once algebra has been 
introduced to the sentence, the '-' could be mistaken for a minus sign.

5.4. Example 4: Reusing established state

CURRENT:
    (3) A -> A: A  MAC<m>
SUGGESTED:
    (3) A -> B: A  MAC<m>

6. API extensions

I don't understand why there is both a set and an order of preference 
for symmetric ciphers. Surely the set could be defined as a sequence 
in order to specify the preference order at the same time.

Similarly, I don't understand why there's only a set of pub key algos 
and not a preference order.

Where the API section offers per-machine switches, it could mention 
the alternative of per-user switches (for multi-user machines).

8. IANA Considerations

Table 4: Delete IV row.

10.2 Informative References

Delete [...rfc2434bis] and [RFC3552] (no longer cited).

Regards

Bob

________________________________________________________________
Bob Briscoe,                                                  BT