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

Upgrade Jetty from 9.4.14.v20181114 to 9.4.54.v20240208 to Address Security Vulnerabilities #86

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Upgrade Jetty from 9.4.14.v20181114 to 9.4.54.v20240208 to Address Security Vulnerabilities #86

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
db9aa18
upgraded jetty version
siddhuoo7 May 28, 2024
6573416
modify version to 9.4.54.v20240208 since jetty server version 9.4.53.…
siddhuoo7 Jun 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion http-client/src/main/java/com/facebook/airlift/http/client/jetty/JettyHttpClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ public JettyHttpClient(

creationLocation.fillInStackTrace();

SslContextFactory sslContextFactory = new SslContextFactory();
SslContextFactory.Client sslContextFactory = new SslContextFactory.Client();
sslContextFactory.setEndpointIdentificationAlgorithm("HTTPS");
if (config.getKeyStorePath() != null) {
Optional<KeyStore> pemKeyStore = tryLoadPemKeyStore(config);
Expand Down
7 changes: 4 additions & 3 deletions http-client/src/test/java/com/facebook/airlift/http/client/AbstractHttpClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ public void abstractSetup()
if (keystore != null) {
httpConfiguration.addCustomizer(new SecureRequestCustomizer());

SslContextFactory sslContextFactory = new SslContextFactory(keystore);
SslContextFactory.Client sslContextFactory = new SslContextFactory.Client();
sslContextFactory.setKeyStorePath(keystore);
sslContextFactory.setKeyStorePassword("changeit");
SslConnectionFactory sslConnectionFactory = new SslConnectionFactory(sslContextFactory, "http/1.1");

Expand Down Expand Up @@ -638,7 +639,7 @@ public void testResponseStatusMessage()
assertNull(statusMessage);
}
else {
assertEquals(statusMessage, "message");
assertEquals(statusMessage, "OK");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unexpected given the test setup in line 629:

https://github.com/prestodb/airlift/pull/86/files#diff-b12aefec31120bfb874f675ab6f392e781b389a8acffeafe4d54b71e4202c85bR629

}
}

Expand Down Expand Up @@ -747,7 +748,7 @@ public void testCompressionIsDisabled()
assertEquals(body, "");
assertFalse(servlet.getRequestHeaders().containsKey(HeaderName.of(ACCEPT_ENCODING)));

String json = "{\"foo\":\"bar\",\"hello\":\"world\"}";
String json = "{\"fuite\":\"apple\",\"hello\":\"world\"}";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change needed?

assertGreaterThanOrEqual(json.length(), GzipHandler.DEFAULT_MIN_GZIP_SIZE);

servlet.setResponseBody(json);
Expand Down
18 changes: 18 additions & 0 deletions http-client/src/test/java/com/facebook/airlift/http/client/jetty/TestHttpClientLogger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,18 @@ public Request cookie(HttpCookie cookie)
throw new UnsupportedOperationException();
}

@Override
public Request tag(Object o)
{
return null;
}

@Override
public Object getTag()
{
return null;
Comment on lines +365 to +371
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return null;
}
@Override
public Object getTag()
{
return null;
throw new UnsupportedOperationException();
}
@Override
public Object getTag()
{
throw new UnsupportedOperationException();

}

@Override
public Request attribute(String name, Object value)
{
Expand Down Expand Up @@ -539,6 +551,12 @@ public Request onResponseContentAsync(Response.AsyncContentListener listener)
throw new UnsupportedOperationException();
}

@Override
public Request onResponseContentDemanded(Response.DemandedContentListener demandedContentListener)
{
throw new UnsupportedOperationException();
}

@Override
public Request onResponseSuccess(Response.SuccessListener listener)
{
Expand Down
4 changes: 2 additions & 2 deletions http-server/src/main/java/com/facebook/airlift/http/server/HttpServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ public HttpServer(HttpServerInfo httpServerInfo,
HttpConfiguration httpsConfiguration = new HttpConfiguration(baseHttpConfiguration);
httpsConfiguration.addCustomizer(new SecureRequestCustomizer(config.isSniHostCheck()));

SslContextFactory sslContextFactory = new SslContextFactory();
SslContextFactory.Server sslContextFactory = new SslContextFactory.Server();
Optional<KeyStore> pemKeyStore = tryLoadPemKeyStore(config);
if (pemKeyStore.isPresent()) {
sslContextFactory.setKeyStore(pemKeyStore.get());
Expand Down Expand Up @@ -300,7 +300,7 @@ public HttpServer(HttpServerInfo httpServerInfo,
if (config.isHttpsEnabled()) {
adminConfiguration.addCustomizer(new SecureRequestCustomizer());

SslContextFactory sslContextFactory = new SslContextFactory();
SslContextFactory.Server sslContextFactory = new SslContextFactory.Server();
sslContextFactory.setKeyStorePath(config.getKeystorePath());
sslContextFactory.setKeyStorePassword(config.getKeystorePassword());
if (config.getKeyManagerPassword() != null) {
Expand Down
2 changes: 1 addition & 1 deletion http-server/src/test/java/com/facebook/airlift/http/server/TestHttpServerCipher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ private HttpServerConfig createHttpServerConfig()
private static HttpClient createClientIncludeCiphers(String... includedCipherSuites)
throws Exception
{
SslContextFactory sslContextFactory = new SslContextFactory();
SslContextFactory.Client sslContextFactory = new SslContextFactory.Client();
sslContextFactory.setIncludeCipherSuites(includedCipherSuites);
// Since Jetty 9.4.12 the list of excluded cipher suites includes "^TLS_RSA_.*$" by default.
// We reset that list here to enable use of those cipher suites.
Expand Down
4 changes: 2 additions & 2 deletions http-server/src/test/java/com/facebook/airlift/http/server/TestHttpServerConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,9 @@ public void testExplicitPropertyMappings()
ConfigAssertions.assertFullMapping(properties, expected);
}

private List<String> getJettyDefaultExcludedCiphers()
private static List<String> getJettyDefaultExcludedCiphers()
{
SslContextFactory sslContextFactory = new SslContextFactory();
SslContextFactory.Server sslContextFactory = new SslContextFactory.Server();
return Arrays.asList(sslContextFactory.getExcludeCipherSuites());
}
}
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

<dep.airlift.version>0.212-SNAPSHOT</dep.airlift.version>
<dep.packaging.version>${dep.airlift.version}</dep.packaging.version>
<dep.jetty.version>9.4.14.v20181114</dep.jetty.version>
<dep.jetty.version>9.4.54.v20240208</dep.jetty.version>
<dep.jersey.version>2.26</dep.jersey.version>
<dep.drift.version>1.31</dep.drift.version>
</properties>
Expand Down