1471126 - Record layer separation

Status: RESOLVED FIXED

(NSS :: Libraries, enhancement)

Description

[Martin Thomson [:mt:]]

QUIC draft -13 will require a use of TLS where the TLS record layer is replaced with QUIC packets.  To support that, we need to provide APIs in two parts:

1. Callbacks or functions that enable reading and writing of the contents of records.  This will allow QUIC to replace the record layer.

2. Callbacks or functions that allow access to the secrets that TLS generates.  This will allow QUIC to use these secrets so that it can generate QUIC packet protection keys.

Proposed API here: https://docs.google.com/document/d/1_fnDGxS3Btdk3iIvfDhe3O8IjbxBcOLQql4FamtmnMs/edit?usp=sharing

Comment 1

[Phabricator Automation]

Attachment: Bug 1471126 - Rename SSL3ContentType and make it public, r?ekr

The renaming here is less widespread than I expected.  I removed the
content_alt_handshake while I was at this; no point in putting that in a public
API.

Comment 2

[Phabricator Automation]

Attachment: Bug 1471126 - Provide a callback for traffic secrets, r?ekr

This seemed very neatly separable, and less complicated than the record
layer separation piece.

Comment 3

[Martin Thomson [:mt:]]

Attachment: Bug 1471126 - Record layer separation, r?ekr

Testing this was a little tricky, I had to do something abominable with NSPR IO
layers to get this to work cleanly, but the results seem reasonably clean.

This isn't the end of this.  There will be another patch that removes the custom
hooks added so that we can decrypt and re-encrypt in tests.  That can use the
new API.

Comment 4

[Phabricator Automation]

Comment on attachment 8987733 [details]
Bug 1471126 - Rename SSL3ContentType and make it public, r?ekr

Eric Rescorla (:ekr) has approved the revision.

https://phabricator.services.mozilla.com/D1823

Comment 5

[Martin Thomson [:mt:]]

The easy part: https://hg.mozilla.org/projects/nss/rev/ed450ac8ac6de3ca6173032507431d4dd933477c

Comment 6

[Martin Thomson [:mt:]]

Attachment: Bug 1471126 - Provide extra information needed to use record layer separation, r?ekr

This started as an attempt to remove the cipher spec update callback we use for
testing.  Using the new, public secrets interface should be better for that.

In doing so, it became apparent that we needed more interfaces to NSS to support
the use of these secrets.  In particular:

1. We need to know what the KDF hash function is for a given cipher suite.  This
   allows users of the secret to use the right hash function.

2. We need to know what cipher spec was picked when sending 0-RTT.  NSS
   currently doesn't expose that information.  (When receiving 0-RTT you can
   safely assume that the negotiated cipher suite is good to use.)

So this patch adds these functions to the appropriate info functions and uses
that information in tests to remove and re-add protection.

Comment 7

[Phabricator Automation]

Comment on attachment 8987747 [details]
Bug 1471126 - Provide a callback for traffic secrets, r?ekr

Eric Rescorla (:ekr) has approved the revision.

https://phabricator.services.mozilla.com/D1824

Comment 8

[Eric Rescorla (:ekr)]

Comment on attachment 8990172 [details]
Bug 1471126 - Provide extra information needed to use record layer separation, r?ekr

Eric Rescorla (:ekr) has approved the revision.

Comment 9

[Martin Thomson [:mt:]]

remote: https://hg.mozilla.org/projects/nss/rev/1a1e0017c77664c7e5e2867a21404b77a9ea97e4
remote: https://hg.mozilla.org/projects/nss/rev/4f9d8e9f0b5c1e2806061c2bc848c0618056d57a
remote: https://hg.mozilla.org/projects/nss/rev/c1a1ee048c07fef6ef889e5ff459ddbda8da0273

Comment 10

[Martin Thomson [:mt:]]

Attachment: Bug 1471126 - Fix return codes from SSL_ForceHandshake and SSL_RecordLayerData, r?ekr

Turns out that there were two errors that made my life using SSL_RecordLayerData hard:

• SSL_ForceHandshake was returning SECFailure/PR_WOULD_BLOCK_ERROR when the record layer was replaced, even when the handshake was complete. This was being obscured in the tests by the fact that we mark sockets as complete through both the callback and SSL_ForceHandshake. I didn't change that aspect of the tests because different tests rely on that being the case. I don't have a good strategy for dealing with that, but I will continue to think on it.

• SSL_RecordLayerData was returning SECFailure/PR_WOULD_BLOCK_ERROR when it succeeded, but the AuthCertificate callback blocked. The contract for SSL_RecordLayerData is that it returns SECSuccess always. I had explicitly ignored this error in tests, which was just a mistake.

Comment 11

[Martin Thomson [:mt:]]

Attachment: Bug 1471126 - Fix documentation, r?KevinJacobs

In D1992, @glandium pointed out an error in the comments.
There is the bigger question of forward compatibility there, but this is just
to fix the obvious error.

Comment 12

[J.C. Jones [:jcj] (he/they)]

https://hg.mozilla.org/projects/nss/rev/4ec0318c964e497d30ed63d3fbe2ba3c279b3e73