Certificate Compression (RFC8879) by tmshort · Pull Request #9155 · openssl/openssl

tmshort · 2019-06-13T19:03:27Z

Add support for brotli compression
Add support for ~~draft-ietf-tls-certificate-compression[-05]~~ RFC8879
Add support for callback API, which allows the user to handle either compression expansion or pre-compressed server certificates.

Checklist

documentation is added or updated
tests are added or updated

tmshort · 2019-06-13T19:04:46Z

Certificate compression is enabled by default; so, it had to be removed from a number of the unit tests because it changes the number of extensions and the messages. One of the info callback tests checks to see that the compressed certificate message is indeed used.

tmshort · 2019-06-13T19:05:23Z

I could not find tests for zlib upon which to base brotli compression.

tmshort · 2019-06-13T19:11:42Z

brotli is the compression method supported by Chromium.

tmshort · 2019-06-13T20:25:28Z

The AppVeyor builds were failing because of make doc-nits, doesn't help that there's no compression documentation at all!

tmshort · 2019-06-18T14:59:56Z

This works with the stable release of Chrome (75).
Wireshark 3.0.0 recognizes the compress_certificate option.

slontis · 2019-06-25T02:45:40Z

why not just typedef this?

The equivalent isn't typedef'd in c_zlib.c; it's also not a public structure that is expected to be used everywhere.

slontis · 2019-06-25T02:49:11Z

why do you check the state for NULL here but not in other methods above?

Appears that brotli_stateful_finish() is the outlier here; all other functions check for state not being NULL. This was based on the original c_zlib.c code, which also did not check its state variable in the finish function.

tmshort · 2019-06-26T18:44:50Z

@kaduk what's the status on this draft, as in, do you think it will go another round, or will it be published as-is (technically) as an RFC?

kaduk · 2020-01-09T20:11:41Z

Per https://www.rfc-editor.org/current_queue.php#draft-ietf-tls-certificate-compression the corresponding document is in the RFC Production Center's publication queue, but is currently waiting for draft-kucherawy-rfc8478bis to be published (currently in IETF Last Call).

tmshort · 2020-01-21T17:18:58Z

re: draft-kucherawy-rfc8478bis I didn't put in zstd support.
Most of the recent changes to the ID for certificate compression are just reference updates; nothing to the protocol

tmshort · 2020-12-15T14:43:48Z

Fully updated for RFC8879

tmshort · 2020-12-15T17:37:40Z

The "GitHub CI / sanitizers" seems to be failing on test 30-test_evp_pkey_provided which appears to be unrelated to this PR.

t8m · 2020-12-15T17:51:40Z

I've rerun the jobs as we do not usually see a failure in 30-test_evp_pkey_provided. Just the encoder-decoder regularly fails.

tmshort · 2020-12-15T18:07:56Z

With config --debug enable-asan enable-ubsan enable-rc5 enable-md2 enable-ec_nistp_64_gcc_128 (the same as the sanitize CI)
My local builds have shown:

SUMMARY: AddressSanitizer: 2020 byte(s) leaked in 35 allocation(s).
../../util/wrap.pl ../../test/evp_pkey_provided_test ../../test/recipes/30-test_evp_pkey_provided => 1
not ok 1 - running evp_pkey_provided_test
30-test_evp_pkey_provided.t ........ 1/? ---------------------------------------
#   Failed test 'running evp_pkey_provided_test'
#   at test/recipes/30-test_evp_pkey_provided.t line 17.
30-test_evp_pkey_provided.t ........ Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests
30-test_pbelu.t .................... ok

tmshort · 2020-12-15T18:32:59Z

I see the above on the master branch as well, commit 0cc0164

mattcaswell · 2021-12-10T10:11:05Z

@mattcaswell @paulidale @t8m How does one get this out of the hold: substantial feature state?

The point of having this hold is to identify significant features that are stuck because they are big and complex. The idea is that the OTC will first review them to confirm whether or not they are a good idea from a technical perspective before being passed to OMC for an assessment of priority (i.e. if and when they should go in). It's on the draft agenda for the next OTC meeting on Tuesday.

slontis · 2021-12-14T10:42:25Z

This will be required for QUIC

tmshort · 2021-12-14T15:53:38Z

check_docs appears to be complaining about an area of CHANGES.md untouched by this PR.

t8m · 2021-12-14T16:26:22Z

check_docs appears to be complaining about an area of CHANGES.md untouched by this PR.

Fix for that in #17274

tmshort · 2021-12-14T16:32:02Z

check_docs appears to be complaining about an area of CHANGES.md untouched by this PR.

Fix for that in #17274

Thanks, I wasn't worried about it... but thought I'd note "it wasn't me!" :)

tmshort · 2022-01-04T21:10:27Z

The point of having this hold is to identify significant features that are stuck because they are big and complex. The idea is that the OTC will first review them to confirm whether or not they are a good idea from a technical perspective before being passed to OMC for an assessment of priority (i.e. if and when they should go in). It's on the draft agenda for the next OTC meeting on Tuesday.

Any updates from OTC @mattcaswell ?

paulidale · 2022-01-11T00:07:30Z

The OTC didn't get to this (and others). It's on the agenda for the next call (later today for me).

mattcaswell · 2022-01-11T10:41:00Z

The OTC didn't get to this (and others). It's on the agenda for the next call (later today for me).

Actually its already been passed to OMC

FdaSilvaYY · 2022-02-04T18:45:48Z

Nit : This feature or flag was introduced... ?

FdaSilvaYY · 2022-02-04T18:48:12Z

If i remember well, TLS Compression is disabled by default, for security reasons.
So this is a documentation fix that should be backported ?

Yup. Since I was in here dealing with (other) compression, I fixed it. But this would be appropriate for backporting.

mattcaswell · 2022-03-09T13:47:53Z

The OMC have voted to remove the current holds ("need OMC decision" and "substantial feature") from this PR. It may now proceed normally as per the standard review process.

paulidale · 2022-03-21T03:05:10Z

Ping @openssl/committers for reviews. I do not feel qualified to do anything better than a too superficial review of this.

* Compressed Certificate extension (server/client) * Server certificates (send/receive) * Client certificate (send/receive)

Add API to call user code to compress certificate Can be used for pre-compressed server certificates

tmshort · 2022-04-26T15:29:36Z

Closing in favor of #18186

tmshort mentioned this pull request Jun 17, 2019

Newly added undocumented symbols #9095

Closed

tmshort force-pushed the master-certcomp branch from c6cf8ef to 1ffb9ea Compare June 17, 2019 19:59

slontis reviewed Jun 25, 2019

View reviewed changes

Comment thread crypto/comp/c_brotli.c Outdated

slontis reviewed Jun 25, 2019

View reviewed changes

Comment thread include/openssl/crypto.h Outdated

slontis reviewed Jun 25, 2019

View reviewed changes

Comment thread ssl/packet.c Outdated

tmshort commented Jul 10, 2019

View reviewed changes

Comment thread ssl/statem/statem_lib.c Outdated

tmshort force-pushed the master-certcomp branch from fb84ea7 to 9940637 Compare August 9, 2019 13:40

tmshort force-pushed the master-certcomp branch from 9940637 to 4b338b3 Compare August 30, 2019 12:25

tmshort force-pushed the master-certcomp branch from 4b338b3 to 2116bd6 Compare October 1, 2019 15:32

tmshort force-pushed the master-certcomp branch from 2116bd6 to 3ed21a4 Compare January 9, 2020 18:45

tmshort force-pushed the master-certcomp branch from 3ed21a4 to 6b6bf0c Compare December 15, 2020 14:42

tmshort changed the title ~~Certificate Compression~~ Certificate Compression (RFC8879) Dec 15, 2020

tmshort force-pushed the master-certcomp branch from 6b6bf0c to 43e53f2 Compare December 15, 2020 17:36

dcooper16 mentioned this pull request Mar 11, 2021

Deprecate zlib support #14506

