Skip to content

feat(server): Add a KeyStoreScanner #134

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

Open
ShahimSharafudeen wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(server): Add a KeyStoreScanner #134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 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 @@ -52,6 +52,7 @@
import org.eclipse.jetty.server.handler.ContextHandlerCollection;
import org.eclipse.jetty.server.handler.StatisticsHandler;
import org.eclipse.jetty.server.handler.gzip.GzipHandler;
import org.eclipse.jetty.util.ssl.KeyStoreScanner;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.eclipse.jetty.util.thread.QueuedThreadPool;
import org.eclipse.jetty.util.thread.Scheduler;
Expand Down Expand Up @@ -245,6 +246,9 @@ public HttpServer(HttpServerInfo httpServerInfo,
if (config.getKeyManagerPassword() != null) {
sslContextFactory.setKeyManagerPassword(config.getKeyManagerPassword());
}
KeyStoreScanner keyStoreScanner = new KeyStoreScanner(sslContextFactory);
Copy link
Copy Markdown

@aaneja aaneja Nov 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Should we add a similar scanner to the while creating theadminConnector on line 329 ?

Copy link
Copy Markdown
Author

@ShahimSharafudeen ShahimSharafudeen Nov 17, 2025

Choose a reason for hiding this comment

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

Added

keyStoreScanner.setScanInterval(config.getKeystoreScanInterval());
server.addBean(keyStoreScanner);
}
if (config.getTrustStorePath() != null) {
Optional<KeyStore> pemTrustStore = tryLoadPemTrustStore(config);
Expand Down Expand Up @@ -329,6 +333,9 @@ public HttpServer(HttpServerInfo httpServerInfo,
-1,
sslConnectionFactory,
new HttpConnectionFactory(adminConfiguration));
KeyStoreScanner keyStoreScanner = new KeyStoreScanner(sslContextFactory);
keyStoreScanner.setScanInterval(config.getKeystoreScanInterval());
adminConnector.addBean(keyStoreScanner);
}
else {
HttpConnectionFactory http1 = new HttpConnectionFactory(adminConfiguration);
Expand Down
15 changes: 15 additions & 0 deletions http-server/src/main/java/com/facebook/airlift/http/server/HttpServerConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ public enum AuthorizationPolicy
private Set<String> defaultAllowedRoles = ImmutableSet.of();
private boolean allowUnsecureRequestsInAuthorizer;

private int keyStoreScanIntervalSeconds;

public boolean isHttpEnabled()
{
return httpEnabled;
Expand Down Expand Up @@ -823,4 +825,17 @@ public List<HttpComplianceViolation> getHttpComplianceViolations()
{
return httpComplianceViolations;
}

@Config("http-server.https.keystore.scan-interval-seconds")
@ConfigDescription("Interval (in seconds) at which the server checks for updates to the HTTPS keystore file")
public HttpServerConfig setKeystoreScanInterval(int keyStoreScanIntervalSeconds)
Copy link
Copy Markdown

@ZacBlanco ZacBlanco Nov 17, 2025

Choose a reason for hiding this comment

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

I think it would make more sense if this was a configuration value of Duration type instead of an integer. It will be easier to configure. Especially since the current iteration of all the method names omit "seconds".

Copy link
Copy Markdown

@aaneja aaneja Nov 18, 2025

Choose a reason for hiding this comment

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

The actual Jetty method takes seconds only unfortunatley. Using a fractional duration rounded down to seconds may be confusing to users

Copy link
Copy Markdown

@aaneja aaneja Nov 19, 2025

Choose a reason for hiding this comment

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

Reltated - https://javadoc.jetty.org/jetty-9/org/eclipse/jetty/util/Scanner.html#setScanInterval(int)

Copy link
Copy Markdown

@ZacBlanco ZacBlanco Nov 19, 2025

Choose a reason for hiding this comment

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

Thanks. Makes sense. I would just recommend making the config name clearer that it accepts a value in seconds (rather than just the description)

Copy link
Copy Markdown
Author

@ShahimSharafudeen ShahimSharafudeen Nov 20, 2025

Choose a reason for hiding this comment

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

Done

{
this.keyStoreScanIntervalSeconds = keyStoreScanIntervalSeconds;
return this;
}

public int getKeystoreScanInterval()
{
return keyStoreScanIntervalSeconds;
}
}
7 changes: 5 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 @@ -90,7 +90,8 @@ public void testDefaults()
.setAllowUnsecureRequestsInAuthorizer(false)
.setSniHostCheck(true)
.setUriComplianceMode(UriCompliance.DEFAULT)
.setHttpComplianceViolations(""));
.setHttpComplianceViolations("")
.setKeystoreScanInterval(0));
}

@Test
Expand Down Expand Up @@ -149,6 +150,7 @@ public void testExplicitPropertyMappings()
.put("http-server.https.sni-host-check", "false")
.put("http-server.uri-compliance.mode", "LEGACY")
.put("http-server.http-compliance.violations", "UNSAFE_HOST_HEADER,MISMATCHED_AUTHORITY")
.put("http-server.https.keystore.scan-interval-seconds", "2")
.build();

HttpServerConfig expected = new HttpServerConfig()
Expand Down Expand Up @@ -203,7 +205,8 @@ public void testExplicitPropertyMappings()
.setAllowUnsecureRequestsInAuthorizer(true)
.setSniHostCheck(false)
.setUriComplianceMode(UriCompliance.LEGACY)
.setHttpComplianceViolations("UNSAFE_HOST_HEADER,MISMATCHED_AUTHORITY");
.setHttpComplianceViolations("UNSAFE_HOST_HEADER,MISMATCHED_AUTHORITY")
.setKeystoreScanInterval(2);

