Remove support for SSLv2 Client Hello by kroeckx · Pull Request #28041 · openssl/openssl

kroeckx · 2025-07-15T09:42:42Z

Drop support for the SSLv2 Client Hello. We allowed that a client send an SSLv2 compatible Client Hello.

Checklist

documentation is added or updated
tests are added or updated

kroeckx · 2025-07-15T12:54:33Z

SSLv2 support itself was dropped in 1.1.0 (2016). I think the reason for keeping the SSLv2 compatible client hello support was that 0.9.8 used to do that by default. But I think it's about time to remove this.

kroeckx · 2025-07-15T13:02:49Z

We have this in ssl_local.h:

/* Flag used on OpenSSL ciphersuite ids to indicate they are for SSLv3+ */
# define SSL3_CK_CIPHERSUITE_FLAG                0x03000000

That is, it's to say it's not an SSLv2 cipher having 3 bytes.

And in include/openssl/ssl3.h all ciphers starts with 0x0300. When this patch is applied, in theory we could remove that 0x3000 from all the cipher values. But I'm not sure now is a good time to do that as it's a public header.

paulidale

Nice work. One suggestion.

doc/man3/SSL_CTX_set_client_hello_cb.pod

paulidale

I'm more than happy to see the end of this one.

mattcaswell · 2025-07-21T11:22:00Z

I think the reason for keeping the SSLv2 compatible client hello support was that 0.9.8 used to do that by default.

That, but also Internet Explorer 6 sent the SSLv2 compatible ClientHello by default. Windows XP shipped with IE6 by default. I assume at the time we were still concerned about lingering XP instances out there. I hope we don't have to worry about that in 2025!!

mattcaswell

It seems this is worthy of a CHANGES.md entry (and arguably NEWS.md as well)

doc/man3/SSL_CTX_set_client_hello_cb.pod

NEWS.md

mattcaswell

Looks good. A few minor comments

ssl/ssl_lib.c

ssl/statem/statem_srvr.c

CHANGES.md

ssl/statem/statem_lib.c

InfoHunter

Great to see this. No need to consider IE6.

openssl-machine · 2025-08-30T00:00:36Z

This pull request is ready to merge

t8m

As this was not merged in time for 3.6 version, could you please replace all the references to 3.6 with 4.0?

openssl-machine · 2025-10-10T00:09:50Z

This PR is waiting for the creator to make requested changes but it has not been updated for 30 days. If you have made changes or commented to the reviewer please make sure you re-request a review (see icon in the 'reviewers' section).

ssl/ssl_lib.c

util/libssl.num

kroeckx · 2026-01-14T11:07:38Z

Thank you for updating it, it looks good.

mattcaswell

LGTM

t8m

2 nits

CHANGES.md

ssl/statem/statem_srvr.c

esyr

ossl_bytes_to_cipher_list might need an additional refactoring pass to get the unneeded local variable replaced with a constant, and a test for actually failing sslv2 hello is a nice to have, but otherwise LGTM.

ssl/ssl_lib.c

test/recipes/70-test_sslrecords.t

tomato42

LGTM

Drop support for the SSLv2 Client Hello. We allowed that a client send an SSLv2 compatible Client Hello.

levitte · 2026-01-15T12:52:17Z

Unfortunately, re-approvals needed

beldmit

LGTM

tomato42

still LGTM

openssl-machine · 2026-01-16T14:00:55Z

This pull request is ready to merge

Drop support for the SSLv2 Client Hello. We allowed that a client send an SSLv2 compatible Client Hello. Reviewed-by: Dmitry Belyavskiy <[email protected]> Reviewed-by: Alicja Kario <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from #28041)

levitte · 2026-01-19T09:34:38Z

It looks like @mattcaswell merged this last Friday, ending up with 09c2bc5 in the master branch.

Therefore closing this.

Drop support for the SSLv2 Client Hello. We allowed that a client send an SSLv2 compatible Client Hello. Reviewed-by: Dmitry Belyavskiy <[email protected]> Reviewed-by: Alicja Kario <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from openssl#28041)

kroeckx force-pushed the sslv2 branch 3 times, most recently from 27c375f to 2d67d58 Compare July 15, 2025 09:53

github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Jul 15, 2025

paulidale reviewed Jul 15, 2025

View reviewed changes

doc/man3/SSL_CTX_set_client_hello_cb.pod Outdated Show resolved Hide resolved

paulidale previously approved these changes Jul 17, 2025

View reviewed changes

paulidale added branch: master Applies to master branch approval: review pending This pull request needs review by a committer triaged: feature The issue/pr requests/adds a feature tests: present The PR has suitable tests present labels Jul 17, 2025

mattcaswell requested changes Jul 21, 2025

View reviewed changes

kroeckx dismissed paulidale’s stale review via af1209b July 27, 2025 10:24

