Skip to content

Commit ac66689

Browse files
authored
Validate Ed25519 keys when deserializing them. (#1356)
* Validate Ed25519 keys when deserializing them. This changes the behavior of the deserialization: before this change, deserializing an invalid serialization may not fail, but will fail later when the key is used. This change makes it fail on deserialization, which is preferable. We don't expect users to be affected by this, because such invalid serialization are not used. * Remove a comment. * Fix tests that need to be fixed.
1 parent ebf95b8 commit ac66689

3 files changed

Lines changed: 20 additions & 10 deletions

File tree

common/src/main/java/org/conscrypt/OpenSslEdDsaPrivateKey.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package org.conscrypt;
1818

19+
import java.io.IOException;
20+
import java.io.ObjectInputStream;
1921
import java.security.PrivateKey;
2022
import java.security.spec.EncodedKeySpec;
2123
import java.security.spec.InvalidKeySpecException;
@@ -117,4 +119,11 @@ public boolean equals(Object o) {
117119
public int hashCode() {
118120
return Arrays.hashCode(privateKeyBytes);
119121
}
122+
123+
private void readObject(ObjectInputStream stream) throws IOException, ClassNotFoundException {
124+
stream.defaultReadObject();
125+
if (privateKeyBytes.length != ED25519_PRIVATE_KEY_SIZE_BYTES) {
126+
throw new IllegalArgumentException("Invalid key size");
127+
}
128+
}
120129
}

common/src/main/java/org/conscrypt/OpenSslEdDsaPublicKey.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package org.conscrypt;
1818

19+
import java.io.IOException;
20+
import java.io.ObjectInputStream;
1921
import java.security.PublicKey;
2022
import java.security.spec.EncodedKeySpec;
2123
import java.security.spec.InvalidKeySpecException;
@@ -114,4 +116,11 @@ public int hashCode() {
114116
}
115117
return Arrays.hashCode(publicKeyBytes);
116118
}
119+
120+
private void readObject(ObjectInputStream stream) throws IOException, ClassNotFoundException {
121+
stream.defaultReadObject();
122+
if (publicKeyBytes.length != ED25519_PUBLIC_KEY_SIZE_BYTES) {
123+
throw new IllegalArgumentException("Invalid key size");
124+
}
125+
}
117126
}

common/src/test/java/org/conscrypt/EdDsaTest.java

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -386,11 +386,7 @@ public void deserializeInvalidPrivateKey_fails() throws Exception {
386386
new ByteArrayInputStream(TestUtils.decodeHex(invalidPrivateKeySerialized));
387387
ObjectInputStream ois = new ObjectInputStream(bais);
388388

389-
// This creates an invalid private key that can't be used to sign.
390-
// It would be better to throw an exception when reading the object.
391-
PrivateKey privateKey = (PrivateKey) ois.readObject();
392-
Signature signer = Signature.getInstance("Ed25519", conscryptProvider);
393-
assertThrows(InvalidKeyException.class, () -> signer.initSign(privateKey));
389+
assertThrows(IllegalArgumentException.class, () -> ois.readObject());
394390
}
395391

396392
@Test
@@ -412,10 +408,6 @@ public void deserializeInvalidPublicKey_fails() throws Exception {
412408
new ByteArrayInputStream(TestUtils.decodeHex(invalidPublicKeySerialized));
413409
ObjectInputStream ois = new ObjectInputStream(bais);
414410

415-
// This creates an invalid public key that can't be used to verify.
416-
// It would be better to throw an exception when reading the object.
417-
PublicKey publicKey = (PublicKey) ois.readObject();
418-
Signature verifier = Signature.getInstance("Ed25519", conscryptProvider);
419-
assertThrows(InvalidKeyException.class, () -> verifier.initVerify(publicKey));
411+
assertThrows(IllegalArgumentException.class, () -> ois.readObject());
420412
}
421413
}

0 commit comments

Comments
 (0)