Closed

tmshort force-pushed the master-certcomp branch from 43e53f2 to 6f4d254 Compare March 19, 2021 18:15

mattcaswell added the hold: need omc decision label Dec 14, 2021

tmshort force-pushed the master-certcomp branch from c6708e6 to 976a765 Compare December 14, 2021 15:03

tmshort force-pushed the master-certcomp branch 2 times, most recently from 5591616 to de55ff0 Compare January 10, 2022 18:47

tmshort force-pushed the master-certcomp branch from de55ff0 to f9e55a0 Compare February 4, 2022 18:39

FdaSilvaYY reviewed Feb 4, 2022

View reviewed changes

mattcaswell removed hold: need omc decision labels Mar 9, 2022

tmshort force-pushed the master-certcomp branch from 8606829 to 4749524 Compare March 9, 2022 20:59

tmshort force-pushed the master-certcomp branch from 4749524 to 4806774 Compare March 18, 2022 23:00

tmshort added 5 commits April 14, 2022 11:18

Add brotli compression support (RFC7924)

9b5b656

Add ZSTD compression support (RFC8478bis)

a9f9d07

Add support for compressed certificates (RFC8879)

f6950d4

* Compressed Certificate extension (server/client) * Server certificates (send/receive) * Client certificate (send/receive)

Add CI to build with brotli and zstd

c4371ae

Add callback API for certificate compression

70eacc3

Add API to call user code to compress certificate Can be used for pre-compressed server certificates

tmshort force-pushed the master-certcomp branch from 4806774 to 70eacc3 Compare April 14, 2022 15:26

tmshort mentioned this pull request Apr 26, 2022

Certificate Compression (RFC8879) #18186

Closed

2 tasks

tmshort closed this Apr 26, 2022

mattcaswell mentioned this pull request Jun 27, 2023

DTLS 1.3 #13900

Open

Uh oh!

Uh oh!

Conversation

tmshort commented Jun 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

tmshort commented Jun 13, 2019

Uh oh!

tmshort commented Jun 13, 2019

Uh oh!

tmshort commented Jun 13, 2019

Uh oh!

tmshort commented Jun 13, 2019

Uh oh!

tmshort commented Jun 18, 2019

Uh oh!

Uh oh!

slontis Jun 25, 2019

Choose a reason for hiding this comment

Uh oh!

tmshort Jun 25, 2019

Choose a reason for hiding this comment

Uh oh!

slontis Jun 25, 2019

Choose a reason for hiding this comment

Uh oh!

tmshort Jun 25, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tmshort commented Jun 26, 2019

Uh oh!

Uh oh!

kaduk commented Jan 9, 2020

Uh oh!

tmshort commented Jan 21, 2020

Uh oh!

tmshort commented Dec 15, 2020

Uh oh!

tmshort commented Dec 15, 2020

Uh oh!

t8m commented Dec 15, 2020

Uh oh!

tmshort commented Dec 15, 2020

Uh oh!

tmshort commented Dec 15, 2020

Uh oh!

mattcaswell commented Dec 10, 2021

Uh oh!

slontis commented Dec 14, 2021

Uh oh!

tmshort commented Dec 14, 2021

Uh oh!

t8m commented Dec 14, 2021

Uh oh!

tmshort commented Dec 14, 2021

Uh oh!

tmshort commented Jan 4, 2022

Uh oh!

paulidale commented Jan 11, 2022

Uh oh!

mattcaswell commented Jan 11, 2022

Uh oh!

FdaSilvaYY Feb 4, 2022

Choose a reason for hiding this comment

Uh oh!

FdaSilvaYY Feb 4, 2022

Choose a reason for hiding this comment

Uh oh!

tmshort Feb 4, 2022

Choose a reason for hiding this comment

Uh oh!

mattcaswell commented Mar 9, 2022

Uh oh!

paulidale commented Mar 21, 2022

Uh oh!

tmshort commented Apr 26, 2022

Uh oh!

tmshort commented Jun 13, 2019 •

edited

Loading