ConfigAssertions.assertFullMapping(properties, expected);
}
Expand Down
47 changes: 42 additions & 5 deletions http-server/src/test/java/com/facebook/airlift/http/server/TestHttpServerProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,16 @@
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.BeforeSuite;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

import java.io.File;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.net.URI;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.security.cert.X509Certificate;
import java.util.Base64;
import java.util.concurrent.ExecutionException;
Expand Down Expand Up @@ -82,6 +85,12 @@ public void setupSuite()
Logging.initialize();
}

@DataProvider(name = "adminMode")
public static Object[][] adminMode()
{
return new Object[][] {{true}, {false}};
}

@BeforeMethod
public void setup()
throws IOException
Expand Down Expand Up @@ -310,16 +319,24 @@ public void testAuth()
}
}

@Test
public void testClientCertificateJava()
@Test(dataProvider = "adminMode")
public void testClientCertificateJava(boolean adminMode)
throws Exception
{
tempDir = createTempDirectory("test-keystore").toFile().getCanonicalFile();
String tempKeyStorePath = tempDir.toPath().resolve("server.keystore").toAbsolutePath().toString();
String originalKeyStorePath = getResource("clientcert-java/server.keystore").getPath();
String replacementKeyStorePath = getResource("clientcert-java/replacementServer.keystore").getPath();

Files.copy(Path.of(originalKeyStorePath), Path.of(tempKeyStorePath), StandardCopyOption.REPLACE_EXISTING);

config.setHttpEnabled(false)
.setAdminEnabled(false)
.setAdminEnabled(adminMode)
.setHttpsEnabled(true)
.setHttpsPort(0)
.setKeystorePath(getResource("clientcert-java/server.keystore").getPath())
.setKeystorePassword("airlift");
.setKeystorePath(tempKeyStorePath)
.setKeystorePassword("airlift")
.setKeystoreScanInterval(1);

createAndStartServer(createCertTestServlet());

Expand All @@ -337,6 +354,26 @@ public void testClientCertificateJava()
assertEquals(response.getStatusCode(), HttpServletResponse.SC_OK);
assertEquals(response.getBody(), "CN=testing,OU=Client,O=Airlift,L=Palo Alto,ST=CA,C=US");
}

Files.copy(Path.of(replacementKeyStorePath), Path.of(tempKeyStorePath), StandardCopyOption.REPLACE_EXISTING);

// Wait for the KeyStoreScanner to detect the file change. Scan interval is 1 second, so sleeping for 2.5 seconds
Thread.sleep(2500);

HttpClientConfig clientConfig2 = new HttpClientConfig()
.setKeyStorePath(getResource("clientcert-java/replacementClient.keystore").getPath())
.setKeyStorePassword("airlift")
.setTrustStorePath(getResource("clientcert-java/replacementClient.truststore").getPath())
.setTrustStorePassword("airlift");

try (JettyHttpClient httpClient = new JettyHttpClient(clientConfig2)) {
StringResponse response = httpClient.execute(
prepareGet().setUri(httpServerInfo.getHttpsUri()).build(),
createStringResponseHandler());

assertEquals(response.getStatusCode(), HttpServletResponse.SC_OK);
assertEquals(response.getBody(), "CN=testing,OU=Client,O=Replacement,L=Palo Alto,ST=CA,C=US");
}
}

@Test
Expand Down
20 changes: 20 additions & 0 deletions http-server/src/test/resources/clientcert-java/clientcert.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,23 @@ openssl pkcs12 -name client -inkey client.key -in client.crt -export -passout pa
# create client truststore
keytool -import -noprompt -alias ca -file ca.crt -storetype pkcs12 -storepass airlift -keystore client.truststore


#A kesytore for testing the KeyStoreScanner's ability to transparently update the ssl config without needing a restart

# create replacement server cert
openssl req -new -key server.key -subj "/C=US/ST=CA/L=Palo Alto/O=Replacement/OU=Server/CN=localhost" -out replacementServer.csr
openssl x509 -req -days 9999 -in replacementServer.csr -CA ca.crt -CAkey ca.key -set_serial 01 -out replacementServer.crt

# create replacement server keystore
openssl pkcs12 -name server -inkey server.key -in replacementServer.crt -export -passout pass:airlift -out replacementServer.keystore
keytool -import -noprompt -alias ca -file ca.crt -storetype pkcs12 -storepass airlift -keystore replacementServer.keystore

# create client cert
openssl req -new -key client.key -subj "/C=US/ST=CA/L=Palo Alto/O=Replacement/OU=Client/CN=testing" -out replacementClient.csr
openssl x509 -req -days 9999 -in replacementClient.csr -CA ca.crt -CAkey ca.key -set_serial 02 -out replacementClient.crt

# create replacement client keystore
openssl pkcs12 -name client -inkey client.key -in replacementClient.crt -export -passout pass:airlift -out replacementClient.keystore

# create replacement client truststore
keytool -import -noprompt -alias ca -file ca.crt -storetype pkcs12 -storepass airlift -keystore replacementClient.truststore
Binary file added http-server/src/test/resources/clientcert-java/replacementClient.keystore
View file
Open in desktop
Binary file not shown.
Binary file added http-server/src/test/resources/clientcert-java/replacementClient.truststore
View file
Open in desktop
Binary file not shown.
Binary file added http-server/src/test/resources/clientcert-java/replacementServer.keystore
View file
Open in desktop
Binary file not shown.
Loading