DTLS: wrong PSK no longer terminates connection immediately

[patagonaa]

During the DTLS Connection ID implementation, code was added to discard records with invalid MACs. This has the effect that connections with invalid credentials (at least when using TLS-PSK) aren't immediately terminated with a bad_record_mac TlsFatalAlert anymore:

bc-csharp/crypto/src/tls/DtlsRecordLayer.cs

Lines 780 to 787 in 04ddd28

	catch (TlsFatalAlert fatalAlert) when (AlertDescription.bad_record_mac == fatalAlert.AlertDescription)
	{
	/*
	* RFC 9146 6. DTLS implementations MUST silently discard records with bad MACs or that are otherwise
	* invalid.
	*/
	return -1;
	}

RFC 9146 6. "Peer Address Update" says

DTLS implementations MUST silently discard records with bad MACs or that are otherwise invalid.

and RFC 7366 3. says

[...] if the MAC verification fails, then processing SHALL terminate immediately. [...] For DTLS, the record MUST be discarded, and a fatal bad_record_mac MAY be generated

So while the new implementation is (in my opinion) technically correct, it is still somewhat flawed. Even when the MAC verification already fails during the handshake (i.e. when receiving the finished message), the packets are just discarded instead of returning a TlsFatalAlert. Clients with invalid credentials now have to wait (and usually resend multiple times) until they run into a timeout, instead of immediately getting the information that their credentials are invalid.

I think during the handshake, bad_record_mac alerts should still be thrown.

[patagonaa]

From what I can tell, maybe the check for bad_record_mac in the DtlsRecordLayer may be removed entirely, as there is a similar check in DtlsTransport (depending on the value of TlsServer.IgnoreCorruptDtlsRecords), which is only active once the Handshake is completed.

also, TlsServer.IgnoreCorruptDtlsRecord should be set to true by default imo, at least when a connection ID has been negotiated.

[peterdettman]

I had to think on this, but in the end I don't think that we should treat the Finished message case any differently. I found at least this related issue for openssl, where they also reject the idea:

openssl/openssl#10906

I already plan to change the default value of IgnoreCorruptDtlsRecord to true in the next major version (presumably a 3.0 release later this year).

[patagonaa]

As I already said, I would prefer if there was a way to shorten the handshake process if the PSK is wrong. The client devices we're communicating with are pretty slow, so we can't use a lower handshake timeout.

If I read the code correctly, IgnoreCorruptDtlsRecord doesn't do anything right now anyway, because the bad_record_mac error is ignored in DtlsRecordLayer instead of thrown to DtlsTransport (where IgnoreCorruptDtlsRecord is handled). If the check in DtlsRecordLayer were to be removed, the alert would still be raised and the connection ended during the handshake, and after the handshake IgnoreCorruptDtlsRecord would work correctly.

Though ultimately, it's your call to decide on an interpretation of the spec.