Add KeyStore support in WebSocket, Function Worker HTTPS Servers #15084

michaeljmarshall · 2022-04-08T05:27:53Z

Motivation

We support configuring KeyStores for the broker and the proxy, but not the WebSocket or the Function Worker. By adding this support, users are able to provide KeyStores of type PCKS12 or JKS, which adds flexibility. Further, these KeyStores simplify support for additional algorithms because we rely on the TLS provider to load the KeyStore instead of loading keys ourselves.

Modifications

Add KeyStoreSSLContexts to the function worker server
Add KeyStoreSSLContexts to the web socket server
Add configurations to the function worker, the web socket, and the proxy configuration files to simply configuration
Rely on toString, not ObjectMapper, when converting the WorkerConfig to a string so that we don't log the KeyStore password. (Add a test to verify this logic. Note that we don't want the ObjectMapper to ignore the field because we use mappers when converting configuration classes.)

Verifying this change

I manually verified that this change works in a minikube cluster. The underlying method named KeyStoreSSLContext#createSslContextFactory is already used and tested, so I don't believe we need additional testing on that component.

Does this pull request potentially affect one of the following parts:

This change adds a new way to configure TLS in the WebSocket and Function Worker HTTPS Servers. As such, it adds new configuration. This configuration is named in the same way that the broker and proxy configuration is named, so it is consistent.

Documentation

I've documented the new configuration in the appropriate configuration files.

github-actions · 2022-04-08T05:28:14Z

@michaeljmarshall:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

eolivelli

great

lhotari

Good work @michaeljmarshall

michaeljmarshall · 2022-04-08T15:14:48Z

@nodece - you might be interested in this work, too. Note that using a KeyStore automatically supports additional algorithms, like ECDSA, for keys, and also adds support for loading many keys.

See apache#15084 for additional context.

nodece · 2022-04-09T09:31:21Z

...websocket/src/main/java/org/apache/pulsar/websocket/service/WebSocketProxyConfiguration.java

+                    + "When using TLS authentication with CACert, the valid value is either OPENSSL or JDK.\n"
+                    + "When using TLS authentication with KeyStore, available values can be SunJSSE, Conscrypt and etc."
+    )
+    private String tlsProvider = null;


Update this doc like available values can be SunJSSE, Conscrypt and etc.

...websocket/src/main/java/org/apache/pulsar/websocket/service/WebSocketProxyConfiguration.java

nodece

Thanks, this PR is great, I left my comments.

...websocket/src/main/java/org/apache/pulsar/websocket/service/WebSocketProxyConfiguration.java

pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/worker/WorkerConfig.java

lhotari

LGTM

michaeljmarshall · 2022-04-12T15:35:36Z

Rebased to resolve conflicts. I didn't make any code changes.

…che#15084) * Add KeyStore support in WebSocket, Function Worker HTTPS Servers * Avoid leaking worker config password * Fix checkstyle * Replace broker with appropriate text in annotations * Update python script for new configs Co-authored-by: Lari Hotari <lhotari@apache.org> We support configuring KeyStores for the broker and the proxy, but not the WebSocket or the Function Worker. By adding this support, users are able to provide KeyStores of type PCKS12 or JKS, which adds flexibility. Further, these KeyStores simplify support for additional algorithms because we rely on the TLS provider to load the KeyStore instead of loading keys ourselves. * Add `KeyStoreSSLContext`s to the function worker server * Add `KeyStoreSSLContext`s to the web socket server * Add configurations to the function worker, the web socket, and the proxy configuration files to simply configuration * Rely on `toString`, not `ObjectMapper`, when converting the `WorkerConfig` to a string so that we don't log the KeyStore password. (Add a test to verify this logic. Note that we don't want the `ObjectMapper` to ignore the field because we use mappers when converting configuration classes.) I manually verified that this change works in a minikube cluster. The underlying method named `KeyStoreSSLContext#createSslContextFactory` is already used and tested, so I don't believe we need additional testing on that component. This change adds a new way to configure TLS in the WebSocket and Function Worker HTTPS Servers. As such, it adds new configuration. This configuration is named in the same way that the broker and proxy configuration is named, so it is consistent. I've documented the new configuration in the appropriate configuration files. (cherry picked from commit a410396)

) * Add KeyStore support in WebSocket, Function Worker HTTPS Servers * Avoid leaking worker config password * Fix checkstyle * Replace broker with appropriate text in annotations * Update python script for new configs Co-authored-by: Lari Hotari <lhotari@apache.org> We support configuring KeyStores for the broker and the proxy, but not the WebSocket or the Function Worker. By adding this support, users are able to provide KeyStores of type PCKS12 or JKS, which adds flexibility. Further, these KeyStores simplify support for additional algorithms because we rely on the TLS provider to load the KeyStore instead of loading keys ourselves. * Add `KeyStoreSSLContext`s to the function worker server * Add `KeyStoreSSLContext`s to the web socket server * Add configurations to the function worker, the web socket, and the proxy configuration files to simply configuration * Rely on `toString`, not `ObjectMapper`, when converting the `WorkerConfig` to a string so that we don't log the KeyStore password. (Add a test to verify this logic. Note that we don't want the `ObjectMapper` to ignore the field because we use mappers when converting configuration classes.) I manually verified that this change works in a minikube cluster. The underlying method named `KeyStoreSSLContext#createSslContextFactory` is already used and tested, so I don't believe we need additional testing on that component. This change adds a new way to configure TLS in the WebSocket and Function Worker HTTPS Servers. As such, it adds new configuration. This configuration is named in the same way that the broker and proxy configuration is named, so it is consistent. I've documented the new configuration in the appropriate configuration files. (cherry picked from commit a410396)

