Re: [TLS] AD review of draft-ietf-tls-dtls-connection-id-07

Hi Achim,

On Fri, Sep 04, 2020 at 08:58:51AM +0200, Achim Kraus wrote:
> Hi Ben,
> 
> see my comments inline.
> 
> Generally you may read the closed issues or PRs with the proper keywords
> in https://github.com/tlswg/dtls-conn-id.
> 
> FMPOV most stuff has been discussed over and over. For me, in many cases
> there are not that strong arguments for the choosen option over the
> others. But these discussion have been at that time. For now, I'm not
> sure, if changing details will bring that benefit.
> So, please, only address and change things, which are really important.

Sure.  If there are particular PRs/issues or search queries that are
relevant, that can help me revisit the previous discussions.

> 
> Am 04.09.20 um 01:02 schrieb Benjamin Kaduk:
> > Hi all,
> >
> > Sorry for the slow processing time -- my queue is still longer than I am
> > happy with.
> >
> > There's a few apparent inconsistencies that we'll need to tighten up,
> > but I don't think there's anything particularly problematic.
> > I made a PR for most of the editorial stuff:
> > https://github.com/tlswg/dtls-conn-id/pull/75
> > And having pulled the github repo up, it looks like there's a few open
> > issues and PRs still, there -- what's the status of those?
> >
> 
> #73 FMPOV it's worth to address the issue Thomas found.
> I would prefer a wording, which point to RFC6347, 4.1.2.1 and explain
> not to use the option "If a DTLS implementation chooses to generate an
> alert".

Seems reasonable to me, thanks.

> > I wonder whether, as a rhetorical device, it would be helpful to give
> > the new record payload structure a new name (DTLSCIDCiphertext?) to
> > distinguish it from the RFC 6347 DTLSCiphertext.  Then we could write
> > things like "in order to distinguish between DTLSCiphertext and
> > DTLSCIDCiphertext", "the DTLSCIDCiphertext record format is only used
> > once encryption is enabled", etc.
> >
> > Section 3
> >
> >     For sending, if a zero-length CID has been negotiated then the RFC
> >     6347-defined record format and content type MUST be used (see
> >     Section 4.1 of [RFC6347]) else the new record layer format with the
> >     tls12_cid content type defined in Figure 3 MUST be used.
> >
> > I'm glad that we take a clear stance on what content-type to use for
> > zero-length CIDs, though I forget what the reasoning was to fall this
> > way vs requiring that the tls12_cid ContentType is used whenever CIDs
> > are negotiated.  I see how this approach makes some processing easier
> > (there is always a CID present when the ContentType is used), but it
> > does have the drawback of requiring use of actual CIDs in order to
> > get encrypted content-type.  If you could remind me why this approach
> > was chosen, that will be helpful for subsequent discussions/IESG
> > review/etc.
> 
> There have benn some discussion about that, see
> 
> https://github.com/tlswg/dtls-conn-id/issues/22
> https://github.com/tlswg/dtls-conn-id/issues/28
> 
> So, I'm also glad that in the end of the discussion there was a
> consense. I'm not sure, but I think that comment
> 
> https://github.com/tlswg/dtls-conn-id/issues/28#issuecomment-458032109
> 
> made mainly the decission.

Excellent; thank you for these references.

> 
> >
> >     For receiving, if the tls12_cid content type is set, then the CID is
> >     used to look up the connection and the security association.  If the
> >     tls12_cid content type is not set, then the connection and security
> >     association is looked up by the 5-tuple and a check MUST be made to
> >     determine whether the expected CID value is indeed zero length.  If
> >     the check fails, then the datagram MUST be dropped.
> >
> > I guess there's maybe a risk that an attacker sets up a CID-ful
> > connection on a given 5-tuple and then disappears, thereby denying
> > the target of the attack the ability to use a CID-less DTLS association
> > on that 5-tuple.  But it would be hard to swamp all the ports, so it's
> > only likely to be an issue when fixed ports are used on both sides ...
> > which is known to happen sometimes.  Perhaps we should document this in
> > the security considerations.
> >
> 
> I'm not sure, if I understand that well.
> Do you assume, that in a network envirnoment, where the 5-tuple is not
> stable, someone may use a CID and so block the (instable) 5-tuple? Yes,
> but in that network your peer MUST anyway use new handshakes and so the
> CID usage will be overwriten by the new handshake.
> If the 5-tuples are stable, then the attack must spoof the 5-tuple, but
> the hello verify mechanism will block that.
> 
> If I did't get it, it may be easier to discus this as issue in the
> github repo.

