Skip to content

Make better use of Tink #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Make better use of Tink #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4900d16
Use Tink utilities for converting signature format
AndrewScull Nov 27, 2023
eb5d034
Use Tink utilities in EC2 key logic
AndrewScull Nov 27, 2023
b935cc9
Use Tink for ECDSA signatures when no provider specified
AndrewScull Nov 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions src/com/google/cose/Ec2Key.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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 {
Expand All @@ -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() {
Expand Down
127 changes: 55 additions & 72 deletions src/com/google/cose/Ec2SigningKey.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -26,25 +32,21 @@
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 java.security.InvalidKeyException;
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.KeyPair;
import java.security.KeyPairGenerator;
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.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 {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);
}

Expand All @@ -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());
Copy link
Collaborator

@vikram-gaur vikram-gaur Sep 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Not sure if we would benefit from computing the value vs having a fixed value here.

Copy link
Contributor Author

@AndrewScull AndrewScull Sep 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know your preference. The current CL preferred fewer magic values.

byte[] x = integer2Bytes(pubPoint.getAffineX(), keySize);
byte[] y = integer2Bytes(pubPoint.getAffineY(), keySize);

byte[] privEncodedKey = keyPair.getPrivate().getEncoded();

Expand All @@ -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);
Expand Down Expand Up @@ -287,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);
Copy link
Collaborator

@vikram-gaur vikram-gaur Sep 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we instead do this if we are asked to use tink provider rather than putting tink by default?

Copy link
Contributor Author

@AndrewScull AndrewScull Sep 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tink isn't a security provider but a library that uses the available providers. In the case of ECDSA signatures, it has a preference for the conscrypt provider if it's available because it's known to have much better performance. I thought it worth trying to capture that performance benefit.

}
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);
}
}
Expand All @@ -309,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);
}
}
}
Loading