…che#15084) * Add KeyStore support in WebSocket, Function Worker HTTPS Servers * Avoid leaking worker config password * Fix checkstyle * Replace broker with appropriate text in annotations * Update python script for new configs Co-authored-by: Lari Hotari <lhotari@apache.org> We support configuring KeyStores for the broker and the proxy, but not the WebSocket or the Function Worker. By adding this support, users are able to provide KeyStores of type PCKS12 or JKS, which adds flexibility. Further, these KeyStores simplify support for additional algorithms because we rely on the TLS provider to load the KeyStore instead of loading keys ourselves. * Add `KeyStoreSSLContext`s to the function worker server * Add `KeyStoreSSLContext`s to the web socket server * Add configurations to the function worker, the web socket, and the proxy configuration files to simply configuration * Rely on `toString`, not `ObjectMapper`, when converting the `WorkerConfig` to a string so that we don't log the KeyStore password. (Add a test to verify this logic. Note that we don't want the `ObjectMapper` to ignore the field because we use mappers when converting configuration classes.) I manually verified that this change works in a minikube cluster. The underlying method named `KeyStoreSSLContext#createSslContextFactory` is already used and tested, so I don't believe we need additional testing on that component. This change adds a new way to configure TLS in the WebSocket and Function Worker HTTPS Servers. As such, it adds new configuration. This configuration is named in the same way that the broker and proxy configuration is named, so it is consistent. I've documented the new configuration in the appropriate configuration files. (cherry picked from commit a410396) (cherry picked from commit e6b5ec9)

…che#15084) * Add KeyStore support in WebSocket, Function Worker HTTPS Servers * Avoid leaking worker config password * Fix checkstyle * Replace broker with appropriate text in annotations * Update python script for new configs Co-authored-by: Lari Hotari <lhotari@apache.org> We support configuring KeyStores for the broker and the proxy, but not the WebSocket or the Function Worker. By adding this support, users are able to provide KeyStores of type PCKS12 or JKS, which adds flexibility. Further, these KeyStores simplify support for additional algorithms because we rely on the TLS provider to load the KeyStore instead of loading keys ourselves. * Add `KeyStoreSSLContext`s to the function worker server * Add `KeyStoreSSLContext`s to the web socket server * Add configurations to the function worker, the web socket, and the proxy configuration files to simply configuration * Rely on `toString`, not `ObjectMapper`, when converting the `WorkerConfig` to a string so that we don't log the KeyStore password. (Add a test to verify this logic. Note that we don't want the `ObjectMapper` to ignore the field because we use mappers when converting configuration classes.) I manually verified that this change works in a minikube cluster. The underlying method named `KeyStoreSSLContext#createSslContextFactory` is already used and tested, so I don't believe we need additional testing on that component. This change adds a new way to configure TLS in the WebSocket and Function Worker HTTPS Servers. As such, it adds new configuration. This configuration is named in the same way that the broker and proxy configuration is named, so it is consistent. I've documented the new configuration in the appropriate configuration files. (cherry picked from commit a410396) (cherry picked from commit 55d41be)

michaeljmarshall self-assigned this Apr 8, 2022

github-actions bot added the doc-label-missing label Apr 8, 2022

michaeljmarshall added type/feature The PR added a new feature or issue requested a new feature area/function area/websocket area/security doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Apr 8, 2022

michaeljmarshall requested review from addisonj, codelipenghui, eolivelli, hezhangjian, jiazhai, lhotari, merlimat and sijie April 8, 2022 05:28

michaeljmarshall changed the title ~~Add more keystore support~~ Add KeyStore support in WebSocket, Function Worker HTTPS Servers Apr 8, 2022

eolivelli approved these changes Apr 8, 2022

View reviewed changes

lhotari approved these changes Apr 8, 2022

View reviewed changes

nicoloboschi approved these changes Apr 8, 2022

View reviewed changes

michaeljmarshall added a commit to datastax/pulsar that referenced this pull request Apr 8, 2022

Add KeyStore support in WebSocket, Function Worker HTTPS Servers

b6d1967

See apache#15084 for additional context.