Skip to content

Commit

Permalink
[SPARK-46132][CORE] Support key password for JKS keys for RPC SSL
Browse files Browse the repository at this point in the history
### What changes were proposed in this pull request?

Add support for a separate key password in addition to the key store password for JKS keys. This is needed for keys which may have a key password in addition to a key store password. We already had this support for the UI SSL support, so for compatibility we should have it here.

This wasn't done earlier as I wasn't sure how to implement it but the discussion in #43998 (comment) suggested the right way.

### Why are the changes needed?

These are needed to support users who may have such keys configured.

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

Added some unit tests

```
build/sbt
> project network-common
> testOnly
```

### Was this patch authored or co-authored using generative AI tooling?

No

Closes #44264 from hasnain-db/separate-pw.

Authored-by: Hasnain Lakhani <hasnain.lakhani@databricks.com>
Signed-off-by: Mridul Muralidharan <mridul<at>gmail.com>
  • Loading branch information
hasnain-db authored and Mridul Muralidharan committed Dec 13, 2023
1 parent b035bb1 commit 83434af
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 4 deletions.
Expand Up @@ -89,7 +89,7 @@ private SSLFactory(final Builder b) {

private void initJdkSslContext(final Builder b)
throws IOException, GeneralSecurityException {
this.keyManagers = keyManagers(b.keyStore, b.keyStorePassword);
this.keyManagers = keyManagers(b.keyStore, b.keyPassword, b.keyStorePassword);
this.trustManagers = trustStoreManagers(
b.trustStore, b.trustStorePassword,
b.trustStoreReloadingEnabled, b.trustStoreReloadIntervalMs
Expand Down Expand Up @@ -391,13 +391,16 @@ private static TrustManager[] defaultTrustManagers(File trustStore, String trust
}
}

private static KeyManager[] keyManagers(File keyStore, String keyStorePassword)
private static KeyManager[] keyManagers(
File keyStore, String keyPassword, String keyStorePassword)
throws NoSuchAlgorithmException, CertificateException,
KeyStoreException, IOException, UnrecoverableKeyException {
KeyManagerFactory factory = KeyManagerFactory.getInstance(
KeyManagerFactory.getDefaultAlgorithm());
char[] passwordCharacters = keyStorePassword != null? keyStorePassword.toCharArray() : null;
factory.init(loadKeyStore(keyStore, passwordCharacters), passwordCharacters);
char[] keyStorePasswordChars = keyStorePassword != null? keyStorePassword.toCharArray() : null;
char[] keyPasswordChars = keyPassword != null?
keyPassword.toCharArray() : keyStorePasswordChars;
factory.init(loadKeyStore(keyStore, keyStorePasswordChars), keyPasswordChars);
return factory.getKeyManagers();
}

Expand Down
@@ -0,0 +1,104 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.spark.network.ssl;

import java.io.File;

import io.netty.buffer.ByteBufAllocator;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.*;

import static org.apache.spark.network.ssl.SslSampleConfigs.*;

public class SSLFactorySuite {

@Test
public void testBuildWithJKSAndPEMWithPasswords() throws Exception {
SSLFactory factory = new SSLFactory.Builder()
.openSslEnabled(true)
.requestedProtocol("TLSv1.3")
.keyStore(new File(SslSampleConfigs.keyStorePath), "password")
.privateKey(new File(SslSampleConfigs.privateKeyPath))
.privateKeyPassword("password")
.keyPassword("password")
.certChain(new File(SslSampleConfigs.certChainPath))
.trustStore(
new File(SslSampleConfigs.trustStorePath),
"password",
true,
10000
)
.build();
factory.createSSLEngine(true, ByteBufAllocator.DEFAULT);
}

@Test
public void testBuildWithJKSAndPEMWithOnlyJKSPassword() throws Exception {
SSLFactory factory = new SSLFactory.Builder()
.openSslEnabled(true)
.requestedProtocol("TLSv1.3")
.keyStore(new File(SslSampleConfigs.keyStorePath), "password")
.privateKey(new File(SslSampleConfigs.unencryptedPrivateKeyPath))
.keyPassword("password")
.certChain(new File(SslSampleConfigs.unencryptedCertChainPath))
.trustStore(
new File(SslSampleConfigs.trustStorePath),
"password",
true,
10000
)
.build();
factory.createSSLEngine(true, ByteBufAllocator.DEFAULT);
}

@Test
public void testBuildWithJKSKeyStorePassword() throws Exception {
// if key password is null, fall back to keystore password
SSLFactory factory = new SSLFactory.Builder()
.requestedProtocol("TLSv1.3")
.keyStore(new File(SslSampleConfigs.keyStorePath), "password")
.trustStore(
new File(SslSampleConfigs.trustStorePath),
"password",
true,
10000
)
.build();
factory.createSSLEngine(true, ByteBufAllocator.DEFAULT);
}

@Test
public void testKeyAndKeystorePasswordsAreDistinct() throws Exception {
assertThrows(RuntimeException.class, () -> {
// Set the wrong password, validate we fail
SSLFactory factory = new SSLFactory.Builder()
.requestedProtocol("TLSv1.3")
.keyStore(new File(SslSampleConfigs.keyStorePath), "password")
.keyPassword("wrong")
.trustStore(
new File(SslSampleConfigs.trustStorePath),
"password",
true,
10000
)
.build();
factory.createSSLEngine(true, ByteBufAllocator.DEFAULT);
});
}
}

0 comments on commit 83434af

Please sign in to comment.