Skip to content

Commit

Permalink
fix comparing ecdsa key/cert public key match (#2630)
Browse files Browse the repository at this point in the history
Signed-off-by: Henry Avetisyan <hga@yahooinc.com>
Co-authored-by: Henry Avetisyan <hga@yahooinc.com>
  • Loading branch information
havetisyan and havetisyan authored May 29, 2024
1 parent f478a3f commit 89dede1
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 15 deletions.
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-----

0 comments on commit 89dede1

Please sign in to comment.