Skip to content

Commit

Permalink
Add cert key type checking to chooseClientAlias
Browse files Browse the repository at this point in the history 
Previously, these functions ignored the keyType (or 'strings') argument to the chooseClientAlias function. Some libraries (e.g. Bouncy Castle) expect that when chooseClientAlias is called and key types are passed in, that it will return null if the cert doesn't use one of the given key types. For example, if ['EC'] was passed in for keyType and the cert contained an RSA key, since this would return 'user' rather than null in that case, it would cause Bouncy Castle to assume using an ECDSA signing algorithm was okay, and cause problems during the Certificate Verify part of the handshake.

This modifies these functions to only return 'user' if keyType is passed in and the cert contains a key of that type. If keyType is empty or null, it will ignore this and continue to check only the issuer.
  • Loading branch information
@nmburgan
nmburgan committed Jan 24, 2022
1 parent 12d4a22 commit 788b228
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 9 deletions.
24 changes: 20 additions & 4 deletions pgjdbc/src/main/java/org/postgresql/ssl/LazyKeyManager.java
View file
Expand Up @@ -102,11 +102,27 @@ public void throwKeyManagerException() throws PSQLException {
if (certchain == null) {
return null;
} else {
X500Principal ourissuer = certchain[certchain.length - 1].getIssuerX500Principal();
X509Certificate cert = certchain[certchain.length - 1];
X500Principal ourissuer = cert.getIssuerX500Principal();
String certKeyType = cert.getPublicKey().getAlgorithm();
boolean keyTypeFound = false;
boolean found = false;
for (Principal issuer : issuers) {
if (ourissuer.equals(issuer)) {
found = true;
if (keyType != null && keyType.length > 0) {
for (String kt : keyType) {
if (kt.equalsIgnoreCase(certKeyType)) {
keyTypeFound = true;
}
}
} else {
// If no key types were passed in, assume we don't care
// about checking that the cert uses a particular key type.
keyTypeFound = true;
}
if (keyTypeFound) {
for (Principal issuer : issuers) {
if (ourissuer.equals(issuer)) {
found = keyTypeFound;
}
}
}
return (found ? "user" : null);
Expand Down
26 changes: 21 additions & 5 deletions pgjdbc/src/main/java/org/postgresql/ssl/PKCS12KeyManager.java
View file
Expand Up @@ -67,7 +67,7 @@ public void throwKeyManagerException() throws PSQLException {
}

@Override
public @Nullable String chooseClientAlias(String[] strings, Principal @Nullable [] principals,
public @Nullable String chooseClientAlias(String[] keyType, Principal @Nullable [] principals,
@Nullable Socket socket) {
if (principals == null || principals.length == 0) {
// Postgres 8.4 and earlier do not send the list of accepted certificate authorities
Expand All @@ -81,11 +81,27 @@ public void throwKeyManagerException() throws PSQLException {
if (certchain == null) {
return null;
} else {
X500Principal ourissuer = certchain[certchain.length - 1].getIssuerX500Principal();
X509Certificate cert = certchain[certchain.length - 1];
X500Principal ourissuer = cert.getIssuerX500Principal();
String certKeyType = cert.getPublicKey().getAlgorithm();
boolean keyTypeFound = false;
boolean found = false;
for (Principal issuer : principals) {
if (ourissuer.equals(issuer)) {
found = true;
if (keyType != null && keyType.length > 0) {
for (String kt : keyType) {
if (kt.equalsIgnoreCase(certKeyType)) {
keyTypeFound = true;
}
}
} else {
// If no key types were passed in, assume we don't care
// about checking that the cert uses a particular key type.
keyTypeFound = true;
}
if (keyTypeFound) {
for (Principal issuer : principals) {
if (ourissuer.equals(issuer)) {
found = keyTypeFound;
}
}
}
return (found ? "user" : null);
Expand Down
27 changes: 27 additions & 0 deletions pgjdbc/src/test/java/org/postgresql/test/ssl/LazyKeyManagerTest.java
View file
Expand Up @@ -20,6 +20,7 @@
import javax.security.auth.callback.CallbackHandler;
import javax.security.auth.callback.PasswordCallback;
import javax.security.auth.callback.UnsupportedCallbackException;
import javax.security.auth.x500.X500Principal;

public class LazyKeyManagerTest {

Expand All @@ -45,6 +46,32 @@ public void testLoadKey() throws Exception {
Assert.assertNotNull(pk);
}

@Test
public void testChooseClientAlias() throws Exception {
LazyKeyManager lazyKeyManager = new LazyKeyManager(
TestUtil.getSslTestCertPath("goodclient.crt"),
TestUtil.getSslTestCertPath("goodclient.pk8"),
new TestCallbackHandler("sslpwd"),
true);
X500Principal testPrincipal = new X500Principal("CN=root certificate, O=PgJdbc test, ST=CA, C=US");
X500Principal[] issuers = new X500Principal[]{testPrincipal};

String validKeyType = lazyKeyManager.chooseClientAlias(new String[]{"RSA"}, issuers, null);
Assert.assertNotNull(validKeyType);

String ignoresCase = lazyKeyManager.chooseClientAlias(new String[]{"rsa"}, issuers, null);
Assert.assertNotNull(ignoresCase);

String invalidKeyType = lazyKeyManager.chooseClientAlias(new String[]{"EC"}, issuers, null);
Assert.assertNull(invalidKeyType);

String containsValidKeyType = lazyKeyManager.chooseClientAlias(new String[]{"EC","RSA"}, issuers, null);
Assert.assertNotNull(containsValidKeyType);

String ignoresBlank = lazyKeyManager.chooseClientAlias(new String[]{}, issuers, null);
Assert.assertNotNull(ignoresBlank);
}

public static class TestCallbackHandler implements CallbackHandler {
char [] password;

Expand Down
54 changes: 54 additions & 0 deletions pgjdbc/src/test/java/org/postgresql/test/ssl/PKCS12KeyTest.java
View file
Expand Up @@ -6,14 +6,22 @@
package org.postgresql.test.ssl;

import org.postgresql.PGProperty;
import org.postgresql.ssl.PKCS12KeyManager;
import org.postgresql.test.TestUtil;

import org.junit.Assert;
import org.junit.Test;

import java.io.IOException;
import java.sql.Connection;
import java.util.Properties;

import javax.security.auth.callback.Callback;
import javax.security.auth.callback.CallbackHandler;
import javax.security.auth.callback.PasswordCallback;
import javax.security.auth.callback.UnsupportedCallbackException;
import javax.security.auth.x500.X500Principal;

public class PKCS12KeyTest {
@Test
public void TestGoodClientP12() throws Exception {
Expand All @@ -29,4 +37,50 @@ public void TestGoodClientP12() throws Exception {
Assert.assertTrue("SSL should be in use", sslUsed);
}
}

@Test
public void TestChooseClientAlias() throws Exception {
PKCS12KeyManager pkcs12KeyManager = new PKCS12KeyManager(TestUtil.getSslTestCertPath("goodclient.p12"), new TestCallbackHandler("sslpwd"));
X500Principal testPrincipal = new X500Principal("CN=root certificate, O=PgJdbc test, ST=CA, C=US");
X500Principal[] issuers = new X500Principal[]{testPrincipal};

String validKeyType = pkcs12KeyManager.chooseClientAlias(new String[]{"RSA"}, issuers, null);
Assert.assertNotNull(validKeyType);

String ignoresCase = pkcs12KeyManager.chooseClientAlias(new String[]{"rsa"}, issuers, null);
Assert.assertNotNull(ignoresCase);

String invalidKeyType = pkcs12KeyManager.chooseClientAlias(new String[]{"EC"}, issuers, null);
Assert.assertNull(invalidKeyType);

String containsValidKeyType = pkcs12KeyManager.chooseClientAlias(new String[]{"EC","RSA"}, issuers, null);
Assert.assertNotNull(containsValidKeyType);

String ignoresBlank = pkcs12KeyManager.chooseClientAlias(new String[]{}, issuers, null);
Assert.assertNotNull(ignoresBlank);
}

public static class TestCallbackHandler implements CallbackHandler {
char [] password;

public TestCallbackHandler(String password) {
if (password != null) {
this.password = password.toCharArray();
}
}

@Override
public void handle(Callback[] callbacks) throws IOException, UnsupportedCallbackException {
for (Callback callback : callbacks) {
if (!(callback instanceof PasswordCallback)) {
throw new UnsupportedCallbackException(callback);
}
PasswordCallback pwdCallback = (PasswordCallback) callback;
if (password != null) {
pwdCallback.setPassword(password);
continue;
}
}
}
}
}

0 comments on commit 788b228

Please sign in to comment.