[GitHub] rdhabalia commented on a change in pull request #1225: Enable specification of TLS Protocol Versions and Cipher Suites
rdhabalia commented on a change in pull request #1225: Enable specification of
TLS Protocol Versions and Cipher Suites
URL: https://github.com/apache/incubator-pulsar/pull/1225#discussion_r167724421
##
File path:
pulsar-common/src/main/java/org/apache/pulsar/common/util/SecurityUtility.java
##
@@ -95,12 +97,22 @@ public static SslContext
createNettySslContextForClient(boolean allowInsecureCon
}
public static SslContext createNettySslContextForServer(boolean
allowInsecureConnection, String trustCertsFilePath,
-String certFilePath, String keyFilePath)
+String certFilePath, String keyFilePath, Set ciphers,
Set protocols)
throws GeneralSecurityException, SSLException,
FileNotFoundException {
X509Certificate[] certificates =
loadCertificatesFromPemFile(certFilePath);
PrivateKey privateKey = loadPrivateKeyFromPemFile(keyFilePath);
SslContextBuilder builder = SslContextBuilder.forServer(privateKey,
(X509Certificate[]) certificates);
+if (ciphers != null && ciphers.size() > 0) {
+builder.ciphers(ciphers);
+}
+
+if (protocols != null && protocols.size() > 0) {
+String[] protocolsArray = new String[protocols.size()];
Review comment:
`builder.protocols(Lists.newArrayList(protocols));` ??
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] rdhabalia commented on a change in pull request #1225: Enable specification of TLS Protocol Versions and Cipher Suites
rdhabalia commented on a change in pull request #1225: Enable specification of
TLS Protocol Versions and Cipher Suites
URL: https://github.com/apache/incubator-pulsar/pull/1225#discussion_r167724421
##
File path:
pulsar-common/src/main/java/org/apache/pulsar/common/util/SecurityUtility.java
##
@@ -95,12 +97,22 @@ public static SslContext
createNettySslContextForClient(boolean allowInsecureCon
}
public static SslContext createNettySslContextForServer(boolean
allowInsecureConnection, String trustCertsFilePath,
-String certFilePath, String keyFilePath)
+String certFilePath, String keyFilePath, Set ciphers,
Set protocols)
throws GeneralSecurityException, SSLException,
FileNotFoundException {
X509Certificate[] certificates =
loadCertificatesFromPemFile(certFilePath);
PrivateKey privateKey = loadPrivateKeyFromPemFile(keyFilePath);
SslContextBuilder builder = SslContextBuilder.forServer(privateKey,
(X509Certificate[]) certificates);
+if (ciphers != null && ciphers.size() > 0) {
+builder.ciphers(ciphers);
+}
+
+if (protocols != null && protocols.size() > 0) {
+String[] protocolsArray = new String[protocols.size()];
Review comment:
`builder.protocols(Lists.newArrayList(protocols));` ??
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] rdhabalia commented on a change in pull request #1225: Enable specification of TLS Protocol Versions and Cipher Suites
rdhabalia commented on a change in pull request #1225: Enable specification of
TLS Protocol Versions and Cipher Suites
URL: https://github.com/apache/incubator-pulsar/pull/1225#discussion_r167744955
##
File path:
pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithProxyAuthorizationTest.java
##
@@ -306,6 +360,79 @@ public void textTlsHostVerificationProxyToBroker(boolean
hostnameVerificationEna
log.info("-- Exiting {} test --", methodName);
}
+/*
+ * This test verifies whether the Client and Proxy honor the protocols and
ciphers specified.
+ * Details description of test cases can be found in
protocolsCiphersProviderCodecProvider
+ */
+@Test(dataProvider = "protocolsCiphersProvider")
+public void tlsCiphersAndProtocols(Set tlsCiphers, Set
tlsProtocols, boolean expectFailure) throws Exception {
+log.info("-- Starting {} test --", methodName);
+String namespaceName = "my-property/proxy-authorization/my-ns";
+createAdminClient();
+
+admin.properties().createProperty("my-property",
+new PropertyAdmin(Lists.newArrayList("appid1", "appid2"),
Sets.newHashSet("proxy-authorization")));
+admin.namespaces().createNamespace(namespaceName);
+
+admin.namespaces().grantPermissionOnNamespace(namespaceName, "Proxy",
+Sets.newHashSet(AuthAction.consume, AuthAction.produce));
+admin.namespaces().grantPermissionOnNamespace(namespaceName, "Client",
+Sets.newHashSet(AuthAction.consume, AuthAction.produce));
+
+ProxyConfiguration proxyConfig = new ProxyConfiguration();
+proxyConfig.setAuthenticationEnabled(true);
+proxyConfig.setAuthorizationEnabled(false);
+proxyConfig.setBrokerServiceURL("pulsar://localhost:" + BROKER_PORT);
+proxyConfig.setBrokerServiceURLTLS("pulsar://localhost:" +
BROKER_PORT_TLS);
+
+proxyConfig.setServicePort(PortManager.nextFreePort());
+proxyConfig.setServicePortTls(PortManager.nextFreePort());
+proxyConfig.setWebServicePort(PortManager.nextFreePort());
+proxyConfig.setWebServicePortTls(PortManager.nextFreePort());
+proxyConfig.setTlsEnabledInProxy(true);
+proxyConfig.setTlsEnabledWithBroker(true);
+
+// enable tls and auth at proxy
+proxyConfig.setTlsCertificateFilePath(TLS_PROXY_CERT_FILE_PATH);
+proxyConfig.setTlsKeyFilePath(TLS_PROXY_KEY_FILE_PATH);
+proxyConfig.setTlsTrustCertsFilePath(TLS_PROXY_TRUST_CERT_FILE_PATH);
+
+
proxyConfig.setBrokerClientAuthenticationPlugin(AuthenticationTls.class.getName());
+proxyConfig.setBrokerClientAuthenticationParameters(
+"tlsCertFile:" + TLS_PROXY_CERT_FILE_PATH + "," +
"tlsKeyFile:" + TLS_PROXY_KEY_FILE_PATH);
+
+Set providers = new HashSet<>();
+providers.add(AuthenticationProviderTls.class.getName());
+conf.setAuthenticationProviders(providers);
+proxyConfig.setAuthenticationProviders(providers);
+proxyConfig.setTlsProtocols(tlsProtocols);
+proxyConfig.setTlsCiphers(tlsCiphers);
+ProxyService proxyService = Mockito.spy(new ProxyService(proxyConfig));
+proxyService.start();
+Thread.sleep(1000);
Review comment:
umm..instead sleep can we check some condition until which we want to wait
using
`org.apache.pulsar.broker.auth.MockedPulsarServiceBaseTest.retryStrategically(..)`,
it will help to avoid one more intermittent test failure. and we can also add
test timeout=5Sec.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] rdhabalia commented on a change in pull request #1225: Enable specification of TLS Protocol Versions and Cipher Suites
rdhabalia commented on a change in pull request #1225: Enable specification of
TLS Protocol Versions and Cipher Suites
URL: https://github.com/apache/incubator-pulsar/pull/1225#discussion_r167724421
##
File path:
pulsar-common/src/main/java/org/apache/pulsar/common/util/SecurityUtility.java
##
@@ -95,12 +97,22 @@ public static SslContext
createNettySslContextForClient(boolean allowInsecureCon
}
public static SslContext createNettySslContextForServer(boolean
allowInsecureConnection, String trustCertsFilePath,
-String certFilePath, String keyFilePath)
+String certFilePath, String keyFilePath, Set ciphers,
Set protocols)
throws GeneralSecurityException, SSLException,
FileNotFoundException {
X509Certificate[] certificates =
loadCertificatesFromPemFile(certFilePath);
PrivateKey privateKey = loadPrivateKeyFromPemFile(keyFilePath);
SslContextBuilder builder = SslContextBuilder.forServer(privateKey,
(X509Certificate[]) certificates);
+if (ciphers != null && ciphers.size() > 0) {
+builder.ciphers(ciphers);
+}
+
+if (protocols != null && protocols.size() > 0) {
+String[] protocolsArray = new String[protocols.size()];
Review comment:
`builder.protocols(protocols.toArray(protocolsArray));` ??
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] rdhabalia commented on a change in pull request #1225: Enable specification of TLS Protocol Versions and Cipher Suites
rdhabalia commented on a change in pull request #1225: Enable specification of TLS Protocol Versions and Cipher Suites URL: https://github.com/apache/incubator-pulsar/pull/1225#discussion_r167724612 ## File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java ## @@ -179,7 +179,10 @@ private String tlsTrustCertsFilePath = ""; // Accept untrusted TLS certificate from client private boolean tlsAllowInsecureConnection = false; - +// Specify the tls protocols and cipher the broker will use to negotiate during TLS Handshake. Review comment: and in `proxy.conf` as well? should we have any test for this PR? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
