From 4900d16d9ef59beac0c22fa608978608ec412574 Mon Sep 17 00:00:00 2001 From: Andrew Scull Date: Mon, 27 Nov 2023 10:11:56 +0000 Subject: [PATCH 1/3] Use Tink utilities for converting signature format Switch to using the Tink methods for converting ECDSA signatures to and from DER. This replaces a custom implementation using BC. --- src/com/google/cose/utils/CoseUtils.java | 114 ++++------------------- 1 file changed, 16 insertions(+), 98 deletions(-) diff --git a/src/com/google/cose/utils/CoseUtils.java b/src/com/google/cose/utils/CoseUtils.java index ff00dc2..2438bec 100644 --- a/src/com/google/cose/utils/CoseUtils.java +++ b/src/com/google/cose/utils/CoseUtils.java @@ -16,6 +16,9 @@ package com.google.cose.utils; +import static com.google.crypto.tink.subtle.EllipticCurves.ecdsaDer2Ieee; +import static com.google.crypto.tink.subtle.EllipticCurves.ecdsaIeee2Der; + import co.nstant.in.cbor.CborBuilder; import co.nstant.in.cbor.CborException; import co.nstant.in.cbor.builder.ArrayBuilder; @@ -41,11 +44,9 @@ import com.google.cose.structure.MacStructure.MacContext; import com.google.cose.structure.SignStructure; import com.google.cose.structure.SignStructure.SignatureContext; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.IOException; import java.math.BigInteger; import java.security.AlgorithmParameters; +import java.security.GeneralSecurityException; import java.security.KeyFactory; import java.security.NoSuchAlgorithmException; import java.security.PublicKey; @@ -58,13 +59,6 @@ import java.security.spec.InvalidKeySpecException; import java.security.spec.InvalidParameterSpecException; import java.security.spec.PKCS8EncodedKeySpec; -import java.util.Arrays; -import org.bouncycastle.asn1.ASN1Encodable; -import org.bouncycastle.asn1.ASN1InputStream; -import org.bouncycastle.asn1.ASN1Integer; -import org.bouncycastle.asn1.ASN1Primitive; -import org.bouncycastle.asn1.ASN1Sequence; -import org.bouncycastle.asn1.DERSequenceGenerator; import org.bouncycastle.jce.ECNamedCurveTable; import org.bouncycastle.jce.spec.ECNamedCurveParameterSpec; @@ -280,9 +274,12 @@ public static Sign1Message generateCoseSign1(CoseKey key, Map protectedHeaders, if (key instanceof OkpSigningKey) { signature = ((OkpSigningKey) key).sign(algorithm, toBeSigned); } else { - signature = signatureDerToCose( - ((Ec2SigningKey) key).sign(algorithm, toBeSigned, null), - algorithm); + signature = ((Ec2SigningKey) key).sign(algorithm, toBeSigned, null); + try { + signature = ecdsaDer2Ieee(signature, 2 * getKeySizeFromAlgorithm(algorithm)); + } catch (GeneralSecurityException e) { + throw new AssertionError(e); + } } return Sign1Message.builder() @@ -313,88 +310,18 @@ public static void verifyCoseSign1Message(CoseKey key, Sign1Message message, SignatureContext.SIGNATURE1, protectedHeaders, null, externalAad, signedMessage ).serialize(); if (key instanceof Ec2SigningKey) { - byte[] signature = signatureCoseToDer(message.getSignature()); + byte[] signature; + try { + signature = ecdsaIeee2Der(message.getSignature()); + } catch (GeneralSecurityException e) { + throw new CoseException("Invalid signature.", e); + } ((Ec2SigningKey) key).verify(algorithm, encodedStructure, signature, null); } else { ((OkpSigningKey) key).verify(algorithm, encodedStructure, message.getSignature()); } } - private static byte[] signatureCoseToDer(byte[] signature) { - // r and s are always positive and may use all bits so use the constructor which - // parses them as unsigned. - BigInteger r = new BigInteger(1, Arrays.copyOfRange( - signature, 0, signature.length / 2)); - BigInteger s = new BigInteger(1, Arrays.copyOfRange( - signature, signature.length / 2, signature.length)); - - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - try { - DERSequenceGenerator seq = new DERSequenceGenerator(baos); - seq.addObject(new ASN1Integer(r.toByteArray())); - seq.addObject(new ASN1Integer(s.toByteArray())); - seq.close(); - } catch (IOException e) { - throw new IllegalStateException("Error generating DER signature", e); - } - return baos.toByteArray(); - } - - /* - * From RFC 8152 section 8.1 ECDSA: - * - * The signature algorithm results in a pair of integers (R, S). These - * integers will be the same length as the length of the key used for - * the signature process. The signature is encoded by converting the - * integers into byte strings of the same length as the key size. The - * length is rounded up to the nearest byte and is left padded with zero - * bits to get to the correct length. The two integers are then - * concatenated together to form a byte string that is the resulting - * signature. - */ - private static byte[] signatureDerToCose(byte[] signature, Algorithm algorithm) - throws CoseException { - ASN1Primitive asn1; - try { - asn1 = new ASN1InputStream(new ByteArrayInputStream(signature)).readObject(); - } catch (IOException e) { - throw new IllegalArgumentException("Error decoding DER signature", e); - } - if (!(asn1 instanceof ASN1Sequence)) { - throw new IllegalArgumentException("Not a ASN1 sequence"); - } - ASN1Encodable[] asn1Encodables = ((ASN1Sequence) asn1).toArray(); - if (asn1Encodables.length != 2) { - throw new IllegalArgumentException("Expected two items in sequence"); - } - if (!(asn1Encodables[0].toASN1Primitive() instanceof ASN1Integer)) { - throw new IllegalArgumentException("First item is not an integer"); - } - BigInteger r = ((ASN1Integer) asn1Encodables[0].toASN1Primitive()).getValue(); - if (!(asn1Encodables[1].toASN1Primitive() instanceof ASN1Integer)) { - throw new IllegalArgumentException("Second item is not an integer"); - } - BigInteger s = ((ASN1Integer) asn1Encodables[1].toASN1Primitive()).getValue(); - - byte[] rBytes = stripLeadingZeroes(r.toByteArray()); - byte[] sBytes = stripLeadingZeroes(s.toByteArray()); - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - int keySize = getKeySizeFromAlgorithm(algorithm); - try { - for (int n = 0; n < keySize - rBytes.length; n++) { - baos.write(0x00); - } - baos.write(rBytes); - for (int n = 0; n < keySize - sBytes.length; n++) { - baos.write(0x00); - } - baos.write(sBytes); - } catch (IOException e) { - throw new CoseException("Error while converting signature to cose spec.", e); - } - return baos.toByteArray(); - } - private static int getKeySizeFromAlgorithm(Algorithm algorithm) { switch (algorithm) { case SIGNING_ALGORITHM_ECDSA_SHA_256: @@ -408,15 +335,6 @@ private static int getKeySizeFromAlgorithm(Algorithm algorithm) { } } - private static byte[] stripLeadingZeroes(byte[] value) { - for (int i = 0; i < value.length; i++) { - if (value[i] != 0x00) { - return Arrays.copyOfRange(value, i, value.length); - } - } - return new byte[0]; - } - private static byte[] getMessageFromDetachedOrPayload(byte[] payloadMessage, byte[] detachedContent) throws CoseException { int payloadLen = payloadMessage == null ? 0 : payloadMessage.length; From eb5d03413b2569d969a08ba590c6892ba5814ab1 Mon Sep 17 00:00:00 2001 From: Andrew Scull Date: Mon, 27 Nov 2023 14:32:12 +0000 Subject: [PATCH 2/3] Use Tink utilities in EC2 key logic Use Tink's helper functions for converting between byte and BigInteger representations and for generating EC key pairs. Delete custom implementations of the same logic. --- src/com/google/cose/Ec2Key.java | 14 ++--- src/com/google/cose/Ec2SigningKey.java | 78 ++++++++------------------ 2 files changed, 30 insertions(+), 62 deletions(-) diff --git a/src/com/google/cose/Ec2Key.java b/src/com/google/cose/Ec2Key.java index 2d45a1c..b48ea33 100644 --- a/src/com/google/cose/Ec2Key.java +++ b/src/com/google/cose/Ec2Key.java @@ -1,5 +1,7 @@ package com.google.cose; +import static com.google.crypto.tink.subtle.SubtleUtil.bytes2Integer; + import co.nstant.in.cbor.CborException; import co.nstant.in.cbor.model.ByteString; import co.nstant.in.cbor.model.DataItem; @@ -11,15 +13,12 @@ import com.google.cose.utils.CborUtils; import com.google.cose.utils.CoseUtils; import com.google.cose.utils.Headers; -import java.math.BigInteger; import java.security.interfaces.ECPublicKey; /** * Abstract class for generic Ec2 key */ public abstract class Ec2Key extends CoseKey { - private static final int SIGN_POSITIVE = 1; - private ECPublicKey publicKey; Ec2Key(DataItem cborKey) throws CborException, CoseException { @@ -46,11 +45,10 @@ void populateKeyFromCbor() throws CborException, CoseException { throw new IllegalStateException("X coordinate provided but Y coordinate is missing."); } final ByteString yCor = CborUtils.asByteString(labels.get(Headers.KEY_PARAMETER_Y)); - publicKey = (ECPublicKey) CoseUtils.getEc2PublicKeyFromCoordinates( - curve, - new BigInteger(SIGN_POSITIVE, xCor.getBytes()), - new BigInteger(SIGN_POSITIVE, yCor.getBytes()) - ); + publicKey = + (ECPublicKey) + CoseUtils.getEc2PublicKeyFromCoordinates( + curve, bytes2Integer(xCor.getBytes()), bytes2Integer(yCor.getBytes())); } public ECPublicKey getPublicKey() { diff --git a/src/com/google/cose/Ec2SigningKey.java b/src/com/google/cose/Ec2SigningKey.java index e0f55ae..b0a1173 100644 --- a/src/com/google/cose/Ec2SigningKey.java +++ b/src/com/google/cose/Ec2SigningKey.java @@ -16,6 +16,12 @@ package com.google.cose; +import static com.google.crypto.tink.subtle.EllipticCurves.fieldSizeInBytes; +import static com.google.crypto.tink.subtle.EllipticCurves.generateKeyPair; +import static com.google.crypto.tink.subtle.EllipticCurves.getCurveSpec; +import static com.google.crypto.tink.subtle.SubtleUtil.bytes2Integer; +import static com.google.crypto.tink.subtle.SubtleUtil.integer2Bytes; + import co.nstant.in.cbor.CborException; import co.nstant.in.cbor.model.ByteString; import co.nstant.in.cbor.model.DataItem; @@ -26,11 +32,10 @@ import com.google.cose.utils.CborUtils; import com.google.cose.utils.CoseUtils; import com.google.cose.utils.Headers; -import java.math.BigInteger; -import java.security.InvalidAlgorithmParameterException; +import com.google.crypto.tink.subtle.EllipticCurves.CurveType; +import java.security.GeneralSecurityException; import java.security.InvalidKeyException; import java.security.KeyPair; -import java.security.KeyPairGenerator; import java.security.NoSuchAlgorithmException; import java.security.NoSuchProviderException; import java.security.PublicKey; @@ -38,13 +43,10 @@ import java.security.SignatureException; import java.security.interfaces.ECPrivateKey; import java.security.interfaces.ECPublicKey; -import java.security.spec.ECGenParameterSpec; import java.security.spec.ECPoint; /** Implements EC2 COSE_Key spec for signing purposes. */ public final class Ec2SigningKey extends Ec2Key { - private static final int SIGN_POSITIVE = 1; - private KeyPair keyPair; public Ec2SigningKey(DataItem cborKey) throws CborException, CoseException { @@ -73,7 +75,7 @@ void populateKeyFromCbor() throws CborException, CoseException { if (key.length == 0) { throw new CoseException("Cannot decode private key. Missing coordinate information."); } - privateKey = CoseUtils.getEc2PrivateKeyFromInteger(curve, new BigInteger(SIGN_POSITIVE, key)); + privateKey = CoseUtils.getEc2PrivateKeyFromInteger(curve, bytes2Integer(key)); } else { privateKey = null; } @@ -96,11 +98,9 @@ void populateKeyFromCbor() throws CborException, CoseException { throw new IllegalStateException("X coordinate provided but Y coordinate is missing."); } final ByteString yCor = CborUtils.asByteString(labels.get(Headers.KEY_PARAMETER_Y)); - final PublicKey publicKey = CoseUtils.getEc2PublicKeyFromCoordinates( - curve, - new BigInteger(SIGN_POSITIVE, xCor.getBytes()), - new BigInteger(SIGN_POSITIVE, yCor.getBytes()) - ); + final PublicKey publicKey = + CoseUtils.getEc2PublicKeyFromCoordinates( + curve, bytes2Integer(xCor.getBytes()), bytes2Integer(yCor.getBytes())); keyPair = new KeyPair(publicKey, privateKey); } @@ -118,69 +118,41 @@ public ECPublicKey getPublicKey() { return (ECPublicKey) this.keyPair.getPublic(); } - // Big endian: Do not reuse for little endian encodings - private static byte[] arrayFromBigNum(BigInteger num, int keySize) - throws IllegalArgumentException { - // Roundup arithmetic from bits to bytes. - byte[] keyBytes = new byte[(keySize + 7) / 8]; - byte[] keyBytes2 = num.toByteArray(); - if (keyBytes.length == keyBytes2.length) { - return keyBytes2; - } - if (keyBytes2.length > keyBytes.length) { - // There should be no more than one padding(0) byte, invalid key otherwise. - if (keyBytes2.length - keyBytes.length > 1 && keyBytes2[0] != 0) { - throw new IllegalArgumentException(); - } - System.arraycopy(keyBytes2, keyBytes2.length - keyBytes.length, keyBytes, 0, keyBytes.length); - } else { - System.arraycopy( - keyBytes2, 0, keyBytes, keyBytes.length - keyBytes2.length, keyBytes2.length); - } - return keyBytes; - } - /** * Generates a COSE formatted Ec2 signing key given a specific algorithm. The selected key size is * chosen based on section 6.2.1 of RFC 5656 */ public static Ec2SigningKey generateKey(Algorithm algorithm) throws CborException, CoseException { - KeyPair keyPair; - int keySize; int header; - String curveName; - + CurveType curveType; switch (algorithm) { case SIGNING_ALGORITHM_ECDSA_SHA_256: - curveName = "secp256r1"; - keySize = 256; + curveType = CurveType.NIST_P256; header = Headers.CURVE_EC2_P256; break; case SIGNING_ALGORITHM_ECDSA_SHA_384: - curveName = "secp384r1"; - keySize = 384; + curveType = CurveType.NIST_P384; header = Headers.CURVE_EC2_P384; break; case SIGNING_ALGORITHM_ECDSA_SHA_512: - curveName = "secp521r1"; - keySize = 521; + curveType = CurveType.NIST_P521; header = Headers.CURVE_EC2_P521; break; default: throw new CoseException("Unsupported algorithm curve: " + algorithm.getJavaAlgorithmId()); } + + KeyPair keyPair; try { - ECGenParameterSpec paramSpec = new ECGenParameterSpec(curveName); - KeyPairGenerator gen = KeyPairGenerator.getInstance("EC"); - gen.initialize(paramSpec); - keyPair = gen.genKeyPair(); + keyPair = generateKeyPair(curveType); ECPoint pubPoint = ((ECPublicKey) keyPair.getPublic()).getW(); - byte[] x = arrayFromBigNum(pubPoint.getAffineX(), keySize); - byte[] y = arrayFromBigNum(pubPoint.getAffineY(), keySize); + int keySize = fieldSizeInBytes(getCurveSpec(curveType).getCurve()); + byte[] x = integer2Bytes(pubPoint.getAffineX(), keySize); + byte[] y = integer2Bytes(pubPoint.getAffineY(), keySize); byte[] privEncodedKey = keyPair.getPrivate().getEncoded(); @@ -192,10 +164,8 @@ public static Ec2SigningKey generateKey(Algorithm algorithm) throws CborExceptio .withCurve(header) .withAlgorithm(algorithm) .build(); - } catch (NoSuchAlgorithmException e) { - throw new CoseException("No provider for algorithm: " + algorithm.getJavaAlgorithmId(), e); - } catch (InvalidAlgorithmParameterException e) { - throw new CoseException("The curve is not supported: " + algorithm.getJavaAlgorithmId(), e); + } catch (GeneralSecurityException e) { + throw new CoseException("Failed generating key.", e); } catch (IllegalArgumentException e) { throw new CoseException( "Invalid Coordinates generated for: " + algorithm.getJavaAlgorithmId(), e); From b935cc931220b9be654240d1595dfe3c3c0dfecf Mon Sep 17 00:00:00 2001 From: Andrew Scull Date: Mon, 27 Nov 2023 10:21:21 +0000 Subject: [PATCH 3/3] Use Tink for ECDSA signatures when no provider specified When there is no security provider passed to the sign and verify methods, rely on Tink to select a good and fast implementation from the available options. Currently, Tink prefers to use Conscrypt if it's available for improved performance. --- src/com/google/cose/Ec2SigningKey.java | 49 ++++++++++++++++---------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/src/com/google/cose/Ec2SigningKey.java b/src/com/google/cose/Ec2SigningKey.java index b0a1173..3734a11 100644 --- a/src/com/google/cose/Ec2SigningKey.java +++ b/src/com/google/cose/Ec2SigningKey.java @@ -32,15 +32,15 @@ import com.google.cose.utils.CborUtils; import com.google.cose.utils.CoseUtils; import com.google.cose.utils.Headers; +import com.google.crypto.tink.subtle.EcdsaSignJce; +import com.google.crypto.tink.subtle.EcdsaVerifyJce; import com.google.crypto.tink.subtle.EllipticCurves.CurveType; +import com.google.crypto.tink.subtle.EllipticCurves.EcdsaEncoding; +import com.google.crypto.tink.subtle.Enums.HashType; import java.security.GeneralSecurityException; -import java.security.InvalidKeyException; import java.security.KeyPair; -import java.security.NoSuchAlgorithmException; -import java.security.NoSuchProviderException; import java.security.PublicKey; import java.security.Signature; -import java.security.SignatureException; import java.security.interfaces.ECPrivateKey; import java.security.interfaces.ECPublicKey; import java.security.spec.ECPoint; @@ -257,18 +257,17 @@ public byte[] sign(Algorithm algorithm, byte[] message, String provider) verifyAlgorithmAllowedByKey(algorithm); verifyOperationAllowedByKey(Headers.KEY_OPERATIONS_SIGN); + ECPrivateKey key = (ECPrivateKey) keyPair.getPrivate(); try { - Signature signature; if (provider == null) { - signature = Signature.getInstance(algorithm.getJavaAlgorithmId()); - } else { - signature = Signature.getInstance(algorithm.getJavaAlgorithmId(), provider); + return new EcdsaSignJce(key, getHashType(algorithm), EcdsaEncoding.DER).sign(message); } - signature.initSign(keyPair.getPrivate()); + + Signature signature = Signature.getInstance(algorithm.getJavaAlgorithmId(), provider); + signature.initSign(key); signature.update(message); return signature.sign(); - } catch (NoSuchAlgorithmException | SignatureException | InvalidKeyException - | NoSuchProviderException e) { + } catch (GeneralSecurityException e) { throw new CoseException("Error while signing message.", e); } } @@ -279,21 +278,35 @@ public void verify(Algorithm algorithm, byte[] message, byte[] signature, String verifyAlgorithmAllowedByKey(algorithm); verifyOperationAllowedByKey(Headers.KEY_OPERATIONS_VERIFY); + ECPublicKey key = (ECPublicKey) keyPair.getPublic(); try { - Signature signer; if (provider == null) { - signer = Signature.getInstance(algorithm.getJavaAlgorithmId()); - } else { - signer = Signature.getInstance(algorithm.getJavaAlgorithmId(), provider); + new EcdsaVerifyJce(key, getHashType(algorithm), EcdsaEncoding.DER) + .verify(signature, message); + return; } - signer.initVerify(keyPair.getPublic()); + + Signature signer = Signature.getInstance(algorithm.getJavaAlgorithmId(), provider); + signer.initVerify(key); signer.update(message); if (!signer.verify(signature)) { throw new CoseException("Failed verification."); } - } catch (NoSuchAlgorithmException | NoSuchProviderException | InvalidKeyException - | SignatureException e) { + } catch (GeneralSecurityException e) { throw new CoseException("Error while verifying ", e); } } + + private static HashType getHashType(Algorithm algorithm) { + switch (algorithm) { + case SIGNING_ALGORITHM_ECDSA_SHA_256: + return HashType.SHA256; + case SIGNING_ALGORITHM_ECDSA_SHA_384: + return HashType.SHA384; + case SIGNING_ALGORITHM_ECDSA_SHA_512: + return HashType.SHA512; + default: + throw new IllegalArgumentException("Unsupported algorithm " + algorithm); + } + } }