Good point, Dan. A caveat to my work here on this patch: I did some C programming and played with the Perl test harness in the early 90s which gave me enough abililty to develop this patch set but I would benefit from any deeper knowledge someone else may have, especially anyone with openssl knowledge.
In terms of due dilligance, I've searched through the source files for references to 'TSProxy' which is the parent directory for a number of the test hardness perl modules changes in the commit and nothing I find indicates it is referenced by the actual openssl C code but only is part of the build/test process. For extra due diligence I built with only the tests change backported and two tests fail. Tests failing is expected because the actual functional backport is missing. When I add the functional backport then all tests succeed, as expected. When I then make the test optional, the same two tests fail, as expected. See the test-results.txt attachment for more. I fix the tests by enabling the fix for 'Test 6' only in openssl-1.1.1/test/recipes/70-test_tls13messages.t by wrapping the test with perl code to set and delete the FIX_LP_1940141 environment variable. As an aside, I now see that other such env var setting does not use single-quoted strings but rather just bare strings; this is allowed in perl and my patch does not followed the style in the rest of the test file. My understanding is that there are several flags for subtests, thus accounting for the two passing or failing. But more to your point about the risk of backporting the tests, my assessment is that the perl modules are only in testing and while there are a non-trivial number of tests added along with the one that tests what is being backported, the fact that they all pass suggests that they are relevant here. That is, it looks to me these additional tests are not-inappropriate, are actually appropriate, and in fact the reason they were added later was this fix forced the issue on the lack of capability in the test coverage. ** Attachment added: "test-results.txt" https://bugs.launchpad.net/ubuntu/+source/openssl/+bug/1940141/+attachment/5572296/+files/test-results.txt -- You received this bug notification because you are a member of Ubuntu Touch seeded packages, which is subscribed to openssl in Ubuntu. https://bugs.launchpad.net/bugs/1940141 Title: OpenSSL servers can send a non-empty status_request in a CertificateRequest Status in openssl package in Ubuntu: Fix Released Status in openssl source package in Bionic: New Bug description: [Impact] openssl does not conform to RFC8446, Sec. 4.4.2.1., by sending a CertificateRequest message to the client with a non-empty status_request extension. This issue was fixed in openssl-1.1.1d and is included in Focal onward. Upstream issue is tracked at https://github.com/openssl/openssl/issues/9767 Upstream patch review at https://github.com/openssl/openssl/pull/9780 The issue leads to various client failures with TLS 1.3 as described in, e.g. https://github.com/golang/go/issues/35722 https://github.com/golang/go/issues/34040 [Test Plan] The issue can be reproduced by building with `enable-ssl-trace` and then running `s_server` like this: ``` openssl s_server -key key.pem -cert cert.pem -status_file test/recipes/ocsp-response.der -Verify 5 ``` And running `s_client` like this: ``` openssl s_client -status -trace -cert cert.pem -key key.pem ``` The output shows a `status_request` extension in the `CertificateRequest` as follows: Received Record Header: Version = TLS 1.2 (0x303) Content Type = ApplicationData (23) Length = 1591 Inner Content Type = Handshake (22) CertificateRequest, Length=1570 request_context (len=0): extensions, length = 1567 extension_type=status_request(5), length=1521 0000 - 01 00 05 ed 30 82 05 e9-0a 01 00 a0 82 05 e2 ....0.......... 000f - 30 82 05 de 06 09 2b 06-01 05 05 07 30 01 01 0.....+.....0.. 001e - 04 82 05 cf 30 82 05 cb-30 82 01 1a a1 81 86 ....0...0...... 002d - 30 81 83 31 0b 30 09 06-03 55 04 06 13 02 47 0..1.0...U....G ...more lines omitted... If the `status_request` extension is present in a `CertificateRequest` then it must be empty according to RFC8446, Sec. 4.4.2.1. [Where problems could occur] The patch disables the `status_request` extension inside a `CertificateRequest`. Applications expecting the incorrect, non-empty reply for the `status_request` extension will break with this patch. To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/openssl/+bug/1940141/+subscriptions -- Mailing list: https://launchpad.net/~touch-packages Post to : touch-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~touch-packages More help : https://help.launchpad.net/ListHelp
