openssl icon indicating copy to clipboard operation
openssl copied to clipboard

Published 20 hours ago • verified publisher icon openssl

Add zlib oneshot compression

Open tmshort opened this issue 2 years ago • 2 comments

Fixes #19520

The existing stream-based ZLIB compression does not close out the stream properly. This is incompatible with RFC1950. Add a one-shot ZLIB compression method (as used with Brotli and Zstd) to do certificate compression.

This was found by tlsfuzzer.

Checklist
  • [ ] documentation is added or updated
  • [ ] tests are added or updated

tmshort avatar Nov 04 '22 01:11 tmshort

24 hours has passed since 'approval: done' was set, but this PR has failing CI tests. Once the tests pass it will get moved to 'approval: ready to merge' automatically, alternatively please review and set the label manually.

openssl-machine avatar Nov 05 '22 18:11 openssl-machine

One of the tests is still pending after 24+ ?

tmshort avatar Nov 06 '22 04:11 tmshort

As far as I can tell, looking at buildbot, there have never been any completed buildbot/master:unix-macos11-m1 builds? @levitte ?

tmshort avatar Nov 07 '22 04:11 tmshort

Merged to master branch. Thank you for your contribution.

t8m avatar Nov 07 '22 10:11 t8m

Hopefully the added bound checks won't trigger Coverity. If so we can fix that later though.

Appears to have. The failure can only occur if sizeof(size_t) > sizeof(unsigned long), which corresponds to an LLP64 system (Windows 64-bit systems with VC++ or MinGW). I can either create a Coverity exception, or do a #if with sizeof(). @t8m any preference?

tmshort avatar Nov 08 '22 01:11 tmshort

I've dismissed the Coverity entries. IMO not worth fixing at all but if you want to submit a PR that silences Coverity in one way or another I'd approve it.

t8m avatar Nov 08 '22 07:11 t8m