SSLEngine: verify DHE signature
The DHE signature wasn't being verified against the server's
certificate. Refactor some code to allow the same code to be used for
both the server and client to create and check the DHE parameters
signature.
Bug: 11631299
Change-Id: Ie98cdab039c2213fa77e7768e145c08a4fd7ad7b
diff --git a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/ClientHandshakeImpl.java b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/ClientHandshakeImpl.java
index 4b29363..b3ae9cb 100644
--- a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/ClientHandshakeImpl.java
+++ b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/ClientHandshakeImpl.java
@@ -439,59 +439,61 @@
"Unexpected exception", e);
return;
}
- } else {
+ } else if (session.cipherSuite.keyExchange == CipherSuite.KEY_EXCHANGE_DHE_DSS
+ || session.cipherSuite.keyExchange == CipherSuite.KEY_EXCHANGE_DHE_DSS_EXPORT
+ || session.cipherSuite.keyExchange == CipherSuite.KEY_EXCHANGE_DHE_RSA
+ || session.cipherSuite.keyExchange == CipherSuite.KEY_EXCHANGE_DHE_RSA_EXPORT
+ || session.cipherSuite.keyExchange == CipherSuite.KEY_EXCHANGE_DH_anon
+ || session.cipherSuite.keyExchange == CipherSuite.KEY_EXCHANGE_DH_anon_EXPORT) {
+ /*
+ * All other key exchanges should have had a DH key communicated via
+ * ServerKeyExchange beforehand.
+ */
+ if (serverKeyExchange == null) {
+ fatalAlert(AlertProtocol.UNEXPECTED_MESSAGE, "Expected ServerKeyExchange");
+ return;
+ }
+ if (session.cipherSuite.isAnonymous() != serverKeyExchange.isAnonymous()) {
+ fatalAlert(AlertProtocol.DECRYPT_ERROR, "Wrong type in ServerKeyExchange");
+ return;
+ }
try {
+ if (!session.cipherSuite.isAnonymous()) {
+ DigitalSignature ds = new DigitalSignature(serverCert.getAuthType());
+ ds.init(serverCert.certs[0]);
+ ds.update(clientHello.getRandom());
+ ds.update(serverHello.getRandom());
+ if (!serverKeyExchange.verifySignature(ds)) {
+ fatalAlert(AlertProtocol.DECRYPT_ERROR, "Cannot verify DH params");
+ return;
+ }
+ }
KeyFactory kf = KeyFactory.getInstance("DH");
KeyAgreement agreement = KeyAgreement.getInstance("DH");
KeyPairGenerator kpg = KeyPairGenerator.getInstance("DH");
- PublicKey serverPublic;
- DHParameterSpec spec;
- if (serverKeyExchange != null) {
- serverPublic = kf.generatePublic(new DHPublicKeySpec(
- serverKeyExchange.par3, serverKeyExchange.par1,
- serverKeyExchange.par2));
- spec = new DHParameterSpec(serverKeyExchange.par1,
- serverKeyExchange.par2);
- } else {
- serverPublic = serverCert.certs[0].getPublicKey();
- spec = ((DHPublicKey) serverPublic).getParams();
- }
+ PublicKey serverDhPublic = kf.generatePublic(new DHPublicKeySpec(
+ serverKeyExchange.par3, serverKeyExchange.par1,
+ serverKeyExchange.par2));
+ DHParameterSpec spec = new DHParameterSpec(serverKeyExchange.par1,
+ serverKeyExchange.par2);
kpg.initialize(spec);
-
KeyPair kp = kpg.generateKeyPair();
- Key key = kp.getPublic();
- if (clientCert != null
- && serverCert != null
- && (session.cipherSuite.keyExchange == CipherSuite.KEY_EXCHANGE_DHE_RSA
- || session.cipherSuite.keyExchange == CipherSuite.KEY_EXCHANGE_DHE_DSS)) {
- PublicKey client_pk = clientCert.certs[0].getPublicKey();
- PublicKey server_pk = serverCert.certs[0].getPublicKey();
- if (client_pk instanceof DHKey
- && server_pk instanceof DHKey) {
- if (((DHKey) client_pk).getParams().getG().equals(
- ((DHKey) server_pk).getParams().getG())
- && ((DHKey) client_pk).getParams().getP()
- .equals(((DHKey) server_pk).getParams().getG())) {
- // client cert message DH public key parameters
- // matched those specified by the
- // server in its certificate,
- clientKeyExchange = new ClientKeyExchange(); // empty
- }
- }
- } else {
- clientKeyExchange = new ClientKeyExchange(
- ((DHPublicKey) key).getY());
- }
- key = kp.getPrivate();
- agreement.init(key);
- agreement.doPhase(serverPublic, true);
+ DHPublicKey pubDhKey = (DHPublicKey) kp.getPublic();
+ clientKeyExchange = new ClientKeyExchange(pubDhKey.getY());
+ PrivateKey privDhKey = kp.getPrivate();
+ agreement.init(privDhKey);
+ agreement.doPhase(serverDhPublic, true);
preMasterSecret = agreement.generateSecret();
} catch (Exception e) {
fatalAlert(AlertProtocol.INTERNAL_ERROR,
"Unexpected exception", e);
return;
}
+ } else {
+ fatalAlert(AlertProtocol.DECRYPT_ERROR, "Unsupported handshake type");
+ return;
}
+
if (clientKeyExchange != null) {
send(clientKeyExchange);
}
diff --git a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/DigitalSignature.java b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/DigitalSignature.java
index af12bb3..ade2c7a 100644
--- a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/DigitalSignature.java
+++ b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/DigitalSignature.java
@@ -211,6 +211,9 @@
public boolean verifySignature(byte[] data) {
if (signature != null) {
try {
+ if (sha_hash == null) {
+ sha_hash = sha.digest();
+ }
signature.update(sha_hash);
return signature.verify(data);
} catch (SignatureException e) {
@@ -229,6 +232,12 @@
}
final byte[] md5_sha;
+ if (sha != null && sha_hash == null) {
+ sha_hash = sha.digest();
+ }
+ if (md5 != null && md5_hash == null) {
+ md5_hash = md5.digest();
+ }
if (md5_hash != null && sha_hash != null) {
md5_sha = new byte[md5_hash.length + sha_hash.length];
System.arraycopy(md5_hash, 0, md5_sha, 0, md5_hash.length);
diff --git a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/ServerHandshakeImpl.java b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/ServerHandshakeImpl.java
index c5e1838..833f1b7 100644
--- a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/ServerHandshakeImpl.java
+++ b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/ServerHandshakeImpl.java
@@ -523,36 +523,13 @@
ds.update(clientHello.getRandom());
ds.update(serverHello.getRandom());
- byte[] tmp;
- byte[] tmpLength = new byte[2];
//FIXME 1_byte==0x00
if (cipher_suite.keyExchange == CipherSuite.KEY_EXCHANGE_RSA_EXPORT) {
- tmp = ServerKeyExchange.toUnsignedByteArray(rsakey.getModulus());
- tmpLength[0] = (byte) ((tmp.length & 0xFF00) >>> 8);
- tmpLength[1] = (byte) (tmp.length & 0xFF);
- ds.update(tmpLength);
- ds.update(tmp);
- tmp = ServerKeyExchange.toUnsignedByteArray(rsakey.getPublicExponent());
- tmpLength[0] = (byte) ((tmp.length & 0xFF00) >>> 8);
- tmpLength[1] = (byte) (tmp.length & 0xFF);
- ds.update(tmpLength);
- ds.update(tmp);
+ ServerKeyExchange.updateSignatureRsa(ds, rsakey.getModulus(),
+ rsakey.getPublicExponent());
} else {
- tmp = ServerKeyExchange.toUnsignedByteArray(dhkeySpec.getP());
- tmpLength[0] = (byte) ((tmp.length & 0xFF00) >>> 8);
- tmpLength[1] = (byte) (tmp.length & 0xFF);
- ds.update(tmpLength);
- ds.update(tmp);
- tmp = ServerKeyExchange.toUnsignedByteArray(dhkeySpec.getG());
- tmpLength[0] = (byte) ((tmp.length & 0xFF00) >>> 8);
- tmpLength[1] = (byte) (tmp.length & 0xFF);
- ds.update(tmpLength);
- ds.update(tmp);
- tmp = ServerKeyExchange.toUnsignedByteArray(dhkeySpec.getY());
- tmpLength[0] = (byte) ((tmp.length & 0xFF00) >>> 8);
- tmpLength[1] = (byte) (tmp.length & 0xFF);
- ds.update(tmpLength);
- ds.update(tmp);
+ ServerKeyExchange.updateSignatureDh(ds, dhkeySpec.getP(), dhkeySpec.getG(),
+ dhkeySpec.getY());
}
hash = ds.sign();
} else {
diff --git a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/ServerKeyExchange.java b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/ServerKeyExchange.java
index fea8076..7ccaf7cd3 100644
--- a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/ServerKeyExchange.java
+++ b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/ServerKeyExchange.java
@@ -98,6 +98,60 @@
}
}
+ public static void updateSignatureRsa(DigitalSignature ds, BigInteger modulus,
+ BigInteger publicExponent) {
+ byte[] tmp;
+ byte[] tmpLength = new byte[2];
+ tmp = ServerKeyExchange.toUnsignedByteArray(modulus);
+ tmpLength[0] = (byte) ((tmp.length & 0xFF00) >>> 8);
+ tmpLength[1] = (byte) (tmp.length & 0xFF);
+ ds.update(tmpLength);
+ ds.update(tmp);
+ tmp = ServerKeyExchange.toUnsignedByteArray(publicExponent);
+ tmpLength[0] = (byte) ((tmp.length & 0xFF00) >>> 8);
+ tmpLength[1] = (byte) (tmp.length & 0xFF);
+ ds.update(tmpLength);
+ ds.update(tmp);
+ }
+
+ public static void updateSignatureDh(DigitalSignature ds, BigInteger p, BigInteger g,
+ BigInteger y) {
+ byte[] tmp;
+ byte[] tmpLength = new byte[2];
+ tmp = ServerKeyExchange.toUnsignedByteArray(p);
+ tmpLength[0] = (byte) ((tmp.length & 0xFF00) >>> 8);
+ tmpLength[1] = (byte) (tmp.length & 0xFF);
+ ds.update(tmpLength);
+ ds.update(tmp);
+ tmp = ServerKeyExchange.toUnsignedByteArray(g);
+ tmpLength[0] = (byte) ((tmp.length & 0xFF00) >>> 8);
+ tmpLength[1] = (byte) (tmp.length & 0xFF);
+ ds.update(tmpLength);
+ ds.update(tmp);
+ tmp = ServerKeyExchange.toUnsignedByteArray(y);
+ tmpLength[0] = (byte) ((tmp.length & 0xFF00) >>> 8);
+ tmpLength[1] = (byte) (tmp.length & 0xFF);
+ ds.update(tmpLength);
+ ds.update(tmp);
+ }
+
+ public boolean verifySignature(DigitalSignature ds) {
+ if (par3 != null) {
+ updateSignatureDh(ds, par1, par2, par3);
+ } else {
+ updateSignatureRsa(ds, par1, par2);
+ }
+ return ds.verifySignature(hash);
+ }
+
+ /**
+ * Will return {@code true} if the signature is {@code null} since this is
+ * considered anonymous.
+ */
+ public boolean isAnonymous() {
+ return hash == null;
+ }
+
/**
* Creates inbound message
* @param in
