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

[#41] feat(server): Add https support for Jetty server #860

Merged
merged 12 commits into from
Dec 5, 2023
Merged

[#41] feat(server): Add https support for Jetty server #860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
d2455cd
[#41] server: Add https support for Jetty Server
Nov 28, 2023
3244424
Address comments
Nov 30, 2023
8db1af3
fix comments
Nov 30, 2023
0627f52
Address comments
Dec 2, 2023
a9bc4ff
fix
Dec 4, 2023
a5b36c4
fix
Dec 4, 2023
25e78fb
fix
Dec 4, 2023
f2b46f2
fix
Dec 4, 2023
bbf3760
fix
Dec 4, 2023
3f083d4
fix
Dec 4, 2023
3666dbc
fix
Dec 4, 2023
9dc2780
Fix markdown
Dec 5, 2023
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
41 changes: 41 additions & 0 deletions docs/security.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,44 @@ The signature algorithm which Gravitino supports is as below:
| PS256 | RSASSA-PSS using SHA-256 and MGF1 with SHA-256 |
| PS384 | RSASSA-PSS using SHA-384 and MGF1 with SHA-384 |
| PS512 | RSASSA-PSS using SHA-512 and MGF1 with SHA-512 |

## HTTPS configuration
Users would better use HTTPS instead of HTTP if users choose OAuth 2.0 as the authenticator.
Because HTTPS will protect the header of request from smuggling and HTTPS will be safer.
If users choose to enable HTTPS, Gravitino won't provide the ability of HTTP service.
Both Gravitino server and Iceberg REST service can configure HTTPS.

### Gravitino server's configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

Gravitino server and Iceberg REST server have it's configuration doc, I wonder whether it's proper to put security configuration here.

Copy link
Contributor Author

@qqqttt123 qqqttt123 Dec 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It's ok for us not to add these configuration options in those documents. Spark also follows this document style.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should add more doc example about how to configure https.

| Configuration item | Description | Default value | Since version |
|-----------------------------------------------------|------------------------------------------------------------|---------------|---------------|
| `gravitino.server.webserver.enableHttps` | Enables https | `false` | 0.3.0 |
| `gravitino.server.webserver.httpsPort` | The https port number of the Jetty web server | `8433` | 0.3.0 |
| `gravitino.server.webserver.keyStorePath` | Path to the key store file | `` | 0.3.0 |
| `gravitino.server.webserver.keyStorePassword` | Password to the key store | `` | 0.3.0 |
| `gravitino.server.webserver.keyStoreType` | The type to the key store | `JKS` | 0.3.0 |
| `gravitino.server.webserver.managerPassword` | Manager password to the key store | `` | 0.3.0 |
| `gravitino.server.webserver.tlsProtocol` | TLS protocol to use. The protocol must be supported by JVM | none | 0.3.0 |
Copy link
Contributor

Choose a reason for hiding this comment

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

How do you configure this? and algorithm, it would be better to add more docs about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I will add more description referring the doc of Spark.

| `gravitino.server.webserver.enableCipherAlgorithms` | The collection of the cipher algorithms which are enabled. | `` | 0.3.0 |
| `gravitino.server.webserver.enableClientAuth` | Enables the authentication of the client | `false` | 0.3.0 |
| `gravitino.server.webserver.trustStorePath` | Path to the trust store file | `` | 0.3.0 |
| `gravitino.server.webserver.trustStorePassword` | Password to the trust store | `` | 0.3.0 |
| `gravitino.server.webserver.trustStoreType` | The type to the trust store | `JKS` | 0.3.0 |

### Iceberg REST service's configuration
| Configuration item | Description | Default value | Since version |
|------------------------------------------------------------|------------------------------------------------------------|---------------|---------------|
| `gravitino.auxService.iceberg-rest.enableHttps` | Enables https | `false` | 0.3.0 |
| `gravitino.auxService.iceberg-rest.httpsPort` | The https port number of the Jetty web server | `8433` | 0.3.0 |
| `gravitino.auxService.iceberg-rest.keyStorePath` | Path to the key store file | `` | 0.3.0 |
| `gravitino.auxService.iceberg-rest.keyStorePassword` | Password to the key store | `` | 0.3.0 |
| `gravitino.uxService.iceberg-rest.keyStoreType` | The type to the key store | `JKS` | 0.3.0 |
| `gravitino.auxService.iceberg-rest.managerPassword` | Manager password to the key store | `` | 0.3.0 |
| `gravitino.auxService.iceberg-rest.tlsProtocol` | TLS protocol to use. The protocol must be supported by JVM | none | 0.3.0 |
| `gravitino.auxService.iceberg-rest.enableCipherAlgorithms` | The collection of the cipher algorithms which are enabled | `` | 0.3.0 |
| `gravitino.auxService.iceberg-rest.enableClientAuth` | Enables the authentication of the client | `false` | 0.3.0 |
| `gravitino.auxService.iceberg-rest.trustStorePath` | Path to the trust store file | `` | 0.3.0 |
| `gravitino.auxService.iceberg-rest.trustStorePassword` | Password to the trust store | `` | 0.3.0 |
| `gravitino.auxService.iceberg-rest.trustStoreType` | The type to the trust store | `JKS` | 0.3.0 |

About `tlsProtocol`, the reference list of protocols can be found in the "Additional JSSE Standard Names" section of the Java security guide. The list for Java 8 can be found at [this](https://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html#jssenames).
About `enableCipherAlgorithms`, the reference list of protocols can be found in the "JSSE Cipher Suite Names" section of the Java security guide. The list for Java 8 can be found at [this](https://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html#ciphersuites)
1 change: 1 addition & 0 deletions ...t/java/com/datastrato/gravitino/integration/test/web/rest/AuthenticationOperationsIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public static void startIntegrationTest() throws Exception {
configs.put(OAuthConfig.DEFAULT_SIGN_KEY.getKey(), publicKey);
configs.put(OAuthConfig.ALLOW_SKEW_SECONDS.getKey(), "6");
configs.put(AuthConstants.HTTP_HEADER_AUTHORIZATION, token);

registerCustomConfigs(configs);
AbstractIT.startIntegrationTest();
}
Expand Down
149 changes: 130 additions & 19 deletions server-common/src/main/java/com/datastrato/gravitino/server/web/JettyServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.codahale.metrics.servlets.MetricsServlet;
import com.datastrato.gravitino.GravitinoEnv;
import com.datastrato.gravitino.metrics.MetricsSystem;
import com.google.common.base.Preconditions;
import java.io.File;
import java.io.IOException;
import java.net.BindException;
Expand All @@ -16,22 +17,28 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.EnumSet;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.LinkedBlockingQueue;
import javax.servlet.DispatcherType;
import javax.servlet.Filter;
import javax.servlet.Servlet;
import org.apache.commons.lang3.StringUtils;
import org.eclipse.jetty.server.ConnectionFactory;
import org.eclipse.jetty.server.HttpConfiguration;
import org.eclipse.jetty.server.HttpConnectionFactory;
import org.eclipse.jetty.server.SecureRequestCustomizer;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.SslConnectionFactory;
import org.eclipse.jetty.server.handler.ErrorHandler;
import org.eclipse.jetty.server.handler.HandlerCollection;
import org.eclipse.jetty.servlet.DefaultServlet;
import org.eclipse.jetty.servlet.FilterHolder;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.eclipse.jetty.util.component.LifeCycle;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.eclipse.jetty.util.thread.QueuedThreadPool;
import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler;
import org.eclipse.jetty.util.thread.Scheduler;
Expand All @@ -44,6 +51,9 @@ public final class JettyServer {

private static final Logger LOG = LoggerFactory.getLogger(JettyServer.class);

private static final String HTTPS = "https";
private static final String HTTP_PROTOCOL = "http/1.1";

private Server server;

private ServletContextHandler servletContextHandler;
Expand Down Expand Up @@ -76,18 +86,56 @@ public synchronized void initialize(
errorHandler.setServer(server);
server.addBean(errorHandler);

// Create and set Http ServerConnector
ServerConnector httpConnector =
createHttpServerConnector(
server,
serverConfig.getRequestHeaderSize(),
serverConfig.getResponseHeaderSize(),
serverConfig.getHost(),
serverConfig.getHttpPort(),
serverConfig.getIdleTimeout());
server.addConnector(httpConnector);

// TODO. Create and set https connector @jerry
if (serverConfig.isEnableHttps()) {
// Create and set Https ServerConnector
Preconditions.checkArgument(
StringUtils.isNotBlank(serverConfig.getKeyStorePath()),
"If enables https, must set keyStorePath");
Preconditions.checkArgument(
StringUtils.isNotBlank(serverConfig.getKeyStorePassword()),
"If enables https, must set keyStorePassword");
Preconditions.checkArgument(
StringUtils.isNotBlank(serverConfig.getManagerPassword()),
"If enables https, must set managerPassword");
if (serverConfig.isEnableClientAuth()) {
Preconditions.checkArgument(
StringUtils.isNotBlank(serverConfig.getTrustStorePath()),
"If enables the authentication of the client, must set trustStorePath");
Preconditions.checkArgument(
StringUtils.isNotBlank(serverConfig.getTrustStorePasword()),
"If enables the authentication of the client, must set trustStorePassword");
}
ServerConnector httpsConnector =
createHttpsServerConnector(
server,
serverConfig.getRequestHeaderSize(),
serverConfig.getResponseHeaderSize(),
serverConfig.getHost(),
serverConfig.getHttpsPort(),
serverConfig.getIdleTimeout(),
serverConfig.getKeyStorePath(),
serverConfig.getKeyStorePassword(),
serverConfig.getManagerPassword(),
serverConfig.getKeyStoreType(),
serverConfig.getTlsProtocol(),
serverConfig.getSupportedAlgorithms(),
serverConfig.isEnableClientAuth(),
serverConfig.getTrustStorePath(),
serverConfig.getTrustStorePasword(),
serverConfig.getTrustStoreType());
server.addConnector(httpsConnector);
} else {
// Create and set Http ServerConnector
ServerConnector httpConnector =
createHttpServerConnector(
server,
serverConfig.getRequestHeaderSize(),
serverConfig.getResponseHeaderSize(),
serverConfig.getHost(),
serverConfig.getHttpPort(),
serverConfig.getIdleTimeout());
server.addConnector(httpConnector);
}

// Initialize ServletContextHandler or WebAppContext
if (shouldEnableUI) {
Expand Down Expand Up @@ -127,11 +175,12 @@ public synchronized void start() throws RuntimeException {
throw new RuntimeException("Failed to start " + serverName + " web server.", e);
}

if (!serverConfig.isEnableHttps()) {
LOG.warn("Users would better use HTTPS to void token data leak.");
}

LOG.info(
"{} web server started on host {} port {}.",
serverName,
serverConfig.getHost(),
serverConfig.getHttpPort());
"{} web server started on host {} port {}.", serverName, serverConfig.getHost(), getPort());
}

public synchronized void join() {
Expand Down Expand Up @@ -162,7 +211,7 @@ public synchronized void stop() {
"{} web server stopped on host {} port {}.",
serverName,
serverConfig.getHost(),
serverConfig.getHttpPort());
getPort());
} catch (Exception e) {
// Swallow the exception.
LOG.warn("Failed to stop {} web server.", serverName, e);
Expand Down Expand Up @@ -260,15 +309,77 @@ private ServerConnector createHttpServerConnector(

HttpConnectionFactory httpConnectionFactory = new HttpConnectionFactory(httpConfig);
ServerConnector connector =
creatorServerConnector(server, new ConnectionFactory[] {httpConnectionFactory});
createServerConnector(server, new ConnectionFactory[] {httpConnectionFactory});
connector.setHost(host);
connector.setPort(port);
connector.setReuseAddress(true);

return connector;
}

private ServerConnector creatorServerConnector(
private int getPort() {
if (serverConfig.isEnableHttps()) {
return serverConfig.getHttpsPort();
} else {
return serverConfig.getHttpPort();
}
}

private ServerConnector createHttpsServerConnector(
Server server,
int reqHeaderSize,
int respHeaderSize,
String host,
int port,
int idleTimeout,
String keyStorePath,
String keyStorePassword,
String keyManagerPassword,
String keyStoreType,
Optional<String> tlsProtocol,
Set<String> supportedAlgorithms,
boolean isEnableClientAuth,
String trustStorePath,
String trustStorePassword,
String trustStoreType) {
HttpConfiguration httpConfig = new HttpConfiguration();
httpConfig.setSecureScheme(HTTPS);
httpConfig.setRequestHeaderSize(reqHeaderSize);
httpConfig.setResponseHeaderSize(respHeaderSize);
httpConfig.setSendServerVersion(true);
httpConfig.setIdleTimeout(idleTimeout);
httpConfig.setSecurePort(port);

SslContextFactory sslContextFactory = new SslContextFactory.Server();
sslContextFactory.setKeyStorePath(keyStorePath);
sslContextFactory.setKeyStorePassword(keyStorePassword);
sslContextFactory.setKeyManagerPassword(keyManagerPassword);
sslContextFactory.setKeyStoreType(keyStoreType);
tlsProtocol.ifPresent(sslContextFactory::setProtocol);
if (!supportedAlgorithms.isEmpty()) {
sslContextFactory.setIncludeCipherSuites(supportedAlgorithms.toArray(new String[0]));
}
if (isEnableClientAuth) {
sslContextFactory.setNeedClientAuth(true);
jerryshao marked this conversation as resolved.
Show resolved Hide resolved
sslContextFactory.setTrustStorePath(trustStorePath);
sslContextFactory.setTrustStorePassword(trustStorePassword);
sslContextFactory.setTrustStoreType(trustStoreType);
}
SecureRequestCustomizer src = new SecureRequestCustomizer();
httpConfig.addCustomizer(src);
HttpConnectionFactory httpConnectionFactory = new HttpConnectionFactory(httpConfig);
SslConnectionFactory sslConnectionFactory =
new SslConnectionFactory(sslContextFactory, HTTP_PROTOCOL);
ServerConnector connector =
createServerConnector(
server, new ConnectionFactory[] {sslConnectionFactory, httpConnectionFactory});
connector.setHost(host);
connector.setPort(port);
connector.setReuseAddress(true);
return connector;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

How to handle exceptions on https initialize. Would it be easy for the user to understand if the original exception is thrown?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What exception should we handle?

private ServerConnector createServerConnector(
Server server, ConnectionFactory[] connectionFactories) {
Scheduler serverExecutor =
new ScheduledExecutorScheduler(serverName + "-webserver-JettyScheduler", true);
Expand Down
Loading