Closed Bug 1443854 Opened 3 years ago Closed 2 years ago

Enable more TLS 1.3 tests for BoGo

Categories

(NSS :: Test, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ttaubert, Assigned: franziskus)

References

Details

Attachments

(1 file)

We currently exclude "*TLS13*". With a draft-23 BoGo (bug 1443799) we should be able to get some more coverage.
There are many tests that fail mostly due to a (IMO) wonky middlebox compat mode implementation in the BoGo code. Per spec [1] the server MUST send a CCS only if the client sends a non-empty SessionID.

The BoGo client doesn't send a SessionID by default, it however always expects a CCS. We only send a CCS when we think that the client opted into the compat mode, as described by the spec.

It seems that we're implementing the spec correctly and BoGo shouldn't expect a CCS, but of course tolerate one if we would send it.

Eric, wdyt?

[1] https://tlswg.github.io/tls13-spec/draft-ietf-tls-tls13.html#rfc.appendix.D.4
Flags: needinfo?(ekr)
Yes, this sounds like a defect in BoGo, or at least an over-strict test.
Flags: needinfo?(ekr) → needinfo?(davidben)
BoGo is a test suite for BoringSSL and tests the stricter specification for BoringSSL. BoringSSL's specification is "always enable compatibility mode". (I don't like the client-triggered clause in the spec. It's needless complexity.)
Flags: needinfo?(davidben)
David, would you be willing to take a patch to BoBo to send the session ID?
Flags: needinfo?(davidben)
Hrm. How about BoGo send the session ID in TLS 1.3 by default, but we'll add one test where BoGo doesn't send one and asserts that the shim server still does compatibility mode? Then you can mask off that one, but the majority of the tests will still check out.
Flags: needinfo?(davidben)
Fantastic
(In reply to David Benjamin from comment #7)
> How's this work for you all?
> https://boringssl-review.googlesource.com/c/boringssl/+/26504

Works great, just tested it locally. Thanks David!
Sorry about the delay. This should be in BoringSSL master now.
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Comment on attachment 8979462 [details]
Bug 1443854 - Enable more TLS 1.3 tests for BoGo

Martin Thomson [:mt:] has approved the revision.

https://phabricator.services.mozilla.com/D1346
Attachment #8979462 - Flags: review+
Assignee: ttaubert → nobody
Status: ASSIGNED → NEW
https://hg.mozilla.org/projects/nss/rev/e2a0d66b122f873a5ca370de7674af377279e37e
Assignee: nobody → franziskuskiefer
Target Milestone: --- → 3.39
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.