Re: [multipathtcp] HMAC output size in draft-ietf-mptcp-rfc6824bis-15 (plus other comments) (with COMMENT)

[Benjamin Kaduk <[e-mail]>]

On Sat, Jun 08, 2019 at 11:21:46AM +0100, Alan Ford wrote:
> Hi Ben,
> 
> Thank you very much for your detailed comments. My responses are inline, and a -18 rev is about to appear with the changes included.
> 
> > It seems that this may just be a relic of the switch from SHA-1 to SHA-256,
> > since the SHA-1 output is 160 bits and SHA-256's is, well, 256 bits.
> > However, I'm concerned that the document is ambiguous about constructing
> > the HMAC field in MP_JOIN.  It may just be an editorial matter and easily
> > resolved, but I cannot state so with confidence without further input.
> > 
> > In particular, Figure 7 refers to a 160-bit field for the HMAC (which
> > would match with the usage of HMAC-SHA-1 in RFC 6824), but there is
> > no mention of truncating the SHA-256 HMAC in the body text (we do have
> > such discussion for the generation of tokens and IDSNs).  Are
> > implementations truncating from 256 to 160 bits for this message or
> > sending the full 256?  It also seems important to know whether there's
> > enough room in the TCP options space for it all to fit, but I didn't get
> > a chance to try to chase that down.  There's also a note in the body text
> > in Section 3.2 that still talks about "generating a 160-bit / 20-octet
> > HMAC" from SHA-256, which should get updated.
> 
> Great catches, than you. This will remain 160 bits due to option space limitations, but shall be truncated as per RFC2104 (i.e. leftmost bits). I will clarify in the text.

Thanks!

> > I also made some other notes while reading through for the crypto
> > perspective, which would have gone in the Comment section if I had balloted
> > in time:
> > 
> > I appreciate the discussion of what happens when Address ID 0 is
> > removed; that saves me from having to ask about it.
> > 
> > Section 3.1
> > 
> >   prefer not to attempt a v0 connection.  An initiator MAY cache
> >   information for a peer about what version of MPTCP it supports if
> >   any, and use this information for future connection attempts.
> > 
> > I'm a bit surprised to not see guidance here for how long to cache
> > (specifically here and not most other places in the document), since
> > this might affect "baseline load" in some environments.
> 
> That’s kinda the reason why we don’t go into detail! This statement is a MAY, as a suggestion that an implementation may wish to do this. There may be different reasons for an implementation choosing to do this, and certain environmental limitations that mean that it is not appropriate.
> 
> > Section 3.2
> > 
> >   packets) that is used as the "message".  This specification defines
> >   that HMAC as defined in [RFC2104] is used, along with the SHA-256
> >   hash algorithm [RFC6234], thus generating a 160-bit / 20-octet HMAC.
> >   Due to option space limitations, the HMAC included in the SYN/ACK is
> > 
> > [As noted in the Discuss section, SHA-256 HMACs will be 256 bits/32 octets.]
> > 
> >   Although calculating an HMAC requires cryptographic operations, it is
> >   believed that the 32-bit token in the MP_JOIN SYN gives sufficient
> >   protection against blind state exhaustion attacks; therefore, there
> >   is no need to provide mechanisms to allow a responder to operate
> >   statelessly at the MP_JOIN stage.
> > 
> > I would have liked to see some discussion in the security considerations
> > about what probability of success an attacker has at brute-forcing this
> > (as compared to port/sequence-number guessing attacks on plain TCP).
> 
> Whilst I’m not sure it’s directly comparable, we can add something like “This is in comparison to state exhaustion on a TCP port, where the port is typically already known, and SYN cookies can be deployed to aid in stateless operation.” although I am not sure this would add anything. I think saying anything like “this is 65536 times more secure“ would be disingenuous given MPTCP tokens are random but TCP ports are typically well-known.

I'm glad to hear that you don't want to say "this is 65536 times more
secure" :)
But we could still talk about how an attacker would need to guess correctly
N bits of information in order to do Y.  IIUC the N here is larger than for
many attacks on plain TCP, but I don't insist on making a direct comparison
since, as you note, it's not quite directly comparable.

> >   Due to option space limitations, the HMAC included in the SYN/ACK is
> >   truncated to the leftmost 64 bits, but this is acceptable since
> >   random numbers are used; thus, an attacker only has one chance to
> >   guess the HMAC correctly (if the HMAC is incorrect, the TCP
> >   connection is closed, so a new MP_JOIN negotiation with a new random
> >   number is required).
> > 
> > nit: I think this might be more clear if we clarified that this is "only
> > one chance to guess the HMAC that matches the nonce previously sent by
> > the peer" -- each party commits to a nonce before the HMAC is generated,
> > and requires the received HMAC to match that nonce.
> 
> Thank you!
> 
> >   The key for the HMAC algorithm, in the case of the message
> >   transmitted by Host A, will be Key-A followed by Key-B, and in the
> >   case of Host B, Key-B followed by Key-A.  These are the keys that
> >   were exchanged in the original MP_CAPABLE handshake.  The "message"
> > 
> > This is a little dicey in theory in terms of putting underlying
> > structure in the HMAC key and the risk of related key attacks, but for
> > SHA-256 I don't know of any specific hazards.  Key
> > combination/derivation methods such as HKDF would alleviate this (mostly
> > theoretical) risk, but given that the goal is "not worse than TCP" I
> > don't see a need to insist upon them here.
> 
> Thanks for the pointer here. I think this would be inappropriate to introduce at this stage given lack of real-world experience and especially since the impact on “friendly” middleboxes may cause issues, and as you say is not required to meet our security goals. But it’s a great pointer for future evolutions of this spec.

