Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Better handling for wrong SNI #5379

Closed
sbordet opened this issue Oct 3, 2020 · 2 comments · Fixed by #5398
Closed

Better handling for wrong SNI #5379

sbordet opened this issue Oct 3, 2020 · 2 comments · Fixed by #5398
Assignees
@sbordet

Comments

@sbordet
Copy link
Contributor

sbordet commented Oct 3, 2020

Jetty version
9.4.x

Description
When the client sends a wrong SNI, and jetty.ssl.sniHostCheck=true Jetty should return a 400, but it returns 200.
@sbordet sbordet self-assigned this Oct 3, 2020
@sbordet
Copy link
Contributor Author

sbordet commented Oct 3, 2020

@gregw strengthening the verifications when jetty.ssl.sniHostCheck=true fails a few tests that use a bogus certificate that has no CN and no SAN.

I don't think we should support bogus certificates like that, so I'm for do the strengthening and fix the tests.
WDYT?

@joakime
Copy link
Contributor

joakime commented Oct 3, 2020

The 400 is returned from the SecureRequestCustomizer. Is that present?

https://github.com/eclipse/jetty.project/blob/a76702baea01186d6349e62e4045968507e66649/jetty-server/src/main/java/org/eclipse/jetty/server/SecureRequestCustomizer.java#L246-L268

There's also a SNI check in SslContextFactory.Server

https://github.com/eclipse/jetty.project/blob/338ada6076ce454afeb10a88bc63e51dfb8c6dad/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java#L2146-L2158

sbordet added a commit that referenced this issue Oct 5, 2020
@sbordet
Fixes #5379 - Better handling for wrong SNI.
62f550c 
Now returning 400 when SNI does not match any certificate
and SecureRequestCustomizer.sniHostCheck=true.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet linked a pull request Oct 5, 2020 that will close this issue
Fixes #5379 - Better handling for wrong SNI. #5398
Merged
sbordet added a commit that referenced this issue Oct 8, 2020
@sbordet
Fixes #5379 - Better handling for wrong SNI.
e54122b 
Reworked the SNI logic.
Added support for IP addresses in the SAN extension of certificates in the X509 class.
Fixed keystores to have CN=localhost and SAN with ip=127.0.0.1 and ip=[::1].
Fixed tests that were not using the correct Host header.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
sbordet added a commit that referenced this issue Oct 12, 2020
@sbordet
Fixes #5379 - Better handling for wrong SNI. (#5398)
1cd15e8 
* Fixes #5379 - Better handling for wrong SNI.

Reworked the SNI logic.
Added support for IP addresses in the SAN extension of certificates in the X509 class.
Fixed keystores to have CN=localhost and SAN with ip=127.0.0.1 and ip=[::1].
Fixed tests that were not using the correct Host header.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
sbordet added a commit that referenced this issue May 18, 2021
@sbordet
Fixes #6276 - Support non-standard domains in SNI and X509.
3a9c34e 
Backported support for IP addresses in X509 (from #5379).
Introduced SslContextFactory.Client.SniProvider to allow applications to specify the SNI names to send to the server.
Improved logging of SNI processing.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This was referenced May 18, 2021
Merged
Closed
sbordet added a commit that referenced this issue May 21, 2021
@sbordet
Fixes #6276 - Support non-standard domains in SNI and X509. (#6296)
04df6d4 
* Fixes #6276 - Support non-standard domains in SNI and X509.

Backported support for IP addresses in X509 (from #5379).
Introduced SslContextFactory.Client.SniProvider to allow applications to specify the SNI names to send to the server.
Improved logging of SNI processing.
Skip X509 matching over IP addresses when the host does
not look like an IP address, to avoid reverse DNS lookup.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
sbordet added a commit that referenced this issue May 21, 2021
@sbordet
Fixes #6276 - Support non-standard domains in SNI and X509. (#6296)
4d45c5f 
Improved support for IP addresses in X509 (after #5379).
Introduced SslContextFactory.Client.SniProvider to allow applications to specify the SNI names to send to the server.
Improved logging of SNI processing.
Skip X509 matching over IP addresses when the host does
not look like an IP address, to avoid reverse DNS lookup.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
(cherry picked from commit 04df6d4)
@sbordet sbordet mentioned this issue May 21, 2021
Merged
sbordet added a commit that referenced this issue May 24, 2021
@sbordet
Fixes #6276 - Support non-standard domains in SNI and X509. (#6296) (#…
cfcb2ef 
…6311)

* Fixes #6276 - Support non-standard domains in SNI and X509. (#6296)

Improved support for IP addresses in X509 (after #5379).
Introduced SslContextFactory.Client.SniProvider to allow applications to specify the SNI names to send to the server.
Improved logging of SNI processing.
Skip X509 matching over IP addresses when the host does
not look like an IP address, to avoid reverse DNS lookup.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
(cherry picked from commit 04df6d4)
lucamilanesio pushed a commit to GerritCodeReview/jgit that referenced this issue Nov 22, 2021
@msohn
Update Jetty to 10.0.6
f91e47f 
- this is the same version eclipse platform currently uses
- update servlet-api to 4.0
- configure keystore used by AppServer with Subject Alternative Names
  for host name and ip address to satisfy more strict SNI checking in
  Jetty 10. See jetty/jetty.project#5379
- add jetty bundles to JGit-dependency-bundles in the jgit p2 repository

Bug: 571932
Bug: 576100
Change-Id: Ibd0240cf7ad4dc201947fd69707f517c3c1fc1c8
@goekay goekay mentioned this issue Jan 29, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sbordet sbordet

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes #5379 - Better handling for wrong SNI. jetty/jetty.project
2 participants
@joakime @sbordet