From f8724d3f42292e01b89e24e685222eb059fa3cf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Thu, 29 Apr 2021 17:51:39 +0200 Subject: [PATCH] integrate EdDSAAuthenticationSigner into default signer using enablePreHash parameter --- .../NfcSyncPGPContentSignerBuilder.java | 101 ++++++------------ .../keychain/pgp/CanonicalizedSecretKey.java | 11 +- .../keychain/pgp/PgpKeyOperation.java | 5 +- 3 files changed, 38 insertions(+), 79 deletions(-) diff --git a/OpenKeychain/src/main/java/org/bouncycastle/openpgp/operator/jcajce/NfcSyncPGPContentSignerBuilder.java b/OpenKeychain/src/main/java/org/bouncycastle/openpgp/operator/jcajce/NfcSyncPGPContentSignerBuilder.java index dd4db13c3..771f45918 100644 --- a/OpenKeychain/src/main/java/org/bouncycastle/openpgp/operator/jcajce/NfcSyncPGPContentSignerBuilder.java +++ b/OpenKeychain/src/main/java/org/bouncycastle/openpgp/operator/jcajce/NfcSyncPGPContentSignerBuilder.java @@ -7,7 +7,6 @@ package org.bouncycastle.openpgp.operator.jcajce; -import org.bouncycastle.bcpg.PublicKeyAlgorithmTags; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPPrivateKey; import org.bouncycastle.openpgp.operator.PGPContentSigner; @@ -31,10 +30,10 @@ public class NfcSyncPGPContentSignerBuilder implements PGPContentSignerBuilder { private JcaPGPDigestCalculatorProviderBuilder digestCalculatorProviderBuilder = new JcaPGPDigestCalculatorProviderBuilder(); + private boolean enablePreHash; private int hashAlgorithm; private int keyAlgorithm; private long keyID; - private boolean isEdDsaAuthenticationSignature = false; private Map signedHashes; @@ -51,11 +50,12 @@ public class NfcSyncPGPContentSignerBuilder } } - public NfcSyncPGPContentSignerBuilder(int keyAlgorithm, int hashAlgorithm, long keyID, Map signedHashes) + public NfcSyncPGPContentSignerBuilder(int keyAlgorithm, long keyID, boolean enablePreHash, int hashAlgorithm, Map signedHashes) { this.keyAlgorithm = keyAlgorithm; - this.hashAlgorithm = hashAlgorithm; this.keyID = keyID; + this.enablePreHash = enablePreHash; + this.hashAlgorithm = hashAlgorithm; this.signedHashes = signedHashes; } @@ -87,13 +87,6 @@ public class NfcSyncPGPContentSignerBuilder return this; } - public NfcSyncPGPContentSignerBuilder configureForEdDsaAuthenticationSignature() - { - isEdDsaAuthenticationSignature = true; - - return this; - } - public PGPContentSigner build(final int signatureType, PGPPrivateKey privateKey) throws PGPException { // NOTE: privateKey is null in this case! @@ -103,64 +96,21 @@ public class NfcSyncPGPContentSignerBuilder public PGPContentSigner build(final int signatureType, final long keyID) throws PGPException { - if (isEdDsaAuthenticationSignature) { - return buildEdDSAAuthenticationSigner(signatureType, keyID); + + final PGPDigestCalculator digestCalculator; + final OutputStream outputStream; + if (enablePreHash) { + digestCalculator = digestCalculatorProviderBuilder.build().get(hashAlgorithm); + outputStream = digestCalculator.getOutputStream(); + } else { + digestCalculator = null; + outputStream = new ByteArrayOutputStream(); } - final PGPDigestCalculator digestCalculator = digestCalculatorProviderBuilder.build().get(hashAlgorithm); - return new PGPContentSigner() { - public int getType() - { - return signatureType; - } + private byte[] digest; - public int getHashAlgorithm() - { - return hashAlgorithm; - } - - public int getKeyAlgorithm() - { - return keyAlgorithm; - } - - public long getKeyID() - { - return keyID; - } - - public OutputStream getOutputStream() - { - return digestCalculator.getOutputStream(); - } - - public byte[] getSignature() { - byte[] digest = digestCalculator.getDigest(); - ByteBuffer buf = ByteBuffer.wrap(digest); - if (signedHashes.containsKey(buf)) { - return (byte[]) signedHashes.get(buf); - } - // catch this when signatureGenerator.generate() is executed and divert digest to card, - // when doing the operation again reuse creationTimestamp (this will be hashed) - throw new NfcInteractionNeeded(digest, getHashAlgorithm()); - } - - public byte[] getDigest() - { - return digestCalculator.getDigest(); - } - }; - } - - public PGPContentSigner buildEdDSAAuthenticationSigner(final int signatureType, final long keyID) - throws PGPException - { - final ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); - - return new PGPContentSigner() - { public int getType() { return signatureType; @@ -187,22 +137,31 @@ public class NfcSyncPGPContentSignerBuilder } public byte[] getSignature() { - byte[] rawData = outputStream.toByteArray(); - - ByteBuffer buf = ByteBuffer.wrap(rawData); + byte[] digest; + if (enablePreHash) { + digest = digestCalculator.getDigest(); + } else { + digest = ((ByteArrayOutputStream) getOutputStream()).toByteArray(); + } + ByteBuffer buf = ByteBuffer.wrap(digest); if (signedHashes.containsKey(buf)) { return (byte[]) signedHashes.get(buf); } - // catch this when signatureGenerator.generate() is executed and divert to card, + // catch this when signatureGenerator.generate() is executed and divert digest to card, // when doing the operation again reuse creationTimestamp (this will be hashed) - throw new NfcInteractionNeeded(rawData, getHashAlgorithm()); + throw new NfcInteractionNeeded(digest, getHashAlgorithm()); } public byte[] getDigest() { - return outputStream.toByteArray(); + if (enablePreHash) { + digest = digestCalculator.getDigest(); + } else { + digest = ((ByteArrayOutputStream) getOutputStream()).toByteArray(); + } + return digest; } }; - } + } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java index 4c1e901d6..6ce4bc7be 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java @@ -223,8 +223,8 @@ public class CanonicalizedSecretKey extends CanonicalizedPublicKey { if (mPrivateKeyState == PRIVATE_KEY_STATE_DIVERT_TO_CARD) { // use synchronous "NFC based" SignerBuilder return new NfcSyncPGPContentSignerBuilder( - mSecretKey.getPublicKey().getAlgorithm(), hashAlgo, - mSecretKey.getKeyID(), signedHashes) + mSecretKey.getPublicKey().getAlgorithm(), mSecretKey.getKeyID(), + true, hashAlgo, signedHashes) .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME); } else { // content signer based on signing key algorithm and chosen hash algorithm @@ -257,12 +257,11 @@ public class CanonicalizedSecretKey extends CanonicalizedPublicKey { byte[]> signedHashes) { if (getAlgorithm() == PublicKeyAlgorithmTags.EDDSA) { // content signer feeding the input directly into the signature engine, - // since EdDSA hashes the input anyway + // no pre-hashing for EdDSA! if (mPrivateKeyState == PRIVATE_KEY_STATE_DIVERT_TO_CARD) { return new NfcSyncPGPContentSignerBuilder( - mSecretKey.getPublicKey().getAlgorithm(), hashAlgorithm, - mSecretKey.getKeyID(), signedHashes) - .configureForEdDsaAuthenticationSignature() + mSecretKey.getPublicKey().getAlgorithm(), mSecretKey.getKeyID(), + false, hashAlgorithm, signedHashes) .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME); } else { return new EdDsaAuthenticationContentSignerBuilder( diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java index 813404c43..f73690b26 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java @@ -1511,8 +1511,9 @@ public class PgpKeyOperation { if (divertToCard) { // use synchronous "NFC based" SignerBuilder builder = new NfcSyncPGPContentSignerBuilder( - pKey.getAlgorithm(), PgpSecurityConstants.SECRET_KEY_BINDING_SIGNATURE_HASH_ALGO, - pKey.getKeyID(), cryptoInput.getCryptoData()) + pKey.getAlgorithm(), pKey.getKeyID(), + true, PgpSecurityConstants.SECRET_KEY_BINDING_SIGNATURE_HASH_ALGO, + cryptoInput.getCryptoData()) .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME); } else { // content signer based on signing key algorithm and chosen hash algorithm