Re: [secdir] Secdir last call review of draft-ietf-tsvwg-datagram-plpmtud-15

[Gorry Fairhurst <[e-mail]>]

Thanks for this review Stephen.

On 04/03/2020 00:17, Stephen Farrell via Datatracker wrote:
> Reviewer: Stephen Farrell
> Review result: Has Issues
>
>
> Hi,
>
> As Hilary Orman always nicely says: do not be alarmed,
> this is just a secdir review:-)
>
> I classed this as "has issues," but the issues below should be
> fairly easy to fix.  Maybe all but 2 are real issues that say are
> worth fixing, if I'm right about 'em (but I'm also often wrong
> too:-)
>
> Cheers,
> S.
>
> - Abstract: This draft aims for proposed standard but is
> updating a BCP (RFC8085/BCP145).  I'm happy to leave the
> process-lawyering for that to others.
We spoke with our TSV ADs and they  don't see any issue with this. BCP 
and Standards track are equivalent.
> - 4.1: I'm not sure if this is recommending that PLs allow for
> padding outside of cryptographic protection. If so, that
> seems like an anti-pattern when considering the overall set of
> requirements one might envisage for a PL. If not, that's fine,
> but would it be worth stating that?
We suggest adding to the security considerations, see later.
> - 4.4: Would you count the AEAD tag length in the MPS of an
> AEAD-encrypting PL or not? I assume not, and the tag length
> is counted as PL overhead even if sent as a sort-of trailer
> within the ciphertext?

Yes, all PL-overhead is included, but I think worth noting explicitly in 
Section 4.1 to avoid doubt. I suggest we add:

OLD:
Protocols that permit exchange of control messages
(without an application data block) can generate a probe packet by
extending a control message with padding data.
NEW:
Protocols that permit exchange of control messages
(without an application data block) can generate a probe packet by
extending a control message with padding data. The total size of a
probe packet includes all headers and padding added to the payload data
being sent (e.g., including  protocol option fields, security-related
fields such as an AEAD tag and TLS record layer padding).

> - 4.4: What'd be the MPS etc for an MP-TCP-like PL? Is that
> well-defined?

For MP-TCP this isn't an issue, but each PL could have it's own issues 
and needs to work out what MPS means.

> - 4.5: (a nit) "The validation SHOULD utilize information that
>    it is not simple for an off-path attacker to determine
> [RFC8085]." A SHOULD that's that vague seems likely prone to
> issues. Might be best to just s/SHOULD/ought/ or something
> like that.

RFC8085 does provide some examples, and this is a common-enough 
transport requirement to protect from DDOS. Could the SHOULD be retained 
and this be fixed by a more specific reference perhaps?

(see Section 5.1 of [RFC8085]).

> - 6.2.3: what if TLS record layer padding is being done as
> well?  Probably just needs a mention, so people don't get
> their sums/APIs wrong.
Agree. Please see proposeed text above and the proposed adds to the 
security considerations below.
> - 6.3: I am surprised that the QUIC description here is ready
> to be an RFC before QUIC itself. I do see there are
> normative references, but the potential for a breaking change
> still exists, and seems a bit unwise. (I'd suggest, holding
> this in the WG 'till the referenced QUIC drafts are in the RFC
> editor queue, or else taking that bit out and putting it into
> a new I-D.)
>
> - section 9: Ok this is a stretch so maybe not worth bothering
>    with but... A PL doing all this may be emitting oddly sized
> padding octets from time to time that piggy-back on
> application data. That (number of padding octets and the
> pattern with which those are emitted) seems like a medium-nice
> covert channel e.g. for exfiltrating data, not necessarily to
> the packet destination but to anyone on-path who can observe
> the signal.

I think this is useful. Security thoughts are always welcome. How about 
adding two extra paras to the Security Considerations, are these near?:

"DPLPMTUD methods can introduce padding data to inflate the length of 
the datagram to the total size required for a probe packet. The total 
size of a probe packet includes all headers and padding added to the 
payload data being sent (e.g., including security-related fields such as 
an AEAD tag and TLS record layer padding). The value of the padding data 
does not influence the DPLPMTUD search algorithm, and therefore needs to 
be set consistent with the policy of the PL.  A secure PL MUST provide 
cryptographic protection for the padding if this is needed to satisfy 
the security requirements of that PL.

A PL that implements this spec would send probe packets, whose size and 
frequency can be observed by the network. Designers of search algorithms 
need to consider whether the size of probe packets and the pattern of 
probing could result in a potential covert channel (e.g., for 
exfiltrating data to an on-path device or the endpoint of the connection). "

Gorry

>