diff options
author | David Zeuthen <zeuthen@google.com> | 2022-07-21 17:11:36 -0400 |
---|---|---|
committer | David Zeuthen <zeuthen@google.com> | 2022-07-22 09:49:14 -0400 |
commit | dc379a45aa3043a478c1c93f93252612e5c8cafe (patch) | |
tree | f5f754c2c98544c31effd3484a775511e9812a1d /identity | |
parent | 2c2ef9a04ab47ed3efb2c7d7747580b755634a7f (diff) | |
download | base-dc379a45aa3043a478c1c93f93252612e5c8cafe.tar.gz |
identity: Fix uncompressed form encoding of P-256 EC Public Key.
Properly encode an P-256 EC Public Key in uncompressed form, in
particular ensure that the resulting blob is always 65 bytes long as
is expected.
Was able to reproduce this with about 4% failures running a
test. After the fix didn't get a failure in 1,000 runs.
Also remove unused Util.integerCollectionToArray() function.
Bug: 239857653
Test: atest --rerun-until-failure 1000 android.security.identity.cts.ProvisioningTest#testProvisionAndRetrieveMultipleTime
Change-Id: I9a8a5570fde5a80f74632606126cdfcc1f6c7c99
Diffstat (limited to 'identity')
-rw-r--r-- | identity/java/android/security/identity/Util.java | 55 |
1 files changed, 39 insertions, 16 deletions
diff --git a/identity/java/android/security/identity/Util.java b/identity/java/android/security/identity/Util.java index e56bd5167906..789ff06f064b 100644 --- a/identity/java/android/security/identity/Util.java +++ b/identity/java/android/security/identity/Util.java @@ -20,12 +20,12 @@ import android.annotation.NonNull; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.math.BigInteger; import java.security.InvalidKeyException; import java.security.NoSuchAlgorithmException; import java.security.PublicKey; import java.security.interfaces.ECPublicKey; import java.security.spec.ECPoint; -import java.util.Collection; import javax.crypto.Mac; import javax.crypto.spec.SecretKeySpec; @@ -36,15 +36,6 @@ import javax.crypto.spec.SecretKeySpec; public class Util { private static final String TAG = "Util"; - static int[] integerCollectionToArray(Collection<Integer> collection) { - int[] result = new int[collection.size()]; - int n = 0; - for (int item : collection) { - result[n++] = item; - } - return result; - } - static byte[] stripLeadingZeroes(byte[] value) { int n = 0; while (n < value.length && value[n] == 0) { @@ -61,15 +52,47 @@ public class Util { static byte[] publicKeyEncodeUncompressedForm(PublicKey publicKey) { ECPoint w = ((ECPublicKey) publicKey).getW(); - // X and Y are always positive so for interop we remove any leading zeroes - // inserted by the BigInteger encoder. - byte[] x = stripLeadingZeroes(w.getAffineX().toByteArray()); - byte[] y = stripLeadingZeroes(w.getAffineY().toByteArray()); + BigInteger x = w.getAffineX(); + BigInteger y = w.getAffineY(); + if (x.compareTo(BigInteger.ZERO) < 0) { + throw new RuntimeException("X is negative"); + } + if (y.compareTo(BigInteger.ZERO) < 0) { + throw new RuntimeException("Y is negative"); + } try { ByteArrayOutputStream baos = new ByteArrayOutputStream(); baos.write(0x04); - baos.write(x); - baos.write(y); + + // Each coordinate may be encoded in 33*, 32, or fewer bytes. + // + // * : it can be 33 bytes because toByteArray() guarantees "The array will contain the + // minimum number of bytes required to represent this BigInteger, including at + // least one sign bit, which is (ceil((this.bitLength() + 1)/8))" which means that + // the MSB is always 0x00. This is taken care of by calling calling + // stripLeadingZeroes(). + // + // We need the encoding to be exactly 32 bytes since according to RFC 5480 section 2.2 + // and SEC 1: Elliptic Curve Cryptography section 2.3.3 the encoding is 0x04 | X | Y + // where X and Y are encoded in exactly 32 byte, big endian integer values each. + // + byte[] xBytes = stripLeadingZeroes(x.toByteArray()); + if (xBytes.length > 32) { + throw new RuntimeException("xBytes is " + xBytes.length + " which is unexpected"); + } + for (int n = 0; n < 32 - xBytes.length; n++) { + baos.write(0x00); + } + baos.write(xBytes); + + byte[] yBytes = stripLeadingZeroes(y.toByteArray()); + if (yBytes.length > 32) { + throw new RuntimeException("yBytes is " + yBytes.length + " which is unexpected"); + } + for (int n = 0; n < 32 - yBytes.length; n++) { + baos.write(0x00); + } + baos.write(yBytes); return baos.toByteArray(); } catch (IOException e) { throw new RuntimeException("Unexpected IOException", e); |