Hi Gorry, On 04/03/2020 15:55, Gorry Fairhurst wrote: > 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. I'll leave you all to enjoy discussion of that in peace so;-) >> - 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. Ack. >> - 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). 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? > >> - 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. 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. > > 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]). 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. > Agree. Please see proposeed text above and the proposed adds to the > security considerations below. 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. > > Gorry > >>
Attachment:
0x5AB2FAF17B172BEA.asc
Description: application/pgp-keys
Attachment:
signature.asc
Description: OpenPGP digital signature
-- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