Yeah, it seems too late for changing it to be reasonable for v1, given the
risk/reward balance.  Perhaps in v2...

> >   If the HMACs are verified as correct, then both hosts have
> >   authenticated each other as being the same peers as existed at the
> >   start of the connection, and they have agreed of which connection
> >   this subflow will become a part.
> > 
> > nit: there's not much notion of "identity" here, so I'd suggest using a
> > verb like "verified" or "assured" rather than
> > "authenticated", e.g., "both hosts have provided assurance to each other
> > that they are the same peers as existed at the start of the connection”.
> 
> Thank you.
> 
> > Section 3.3
> > 
> >   The 'F' flag indicates "Data FIN".  If present, this means that this
> >   mapping covers the final data from the sender.  This is the
> >   connection-level equivalent to the FIN flag in single-path TCP.  A
> >   connection is not closed unless there has been a Data FIN exchange,
> >   or an implementation-specific, connection-level timeout.  [...]
> > 
> > Is this really appropriate to lave implementation-specific, since it
> > could result in peers disagreeing about whether a connection is closed?
> 
> These kinds of issues happen all the time in TCP today as different implementations or network conditions may lead to ends timing out at different points. This will be discovered by the peer through a TCP RST. The text existed primarily to cover MPTCP scenarios equivalent to those found in TCP today. However, this text pre-dates the MP_FASTCLOSE method, which would be preferred in this environment, and so I’ve clarified this is a send timeout and also pointed to MP_FASTCLOSE as another option.
> 
> >                       The presence of the checksum can be inferred from
> >   the length of the option.  If a checksum is present, but its use had
> >   not been negotiated in the MP_CAPABLE handshake, the checksum field
> >   MUST be ignored.  [...]
> > 
> > Why is ignoring better than RST-ing?  We abort on other cases of
> > tampering, IIUC.
> 
> Hmm, I guess you could call this tampering (adding an unexpected field)! I think the rationale here was if an implementation for some reason misunderstood the negotiation, or was lazy and just checksummed everything, then a receiver should just not care. But for consistency it seems right to close the connection, so we can change that.

I guess I didn't consider the lazy implementation checksumming everything,
but if you think this is a good change, I'm happy to see it.

> > Section 3.3.1
> > 
> >   The data sequence mapping specifies a mapping from subflow sequence
> >   space to data sequence space.  This is expressed in terms of starting
> >   sequence numbers for the subflow and the data level, and a length of
> >   bytes for which this mapping is valid.  This explicit mapping for a
> >   range of data was chosen rather than per-packet signaling to assist
> >   with compatibility with situations where TCP/IP segmentation or
> >   coalescing is undertaken separately from the stack that is generating
> >   the data flow (e.g., through the use of TCP segmentation offloading
> >   on network interface cards, or by middleboxes such as performance
> >   enhancing proxies).  [...]
> > 
> > What happens if the resegmentation/coalescing causes the end of the data
> > mapping to fall in the middle of a segment but checksums are not enabled?
> > There's some discussion of this scenario in Section 3.7 and a note that
> > the checksums would detect this but without checksums only a signal at a
> > different layer would detect tampering.  It might be worth discussing
> > the consequences (corruption in the application stream?) if no such
> > signal is available.
> 
> Thanks for the thought. This kind of thing is implied in various places but it is probably worth spelling out. So I have added the following two sentences to this paragraph: "Without checksumming enabled, corrupt data may be delivered to the application if a middlebox alters segment boundaries, alters content, or does not deliver all segments covered by a data sequence mapping. It is therefore RECOMMENDED to use checksumming unless it is known the network path contains no such devices.<"

Thank you!

> > Section 3.4.1
> > 
> >   Due to the proliferation of NATs, it is reasonably likely that one
> >   host may attempt to advertise private addresses [RFC1918].  It is not
> >   desirable to prohibit this, since there may be cases where both hosts
> >   have additional interfaces on the same private network, and a host
> >   MAY want to advertise such addresses.  The MP_JOIN handshake to
> >   create a new subflow (Section 3.2) provides mechanisms to minimize
> >   security risks.  The MP_JOIN message contains a 32-bit token that
> >   uniquely identifies the connection to the receiving host.  If the
> >   token is unknown, the host will return with a RST.  In the unlikely
> >   event that the token is valid at the receiving host, subflow setup
> >   will continue, but the HMAC exchange must occur for authentication.
> > 
> > nit: I don't think we actually specified that this is for the case of
> > hosts on different private networks using the same private address
> > range, so "unlikely event" comes as a surprise.
> 
> I don’t quite understand this comment. This text refers to a clash of 32-bit tokens (which are hashes and should be fairly random). If for the purposes of this exercise they are truly random, the probability of such a clash would be 1 in 2147483648 - that seems reasonably unlikely to me!

