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

From: Stephen Farrell via Datatracker <[email-masked]>
Date: Tue, 03 Mar 2020 16:17:11 -0800

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.

- 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?

- 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?

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

- 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. 

- 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. 

- 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.

---

From: Gorry Fairhurst <[email-masked]>
Date: Wed, 4 Mar 2020 15:55:20 +0000

Thanks for this review Stephen.

> 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.
> but would it be worth stating that?
We suggest adding to the security considerations, see later.
> is counted as PL overhead even if sent as a sort-of trailer

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

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.

> issues. Might be best to just s/SHOULD/ought/ or something

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]).

> well?  Probably just needs a mention, so people don't get
Agree. Please see proposeed text above and the proposed adds to the 
security considerations below.
> the packet destination but to anyone on-path who can observe

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

---

From: Stephen Farrell <[email-masked]>
Date: Thu, 5 Mar 2020 12:47:00 +0000

Hi Gorry,

On 04/03/2020 15:55, Gorry Fairhurst 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:-)
>> 
>> 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.

I'll leave you all to enjoy discussion of that in peace so;-)

>> 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?

Ack.

>> 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).

I like the NEW text. But I'm still not clear if a 16 octet
AEAD tag is to be subtracted from the MPS or not;-) I guess
that'd depend on e.g. the specific ciphersuite in use and
so could vary? That might be just me though - I've always
found discussion about packet size limits ambiguous as some
people do and some do not account for overhead when they
talk about sizes. Maybe if you gave an example that'd make
it crystal clear even for the likes of me?

> 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.

Ok.

> 
>> - 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.
> 
> transport requirement to protect from DDOS. Could the SHOULD be
> retained and this be fixed by a more specific reference perhaps?
> 

Yeah, that seems right.

> 
>> - 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.

Ack.

>> - 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.)

I'll leave that one so:-) It's not a security issue, so
for the purposes of a secdir review, it oughtn't be a
blocker of any sort.

>> 
>> - 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.

That seems almost ok:-) I'd maybe switch the last sentence
around to say something like: "If a PL can make use of
cryptographic confidentiality or data-integrity mechanisms,
then adding anything (incl. padding) for DPLPMTUD that is
not protected by those cryptographic mechanisms is an anti-
pattern to be avoided, even if technically possible."

> 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). "

WFM,
Thanks,
S.