I think you got it; I am just failing to remember where the "MUST anyway
use new handshakes" requirement comes in from.  And I guess that also
raises the question of what the expected behavior is when a server expects
CID-ful traffic on a given 5-tuple and receives an (unencrypted)
ClientHello on that 5-tuple.

> >     When receiving a datagram with the tls12_cid content type, the new
> >     MAC computation defined in Section 5 MUST be used.  When receiving a
> >     datagram with the RFC 6347-defined record format the MAC calculation
> >     defined in Section 4.1.2 of [RFC6347] MUST be used.
> >
> > I guess that "4.1.2" includes "4.1.2.5 New Cipher Suites", so this is
> > not inadvertently closing off extensibility (not that we expect much in
> > the way of new 1.2 ciphersuites)...right?
> 
> The MAC calculation have also been discussed:
> https://github.com/tlswg/dtls-conn-id/issues/25
> 
> If the CID is to be included into the MAC, then in my opinion, that must
> be defined for each supported cipher suite.
> 
> >
> > Section 4
> >
> >     When CIDs are being used, the content to be sent is first wrapped
> >     along with its content type and optional padding into a
> >     DTLSInnerPlaintext structure.  This newly introduced structure is
> >
> > Is it worth mentioning that this is anlogous to the TLS 1.3
> > TLSInnerPlaintext?  (I do see that we have such a reference a couple
> > paragraphs later in the context of the record padding functionality.)
> >
> >     enc_content  The encrypted form of the serialized DTLSInnerPlaintext
> >        structure.
> >
> > In Section 5.2 we refer to the DTLSCiphertext.enc_content as
> > the intermediate encrypted stuff used as input to the MAC for
> > Encrypt-then-MAC processing, which maps up well to this description, but
> > is in conflict with DTLSCiphertext being the bits that actually go on
> > the wire.  So we seem to be internally inconsistent about whether
> > enc_content includes the EtM MAC.  Most likely it will be easiest to
> > address this in Section 5.2, though.
> >
> > Section 5.x
> >
> >         MAC(MAC_write_key, seq_num +
> >
> > The TLS 1.2 notation is "seq_num" for the implicit sequence number, but
> > DTLS 1.2 says that the MAC input is the concatenation of the DTLS epoch
> > and the DTLS (explicit) sequence number.  I do not see this
> > concatenation given the name "seq_num" anywhere, so I think we need to
> > reformulate this expression.
> >
> >             cid +
> >             cid_length +
> >
> > Does this construction preserve injectivity?  It seems easier to reason
> > about when the length of an element is always before or always after the
> > element itself, but we put the length first for some of the other
> > fields (that appear after these) so there seems to be some malleability.
> 
> That order was also discussed a lot.
> https://github.com/tlswg/dtls-conn-id/pull/29
> I would prefer, if this is not changed again without strong arguments!

Thanks for the pointer!
I am not sure that the specific question about injectivity was raised
there, though.  (The topic of whether "seq_num" includes epoch was raised
but I did not see a clear resolution on my first reading, just
https://github.com/tlswg/dtls-conn-id/pull/29#discussion_r246152379)

Specifically, the question of "injectivity" is referring to a scenario
where I can use different actual values for (cid, cid_length,
length_of_DTLSInnerPlaintext, etc.) but have a collision in the constructed

cid + cid_length + length_of_DTLSInnerPlaintext + ...

(Hmm, we should probably say that length_of_DTLSInnerPlaintext is a 2-byte
field...)

Attempting to construct a trivial example on the fly, (hex)

01 01 02 02 01 <513 bytes of plaintext content>

could be cid_length=1, cid=0x01, length_of_DTLSInnerPlaintext=0x0202,
DTLSInnerPlaintext.content = 0x01 <513 bytes>, or it could be
cid_length=2, cis=0x0101, length_of_DTLSInnerPlaintext=0x0201,
DTLSInnerPlaintext.content = <513 bytes>.  The possibility of such a
collision weakens the cryptographic protection and should be avoided.

> >
> > Section 6
> >
> >     -  There is a strategy for ensuring that the new peer address is able
> >        to receive and process DTLS records.  No such test is defined in
> >        this specification.
> >     [...]
> >     Application protocols that implement protection against these attacks
> >     depend on being aware of changes in peer addresses so that they can
> >     engage the necessary mechanisms.  When delivered such an event, an
> >     application layer-specific address validation mechanism can be
> >     triggered, for example one that is based on successful exchange of
> >     minimal amount of ping-pong traffic with the peer.  Alternatively, an
> >     DTLS-specific mechanism may be used, as described in
> >     [I-D.tschofenig-tls-dtls-rrc].
> >
> > I feel like these are supposed to be talking about the same thing, but
> > the first sentence of the latter paragraph leaves me confused.  Is the
> > idea that "application protocols that implement the functionality for
> > ensuring that the peer can receive and process DTLS records" are what
> > depend on being aware of the changes of address?
> >
> 
> That should be at least one option. Using CoAP on the top of DTLS 1.2
> with CID may used CoAP-feature to ensure, that the changed address is
> valid, if a larger message is to be sent back at all.

Okay.  I was mostly asking if there was a way to condense the text and
avoid covering the same topic twice, but it is not of critical importance.

Thanks,

Ben

> > Section 8
> >
> >     With multi-homing, a passive attacker is able to correlate the
> >     communication interaction over the two paths and the sequence number
> >     makes it possible to correlate packets across CID changes.  The lack
> >
> > DTLS 1.2 CIDs don't have CID changes (other than by rehandshaking, which
> > resets the sequence number space); the last clause seems stale from DTLS
> > 1.3?
> >
> > Section 9
> >
> > We may want to reference draft-gont-numeric-ids-sec-considerations as it
> > relates to CID generation.
> >
> >     An on-path adversary can create reflection attacks against third
> >     parties because a DTLS peer has no means to distinguish a genuine
> >     address update event (for example, due to a NAT rebinding) from one
> >     that is malicious.  This attack is of concern when there is a large
> >
> > This is why we have the "[t]here is a strategy for ensuring that the new
> > peer address is able to receive and process DTLS records" requirement,
> > right?  We should probably mention that (and that it's not perfect,
> > since you have to send some records to the not-yet-verified peer address
> > as part of the verification process).
> >
> >     asymmetry of request/response message sizes.
> >
> > Is this just when the request is small and the response large, or is the
> > reverse scenario also of concern?
> >
> > Section 10
> >
> >     connection_id(TBD1) as described in the table below.  IANA is
> >     requested to add an extra column to the TLS ExtensionType Values
> >     registry to indicate whether an extension is only applicable to DTLS.
> >
> > Do we want a "DTLS-Only" column or a "DTLS/TLS" column that takes values
> > "DTLS", "TLS", and "both"?
> >
> >     IANA is requested to allocate tls12_cid(TBD2) in the "TLS ContentType
> >     Registry".  The tls12_cid ContentType is only applicable to DTLS 1.2.
> >
> > We need a value for the "description" column, and should probably
> > explicitly say that it has DTLS-OK "Y".
> >
> > Section 11.1
> >
> > The way we refer to draft-tschofenig-tls-dtls-rrc does not actually seem
> > to require it to be a normative reference.
> >
> > I'm sure we will catch some grief for normatively referring to both 5246
> > and 8446, but there's not really a way around it.
> >
> > Thanks,
> >
> > Ben
> >
> > _______________________________________________
> > TLS mailing list
> > TLS@ietf.org
> > https://www.ietf.org/mailman/listinfo/tls
> >
> 