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.

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

Embedded Jetty throws null pointer exception #6076

Closed
parmindersk opened this issue Mar 21, 2021 · 6 comments · Fixed by #6078
Closed

Embedded Jetty throws null pointer exception #6076

parmindersk opened this issue Mar 21, 2021 · 6 comments · Fixed by #6078
Assignees

Comments

@parmindersk
Copy link

Jetty version
11.0.1
Java version
11
OS type/version
MacOX
Description
I'm using this code verbatim to run an embedded Jetty server.

HTTP calls work totally fine. However, HTTPS calls start failing after sometime with a null pointer exception

java.lang.NullPointerException
	at org.eclipse.jetty.server.SecureRequestCustomizer.customize(SecureRequestCustomizer.java:247)
	at org.eclipse.jetty.server.SecureRequestCustomizer.customize(SecureRequestCustomizer.java:205)
	at org.eclipse.jetty.server.HttpChannel.lambda$handle$0(HttpChannel.java:395)
	at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:656)
	at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:391)
	at org.eclipse.jetty.server.HttpChannel.run(HttpChannel.java:351)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:333)

Debugging code I found that the host is null in this code block in Jetty's org.eclipse.jetty.server.SecureRequestCustomizer:

String sniHost = (String)sslSession.getValue(SslContextFactory.Server.SNI_HOST);
@gregw
Copy link
Contributor

gregw commented Mar 21, 2021

@parmindersk there is something strange about your stacktrace and report.
For me line (SecureRequestCustomizer.java:247) is

246:            String sniHost = (String)sslSession.getValue(SslContextFactory.Server.SNI_HOST);
247:            X509 cert = new X509(null, (X509Certificate)sslSession.getLocalCertificates()[0]);
248:            String serverName = request.getServerName();

So line 247 could throw a NPE if sslSession was null, but if that was the case then line 246 would have thrown.
The other option is that getLocalCertificates() returns null.

If sniHost is null, it looks to me that it is null protected later on, so it should not throw a NPE.

Can you confirm what line 247 is in your version?

@sbordet does getLocalCertificates() returning null remind you of another issue?

@parmindersk
Copy link
Author

@gregw These are my versions:

        def JETTY_VER = "11.0.1"
	compile "org.eclipse.jetty:jetty-server:${JETTY_VER}"
	compile "org.eclipse.jetty:jetty-servlet:${JETTY_VER}"
	compile "org.eclipse.jetty.http2:http2-server:${JETTY_VER}"
	compile "org.eclipse.jetty:jetty-alpn-server:${JETTY_VER}"
	compile "org.eclipse.jetty:jetty-alpn-java-server:${JETTY_VER}"
	compile "org.eclipse.jetty:jetty-alpn-conscrypt-server:${JETTY_VER}"

Regarding code, the lines are on the same lines that you mention.

For now, I disabled the sniHostCheck and it has been working fine since then.

HttpConfiguration httpsConfig = new HttpConfiguration(httpConfig);
httpsConfig.addCustomizer(new SecureRequestCustomizer(false));

@gregw
Copy link
Contributor

gregw commented Mar 22, 2021

I've created PR #6078 to handle the only NPE I see possible there. The connection will still likely fail with, but with a 400 Bad Request response

gregw added a commit that referenced this issue Mar 22, 2021
Fix #6076 Protect from null local certificates

Signed-off-by: Greg Wilkins <gregw@webtide.com>
@sbordet
Copy link
Contributor

sbordet commented Mar 22, 2021

@gregw I don't understand how it was possible to receive a request on a TLS connection whose server-side did not send a certificate to the client.

I think your null check on the PR is unnecessary but a simple null check won't harm much.

I think the problem reported in this issue is a different one however.

@parmindersk can you reproduce the problem with full DEBUG logs enabled and attach the logs to this issue?

gregw added a commit that referenced this issue Mar 22, 2021
Updates from review
cache resulting X509 in session

Signed-off-by: Greg Wilkins <gregw@webtide.com>
gregw added a commit that referenced this issue Mar 23, 2021
Fix #6076 Protect from null local certificates
cache resulting X509 in session

Signed-off-by: Greg Wilkins <gregw@webtide.com>
@gregw
Copy link
Contributor

gregw commented Mar 23, 2021

This could be cherry picked back to jetty-9 once it proves stable in jetty-10

@gregw gregw reopened this Mar 23, 2021
@gregw
Copy link
Contributor

gregw commented May 31, 2021

9 already has NPE protection

@gregw gregw closed this as completed May 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants