1809196 - DTLS1.3 Interoperability check

Status: ASSIGNED

(NSS :: Libraries, enhancement, P2)

Comment 1

[Anna Weine]

Attachment: WIP: Bug 1809196 - DTLS1.3 Interoperability check

Comment 2

[Anna Weine]

Caught during the WolfSSL interoperab check:
The supported extension was sending wrong identifiers. Instead of the DTLS1.3 id (0xfefc), the client was sending the TLS1.3 ID.
Could be solved by undefining the DTLS_1_3_DRAFT_VERSION used in tls13_EncodeVersion function.

Upd: https://bugzilla.mozilla.org/show_bug.cgi?id=1818222

Comment 3

[Anna Weine]

As already specified in the bug https://bugzilla.mozilla.org/show_bug.cgi?id=1809872,
the current implementation of DTLS1.3 does not manage the epoch/sequence number properly.
Instead of one uint64, we need to have a pair of uint64 for the epoch and the sequence number.

During the WolfSSL interoperab check we realised that the ACK format we send is not treated properly by the WolfSSL library. As mentioned earlier, it expects a pair of uint64.

Quick draft of the modification that worked:
in the dtls13_SendAck function to modify the compute the epoch and seqNum using entry->record.

dtls13_SendAck(sslSocket *ss): 
PRUint64 epochRevealed = (PRUint64) entry->record >> 48;
PRUint64 seqNumRevealed = (PRUint64) entry->record && 0xffffffffffff; <- clean the rest of the 48 bits
rv = sslBuffer_AppendNumber(&buf, epochRevealed, 8);
rv = sslBuffer_AppendNumber(&buf, seqNumRevealed, 8);

Upd: https://bugzilla.mozilla.org/show_bug.cgi?id=1809872

Comment 4

[Martin Thomson [:mt:]]

For the ACK format, I suggest doing just the fix you have there (basically pad the epoch and the sequence number). We might not want to support more than a 16 bit epoch in NSS because changing it to a larger size might have ABI compatibility issues. And a 48-bit sequence number is plenty large enough (we should be updating keys more often than that).

Comment 5

[Anna Weine]

There is an inconsistency in computing hashes on your side vs WolfSSL side.

WolfSSL transcript function skips the MessageSequence + Fragment Offset + Fragment Length (Handshake header) fields while computing the transcript, we compute using these fields.

RFC (TLS1.3): This value (transcript one) is computed by hashing the concatenation
of each included handshake message, including the handshake message
header carrying the handshake message type and length fields, but not
including record layer headers.

Could be solved (ugly, i know) by:
the function ssl3_AppendHandshakeInternal (https://searchfox.org/mozilla-central/source/security/nss/lib/ssl/sslencode.c#287)
is responsible (among other things) for uploading the client message to ssl.hs.messages. Instead of the full message living in src (https://searchfox.org/mozilla-central/source/security/nss/lib/ssl/sslencode.c#308), I copy the first 4 bytes (that correspond to HandshakeType and Length), then skip the next 8 bytes (MessageSequence + Fragment Offset + Fragment Length), finally I copy the rest.

if (!suppressHash && (!ss->firstHsDone || ss->version < SSL_LIBRARY_VERSION_TLS_1_3)) {
uint8_t* bufferToHash = (uint8_t * ) malloc(sizeof (uint8_t) * (bytes - 8));
 if (bytes > 12) { // temp hack
   memcpy(bufferToHash, src, 4);
   memcpy(bufferToHash + 4, src + 12, bytes - 12);
   rv = ssl3_UpdateHandshakeHashes(ss, bufferToHash, bytes - 8); // instead of rv = ssl3_UpdateHandshakeHashes(ss, src, bytes);
 }

Comment 6

[Anna Weine]

It seems that there is a difference between the AEAD Nonce generation (https://searchfox.org/mozilla-central/source/security/nss/lib/ssl/tls13con.c#5671).

Let's call the extracted nonce as IV, and the buffer to randomize it - Nonce.
For WolfSSL implementation the nonce is presented as a sequence number, for us it includes epoch etc.
See RFC 8446: 5.3. Per-Record Nonce

Quick fix (here we just upload the seqNum).

static SECStatus
tls13_FormatAdditionalData(
    sslSocket *ss,
    const PRUint8 *header, unsigned int headerLen,
    DTLSEpoch epoch, sslSequenceNumber seqNum,
    PRUint8 *aad, unsigned int *aadLength, unsigned int maxLength)
{
    SECStatus rv;
    sslBuffer buf = SSL_BUFFER_FIXED(aad, maxLength);

    if (IS_DTLS(ss))
    {
        rv = sslBuffer_AppendNumber(&buf, seqNum, 8);
        if (rv != SECSuccess) {
            return SECFailure;
        }
    }

P.s. it could be done just done in some cases, I did not look further than decryption of extensions.
I saw this https://crypto.stackexchange.com/questions/50025/why-does-tls-1-3-use-random-looking-nonces-for-aead as well.

Comment 7

[Anna Weine]

Not surprisingly (See the message right above), the function to modify the AEAD IV used to protect the record requires a modification.

Quick hack: The function https://searchfox.org/mozilla-central/source/security/nss/lib/ssl/tls13con.c#4172 updates the IV with the epoch number. We do not need it in the DTLS1.3 case.

unsigned int
tls13_SetupAeadIv(PRBool isDTLS, unsigned char *ivOut, unsigned char *ivIn,
                  unsigned int offset, unsigned int ivLen, DTLSEpoch epoch)
{
    PORT_Memcpy(ivOut, ivIn, ivLen);
    if (isDTLS && (ss->version < SSL_LIBRARY_VERSION_TLS_1_3)) { // <---- instead of if (isDTLS)
    // modification of the IV implemented before

Upd: https://bugzilla.mozilla.org/show_bug.cgi?id=1818235

Comment 8

[Anna Weine]

(In reply to nkulatova from comment #3)

As already specified in the bug https://bugzilla.mozilla.org/show_bug.cgi?id=1809872,
the current implementation of DTLS1.3 does not manage the epoch/sequence number properly.
Instead of one uint64, we need to have a pair of uint64 for the epoch and the sequence number.

During the WolfSSL interoperab check we realised that the ACK format we send is not treated properly by the WolfSSL library. As mentioned earlier, it expects a pair of uint64.

Related problem. The dtls13_HandleAck function processes incorrectly the acknowledgements. Instead of seeing them as (epoch, seq), it parses it as (seq). That's why the acknowledgement of the messages does not work.

Quick fix:

rv = ssl3_ConsumeHandshakeNumber64(ss, &epoch, 8, &b, &l);
rv = ssl3_ConsumeHandshakeNumber64(ss, &seq, 8, &b, &l);
seq = dtls_CombineSequenceNumber(epoch, seq);

Comment 9

[Anna Weine]

Important: the WolfSSL MTU is equal to 1400, whereas the messages (the certificates) lengths we send are above.

Fix: while calling WolfSSL: WOLFSSL_MAX_MTU=1500

Comment 10

[Martin Thomson [:mt:]]

If WolfSSL drops datagrams, then we should retry with a smaller size after a timeout. Is that not happening?

Comment 11

[Anna Weine]

tstclnt was failing before that. But yes, that works.

Comment 12

[Anna Weine]

Attachment: WIP: Bug 1809196 - DTLS1.3 Interoperability testing script and required data