kroeckx force-pushed the sslv2 branch from af1209b to 0054325 Compare July 27, 2025 10:27

kroeckx requested a review from mattcaswell July 27, 2025 10:27

FdaSilvaYY reviewed Jul 27, 2025

View reviewed changes

doc/man3/SSL_CTX_set_client_hello_cb.pod Show resolved Hide resolved

NEWS.md Show resolved Hide resolved

mattcaswell requested changes Jul 28, 2025

View reviewed changes

ssl/ssl_lib.c Outdated Show resolved Hide resolved

ssl/statem/statem_srvr.c Outdated Show resolved Hide resolved

CHANGES.md Outdated Show resolved Hide resolved

kroeckx commented Jul 29, 2025

View reviewed changes

ssl/statem/statem_lib.c Outdated Show resolved Hide resolved

InfoHunter previously approved these changes Aug 28, 2025

View reviewed changes

paulidale previously approved these changes Aug 28, 2025

View reviewed changes

paulidale added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Aug 28, 2025

openssl-machine removed the approval: done This pull request has the required number of approvals label Aug 30, 2025

openssl-machine added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Aug 30, 2025

t8m requested changes Sep 9, 2025

View reviewed changes

t8m removed the approval: ready to merge The 24 hour grace period has passed, ready to merge label Sep 9, 2025

kroeckx commented Jan 14, 2026

View reviewed changes

ssl/ssl_lib.c Outdated Show resolved Hide resolved

levitte force-pushed the sslv2 branch from 9125384 to 7eed5f7 Compare January 14, 2026 10:58

kroeckx commented Jan 14, 2026

View reviewed changes

util/libssl.num Outdated Show resolved Hide resolved

levitte force-pushed the sslv2 branch from 7eed5f7 to 5f69e1a Compare January 14, 2026 11:03

mattcaswell previously approved these changes Jan 14, 2026

View reviewed changes

mattcaswell added the approval: review pending This pull request needs review by a committer label Jan 14, 2026

loci-dev mentioned this pull request Jan 14, 2026

UPSTREAM PR #28041: Remove support for SSLv2 Client Hello auroralabs-loci/openssl#355

Open

2 tasks

t8m requested changes Jan 14, 2026

View reviewed changes

CHANGES.md Outdated Show resolved Hide resolved

ssl/statem/statem_srvr.c Outdated Show resolved Hide resolved

esyr previously approved these changes Jan 14, 2026

View reviewed changes

ssl/ssl_lib.c Show resolved Hide resolved

test/recipes/70-test_sslrecords.t Show resolved Hide resolved

tomato42 previously approved these changes Jan 15, 2026

View reviewed changes

Remove support for SSLv2 Client Hello

9f7ef92

Drop support for the SSLv2 Client Hello. We allowed that a client send an SSLv2 compatible Client Hello.

levitte dismissed stale reviews from tomato42, esyr, and mattcaswell via 9f7ef92 January 15, 2026 12:51

levitte force-pushed the sslv2 branch from 5f69e1a to 9f7ef92 Compare January 15, 2026 12:51

beldmit approved these changes Jan 15, 2026

View reviewed changes

tomato42 approved these changes Jan 15, 2026

View reviewed changes

levitte added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Jan 15, 2026

t8m approved these changes Jan 15, 2026

View reviewed changes

mattcaswell approved these changes Jan 15, 2026

View reviewed changes

openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Jan 16, 2026

levitte closed this Jan 19, 2026

Uh oh!

Comments

Conversation

kroeckx commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

kroeckx commented Jul 15, 2025

Uh oh!

kroeckx commented Jul 15, 2025

Uh oh!

paulidale left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

paulidale left a comment

Choose a reason for hiding this comment

Uh oh!

mattcaswell commented Jul 21, 2025

Uh oh!

mattcaswell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mattcaswell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

InfoHunter left a comment

Choose a reason for hiding this comment

Uh oh!

openssl-machine commented Aug 30, 2025

Uh oh!

t8m left a comment

Choose a reason for hiding this comment

Uh oh!

openssl-machine commented Oct 10, 2025

Uh oh!

Uh oh!

Uh oh!

kroeckx commented Jan 14, 2026

Uh oh!

mattcaswell left a comment

Choose a reason for hiding this comment

Uh oh!

t8m left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

esyr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tomato42 left a comment

Choose a reason for hiding this comment

Uh oh!

levitte commented Jan 15, 2026

Uh oh!

beldmit left a comment

Choose a reason for hiding this comment

Uh oh!

tomato42 left a comment

Choose a reason for hiding this comment

Uh oh!

openssl-machine commented Jan 16, 2026

Uh oh!

levitte commented Jan 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

kroeckx commented Jul 15, 2025 •

edited

Loading