Sorry for being unclear -- this is just a quibble with the wording/grammar.
Perhaps "In the unlikely event that the hosts are on different private
networks but the token is valid at the receiving host" would be more clear
as to the meaning.  That is, the text itself doesn't say "this works fine
if both hosts are on the same network, but breaks in the following way if
they are different private networks using the same address range".

> > Section 5
> > 
> >   original connection setup.  It also provides verification that the
> >   peer can receive traffic at this new address.  Replay attacks would
> >   still be possible when only keys are used; therefore, the handshakes
> >   use single-use random numbers (nonces) at both ends -- this ensures
> >   the HMAC will never be the same on two handshakes.  Guidance on
> > 
> > I think it's worth saying a bit more about the "single-use" property
> > and that the party generating the nonce can discard it in bounded time,
> > since the message reacting to the nonce will have aged out.
> > On the other hand, generally when we use this sort of time-bounded
> > mechanism with nonces we do still need to take steps to ensure
> > consistent behavior within the replay window, e.g., cache the packet
> > to ensure that any duplicates received are true retransmits and not
> > an attacker having won a race with the real recipient.
> 
> Added a comment about nonce lifetime being in the lifetime of the TCP handshake.
> 
> >   The use of crypto capability bits in the initial connection handshake
> >   to negotiate use of a particular algorithm allows the deployment of
> >   additional crypto mechanisms in the future.  Note that this
> >   negotiation would be susceptible to a bid-down attack by an on-path
> >   active attacker who could modify the crypto capability bits response
> >   from the receiver to use a less secure crypto mechanism.  The
> >   security mechanism presented in this document should therefore
> >   protect against all forms of flooding and hijacking attacks discussed
> >   in [RFC6181].
> > 
> > Nit: the language used here is weird, since we "note that" there's an
> > additional attack, but then conclude ("should therefore") that all
> > attacks are covered.  So perhaps "nonetheless" is a better conjunction,
> > to show that the remaining attack is not one discussed in 6181?
> 
> Thank you.
> 
> >   The version negotiation specified in Section 3.1, if differing MPTCP
> >   versions shared a common negotiation format, would allow an on-path
> >   attacker to apply a theoretical bid-down attack.  Since the v1 and v0
> >   protocols have a different handshake, such an attack would require
> >   the client to re-establish the connection using v0, and this being
> >   supported by the server.  Note that an on-path attacker would have
> >   access to the raw data, negating any other TCP-level security
> >   mechanisms.  [...]
> > 
> > It is perhaps worth distinguishing between an on-path attacker that can
> > freely modify/add/discard all traffic from one that can freely inject
> > but not block traffic.
> 
> Most attacks against the MPTCP signalling would have to be on-path. Injection attacks would require being in-window with TCP sequence numbers so would need some on-path knowledge, or a very lucky guess. The additional security mechanisms on MPTCP signalling are designed to mitigate against these risks.

I think we're in agreement here, in that the attacker basically always has
to be on-path and can thus read the "real" traffic.  I think there are
scenarios where an attacker can read the on-path traffic but not rewrite
all traffic, so if the attacker injects new data, both the attacker's data
and the legitimate data arrive, which the receiver could detect as bogus
behavior and close the connection.  (The receiver is not obligated to do
so, though, IIUC.)  So there isn't necessarily more specifically to say
here, though I didn't do a full sweep of the document.

Thanks again,

Ben

> >   Whilst this specification defines a "medium" security solution,
> >   meeting the criteria specified at the start of this section and the
> >   threat analysis ([RFC6181]), since attacks only ever get worse, it is
> >   likely that a future Standards Track version of MPTCP would need to
> >   be able to support stronger security.  [...]
> > 
> > nit: this *is* a standards-track version; just "future version" will
> > be fine.
> 
> Thank you.
> 
> > Appendix B.3
> > 
> >   Once this is done, the received cookie can be used for TFO, as shown
> >   in Figure Figure 19.  [...]
> > 
> > nit: "Figure Figure" is redundantly redundant.  (Also in later
> > references.)
> 
> Thank you.
> 
> > Appendix C.1.1
> > 
> >   Local.Token (32 bits):  This is the token chosen by the local host on
> >      this MPTCP connection.  The token must be unique among all
> >      established MPTCP connections, generated from the local key.
> > 
> > nit: I think this is "and is generated from the local key" -- the
> > uniqueness is about all MPTCP connections on the host, not just
> > those using a given key.  (This follows naturally from the deterministic
> > procedure used to generate the token from the key, but we should still
> > be precise with our language.)
> 
> And thanks again - much appreciated!
> 
> Best regards,
> Alan
>