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.

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

fix comparing ecdsa key/cert public key match #2630

Merged
merged 1 commit into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
38 changes: 25 additions & 13 deletions libs/java/cert_refresher/src/main/java/com/oath/auth/Utils.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@
package com.oath.auth;

import org.bouncycastle.asn1.pkcs.PrivateKeyInfo;
import org.bouncycastle.jcajce.provider.asymmetric.ec.BCECPrivateKey;
import org.bouncycastle.jce.spec.ECPublicKeySpec;
import org.bouncycastle.math.ec.ECMultiplier;
import org.bouncycastle.math.ec.ECPoint;
import org.bouncycastle.math.ec.FixedPointCombMultiplier;
import org.bouncycastle.openssl.PEMKeyPair;
import org.bouncycastle.openssl.PEMParser;
import org.bouncycastle.openssl.jcajce.JcaPEMKeyConverter;
Expand All @@ -36,7 +41,7 @@
import java.security.cert.X509Certificate;
import java.security.interfaces.ECKey;
import java.security.interfaces.RSAKey;
import java.security.spec.ECParameterSpec;
import java.security.spec.InvalidKeySpecException;
import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;
Expand Down Expand Up @@ -540,7 +545,9 @@ public static KeyStore createKeyStore(
return keyStore;
}

static boolean verifyPrivatePublicKeyMatch(PrivateKey privateKey, PublicKey publicKey) {
static boolean verifyPrivatePublicKeyMatch(PrivateKey privateKey, PublicKey publicKey)
throws NoSuchAlgorithmException, InvalidKeySpecException {

if (publicKey instanceof RSAKey) {
if (!(privateKey instanceof RSAKey)) {
return false;
Expand All @@ -552,14 +559,16 @@ static boolean verifyPrivatePublicKeyMatch(PrivateKey privateKey, PublicKey publ
if (!(privateKey instanceof ECKey)) {
return false;
}
ECKey pubECKey = (ECKey) publicKey;
ECKey prvECKey = (ECKey) privateKey;
ECParameterSpec pubECParam = pubECKey.getParams();
ECParameterSpec prvECParam = prvECKey.getParams();
return (pubECParam.getCurve().equals(prvECParam.getCurve()) &&
pubECParam.getGenerator().equals(prvECParam.getGenerator()) &&
pubECParam.getOrder().compareTo(prvECParam.getOrder()) == 0 &&
pubECParam.getCofactor() == prvECParam.getCofactor());
KeyFactory keyFactory = KeyFactory.getInstance(privateKey.getAlgorithm());
BCECPrivateKey ecPrivKey = (BCECPrivateKey) privateKey;
ECMultiplier ecMultiplier = new FixedPointCombMultiplier();
org.bouncycastle.jce.spec.ECParameterSpec ecParamSpec = ecPrivKey.getParameters();
ECPoint ecPointQ = ecMultiplier.multiply(ecParamSpec.getG(), ecPrivKey.getD());
ECPublicKeySpec prvKeySpec = new ECPublicKeySpec(ecPointQ, ecParamSpec);

ECPublicKeySpec pubKeySpec = keyFactory.getKeySpec(publicKey, ECPublicKeySpec.class);

return prvKeySpec.getQ().equals(pubKeySpec.getQ()) && prvKeySpec.getParams().equals(pubKeySpec.getParams());
}
return false;
}
Expand All @@ -571,10 +580,13 @@ static void verifyPrivateKeyCertsMatch(PrivateKey privateKey, List<? extends Cer
}
// we need to make sure at least one of the certificates matches
// the public key for the given private key
for (Certificate certificate : certificates) {
if (verifyPrivatePublicKeyMatch(privateKey, certificate.getPublicKey())) {
return;
try {
for (Certificate certificate : certificates) {
if (verifyPrivatePublicKeyMatch(privateKey, certificate.getPublicKey())) {
return;
}
}
} catch (Exception ignored) {
}
throw new KeyRefresherException("Public key mismatch");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,54 @@ public void testCreateKeyStoreECMismatch() throws Exception {
// first enabled public key match
Utils.setDisablePublicKeyCheck(false);
try {
Utils.createKeyStore("ec_public_x509.cert", "unit_test_ec_private2.key");
Utils.createKeyStore("ec_public_x509.cert", "unit_test_ec_private_3.key");
fail();
} catch (KeyRefresherException ex) {
assertEquals(ex.getMessage(), "Public key mismatch");
}
try {
Utils.createKeyStore("ec_public_x509.cert", "unit_test_ec_private_2.key");
fail();
} catch (KeyRefresherException ex) {
assertEquals(ex.getMessage(), "Public key mismatch");
}
try {
Utils.createKeyStore("ec_public_x509_2.cert", "unit_test_ec_private_3.key");
fail();
} catch (KeyRefresherException ex) {
assertEquals(ex.getMessage(), "Public key mismatch");
}
// our valid pairs should work
KeyStore keyStore = Utils.createKeyStore("ec_public_x509.cert", "unit_test_ec_private.key");
assertNotNull(keyStore);
keyStore = Utils.createKeyStore("ec_public_x509_2.cert", "unit_test_ec_private_2.key");
assertNotNull(keyStore);
// now disable public key match
Utils.setDisablePublicKeyCheck(true);
KeyStore keyStore = Utils.createKeyStore("ec_public_x509.cert", "unit_test_ec_private2.key");
keyStore = Utils.createKeyStore("ec_public_x509.cert", "unit_test_ec_private_3.key");
assertNotNull(keyStore);
Utils.setDisablePublicKeyCheck(false);
}

@Test
public void testCreateKeyStoreAlgorithmMismatch() throws Exception {

// first enabled public key match
Utils.setDisablePublicKeyCheck(false);
try {
Utils.createKeyStore("ec_public_x509.cert", "unit_test_rsa_private.key");
fail();
} catch (KeyRefresherException ex) {
assertEquals(ex.getMessage(), "Public key mismatch");
}
try {
Utils.createKeyStore("rsa_public_x509.cert", "unit_test_ec_private.key");
fail();
} catch (KeyRefresherException ex) {
assertEquals(ex.getMessage(), "Public key mismatch");
}
}

@Test
public void testCreateKeyStoreChain() throws Exception {
KeyStore keyStore = Utils.createKeyStore("rsa_public_x510_w_intermediate.cert", "unit_test_rsa_private.key");
Expand Down
12 changes: 12 additions & 0 deletions libs/java/cert_refresher/src/test/resources/ec_public_x509_2.cert
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-----BEGIN CERTIFICATE-----
MIIBsDCCAVYCCQCKZ1MvX0auizAKBggqhkjOPQQDAjBgMQswCQYDVQQGEwJVUzEL
MAkGA1UECAwCQ0ExEjAQBgNVBAcMCVN1bm55dmFsZTEYMBYGA1UECgwPTXkgVGVz
dCBDb21wYW55MRYwFAYDVQQDDA1hdGhlbnouc3luY2VyMB4XDTI0MDUyNjE5MzU0
OVoXDTI1MDUyNjE5MzU0OVowYDELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNBMRIw
EAYDVQQHDAlTdW5ueXZhbGUxGDAWBgNVBAoMD015IFRlc3QgQ29tcGFueTEWMBQG
A1UEAwwNYXRoZW56LnN5bmNlcjBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABAmT
x0yHYsR1WsVQZu7qTrREOzuYa/cJmrmRfcUbNATSn9aH2WTla6AOiwikAE1Vqmw4
3nSqcmn0ev7xhEF+bJEwCgYIKoZIzj0EAwIDSAAwRQIhAPy9LIcDEL39KgtnMsmP
VNVlZ/tqQe7YwuiQP3utv0MAAiBviBg8bxlu4LVTXBb7Zz7aRidXAwe0keRuIgyO
V9XLUw==
-----END CERTIFICATE-----
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-----BEGIN EC PRIVATE KEY-----
MHcCAQEEII9QxtCy55qoB5/o10YkCJUaUp+qo8e3mF6wcZGHoA82oAoGCCqGSM49
AwEHoUQDQgAECZPHTIdixHVaxVBm7upOtEQ7O5hr9wmauZF9xRs0BNKf1ofZZOVr
oA6LCKQATVWqbDjedKpyafR6/vGEQX5skQ==
-----END EC PRIVATE KEY-----
Loading