From 53df487e59df397220759e52e9f62648e042883b Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Fri, 10 Jun 2022 17:43:51 +0200 Subject: [PATCH] Adopt changes from SOP-Java and add test for using incapable keys --- .../misc/SignUsingPublicKeyBehaviorTest.java | 2 +- .../org/pgpainless/key/info/KeyRingInfo.java | 27 ++++++++ .../java/org/pgpainless/sop/DearmorImpl.java | 8 ++- .../org/pgpainless/sop/DetachedSignImpl.java | 7 +- .../java/org/pgpainless/sop/EncryptImpl.java | 3 + .../org/pgpainless/sop/InlineSignImpl.java | 64 +++++++----------- .../org/pgpainless/sop/IncapableKeysTest.java | 67 +++++++++++++++++++ 7 files changed, 137 insertions(+), 41 deletions(-) create mode 100644 pgpainless-sop/src/test/java/org/pgpainless/sop/IncapableKeysTest.java diff --git a/pgpainless-cli/src/test/java/org/pgpainless/cli/misc/SignUsingPublicKeyBehaviorTest.java b/pgpainless-cli/src/test/java/org/pgpainless/cli/misc/SignUsingPublicKeyBehaviorTest.java index 81562d28..affe621e 100644 --- a/pgpainless-cli/src/test/java/org/pgpainless/cli/misc/SignUsingPublicKeyBehaviorTest.java +++ b/pgpainless-cli/src/test/java/org/pgpainless/cli/misc/SignUsingPublicKeyBehaviorTest.java @@ -99,7 +99,7 @@ public class SignUsingPublicKeyBehaviorTest { } @Test - @ExpectSystemExitWithStatus(SOPGPException.BadData.EXIT_CODE) + @ExpectSystemExitWithStatus(SOPGPException.KeyCannotSign.EXIT_CODE) public void testSignatureCreationAndVerification() throws IOException { originalSout = System.out; InputStream originalIn = System.in; diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/info/KeyRingInfo.java b/pgpainless-core/src/main/java/org/pgpainless/key/info/KeyRingInfo.java index 3ea2f424..b73087b4 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/info/KeyRingInfo.java +++ b/pgpainless-core/src/main/java/org/pgpainless/key/info/KeyRingInfo.java @@ -21,6 +21,7 @@ import java.util.regex.Pattern; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import org.bouncycastle.bcpg.S2K; import org.bouncycastle.bcpg.sig.PrimaryUserID; import org.bouncycastle.bcpg.sig.RevocationReason; import org.bouncycastle.openpgp.PGPKeyRing; @@ -1039,6 +1040,32 @@ public class KeyRingInfo { return !getEncryptionSubkeys(purpose).isEmpty(); } + public boolean isUsableForSigning() { + List signingKeys = getSigningSubkeys(); + for (PGPPublicKey pk : signingKeys) { + PGPSecretKey sk = getSecretKey(pk.getKeyID()); + if (sk == null) { + // Missing secret key + continue; + } + S2K s2K = sk.getS2K(); + // Unencrypted key + if (s2K == null) { + return true; + } + + // Secret key on smart-card + int s2kType = s2K.getType(); + if (s2kType >= 100 && s2kType <= 110) { + continue; + } + // protected secret key + return true; + } + // No usable secret key found + return false; + } + private KeyAccessor getKeyAccessor(@Nullable String userId, long keyID) { if (getPublicKey(keyID) == null) { throw new NoSuchElementException("No subkey with key id " + Long.toHexString(keyID) + " found on this key."); diff --git a/pgpainless-sop/src/main/java/org/pgpainless/sop/DearmorImpl.java b/pgpainless-sop/src/main/java/org/pgpainless/sop/DearmorImpl.java index 1cebec6e..ac53d415 100644 --- a/pgpainless-sop/src/main/java/org/pgpainless/sop/DearmorImpl.java +++ b/pgpainless-sop/src/main/java/org/pgpainless/sop/DearmorImpl.java @@ -12,13 +12,19 @@ import java.io.OutputStream; import org.bouncycastle.openpgp.PGPUtil; import org.bouncycastle.util.io.Streams; import sop.Ready; +import sop.exception.SOPGPException; import sop.operation.Dearmor; public class DearmorImpl implements Dearmor { @Override public Ready data(InputStream data) throws IOException { - InputStream decoder = PGPUtil.getDecoderStream(data); + InputStream decoder; + try { + decoder = PGPUtil.getDecoderStream(data); + } catch (IOException e) { + throw new SOPGPException.BadData(e); + } return new Ready() { @Override diff --git a/pgpainless-sop/src/main/java/org/pgpainless/sop/DetachedSignImpl.java b/pgpainless-sop/src/main/java/org/pgpainless/sop/DetachedSignImpl.java index 3b985de6..704b5af5 100644 --- a/pgpainless-sop/src/main/java/org/pgpainless/sop/DetachedSignImpl.java +++ b/pgpainless-sop/src/main/java/org/pgpainless/sop/DetachedSignImpl.java @@ -25,6 +25,7 @@ import org.pgpainless.encryption_signing.ProducerOptions; import org.pgpainless.encryption_signing.SigningOptions; import org.pgpainless.exception.KeyException; import org.pgpainless.key.SubkeyIdentifier; +import org.pgpainless.key.info.KeyRingInfo; import org.pgpainless.util.ArmoredOutputStreamFactory; import org.pgpainless.util.Passphrase; import sop.MicAlg; @@ -54,11 +55,15 @@ public class DetachedSignImpl implements DetachedSign { } @Override - public DetachedSign key(InputStream keyIn) throws SOPGPException.KeyIsProtected, SOPGPException.BadData, IOException { + public DetachedSign key(InputStream keyIn) throws SOPGPException.KeyCannotSign, SOPGPException.BadData, IOException { try { PGPSecretKeyRingCollection keys = PGPainless.readKeyRing().secretKeyRingCollection(keyIn); for (PGPSecretKeyRing key : keys) { + KeyRingInfo info = PGPainless.inspectKeyRing(key); + if (!info.isUsableForSigning()) { + throw new SOPGPException.KeyCannotSign("Key " + info.getFingerprint() + " does not have valid, signing capable subkeys."); + } protector.addSecretKey(key); signingOptions.addDetachedSignature(protector, key, modeToSigType(mode)); } diff --git a/pgpainless-sop/src/main/java/org/pgpainless/sop/EncryptImpl.java b/pgpainless-sop/src/main/java/org/pgpainless/sop/EncryptImpl.java index 635ab82f..3c5f8e8a 100644 --- a/pgpainless-sop/src/main/java/org/pgpainless/sop/EncryptImpl.java +++ b/pgpainless-sop/src/main/java/org/pgpainless/sop/EncryptImpl.java @@ -21,6 +21,7 @@ import org.pgpainless.encryption_signing.EncryptionOptions; import org.pgpainless.encryption_signing.EncryptionStream; import org.pgpainless.encryption_signing.ProducerOptions; import org.pgpainless.encryption_signing.SigningOptions; +import org.pgpainless.exception.KeyException; import org.pgpainless.exception.WrongPassphraseException; import org.pgpainless.util.Passphrase; import sop.Ready; @@ -105,6 +106,8 @@ public class EncryptImpl implements Encrypt { .keyRingCollection(cert, false) .getPgpPublicKeyRingCollection(); encryptionOptions.addRecipients(certificates); + } catch (KeyException.UnacceptableEncryptionKeyException e) { + throw new SOPGPException.CertCannotEncrypt(e.getMessage(), e); } catch (IOException | PGPException e) { throw new SOPGPException.BadData(e); } diff --git a/pgpainless-sop/src/main/java/org/pgpainless/sop/InlineSignImpl.java b/pgpainless-sop/src/main/java/org/pgpainless/sop/InlineSignImpl.java index d7b0c161..639d8c6e 100644 --- a/pgpainless-sop/src/main/java/org/pgpainless/sop/InlineSignImpl.java +++ b/pgpainless-sop/src/main/java/org/pgpainless/sop/InlineSignImpl.java @@ -14,20 +14,17 @@ import java.util.List; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPSecretKeyRing; import org.bouncycastle.openpgp.PGPSecretKeyRingCollection; -import org.bouncycastle.openpgp.PGPSignature; import org.bouncycastle.util.io.Streams; import org.pgpainless.PGPainless; import org.pgpainless.algorithm.DocumentSignatureType; -import org.pgpainless.encryption_signing.EncryptionResult; import org.pgpainless.encryption_signing.EncryptionStream; import org.pgpainless.encryption_signing.ProducerOptions; import org.pgpainless.encryption_signing.SigningOptions; import org.pgpainless.exception.KeyException; -import org.pgpainless.key.SubkeyIdentifier; +import org.pgpainless.key.OpenPgpFingerprint; +import org.pgpainless.key.info.KeyRingInfo; import org.pgpainless.util.Passphrase; -import sop.MicAlg; -import sop.ReadyWithResult; -import sop.SigningResult; +import sop.Ready; import sop.enums.InlineSignAs; import sop.exception.SOPGPException; import sop.operation.InlineSign; @@ -38,6 +35,7 @@ public class InlineSignImpl implements InlineSign { private InlineSignAs mode = InlineSignAs.Binary; private final SigningOptions signingOptions = new SigningOptions(); private final MatchMakingSecretKeyRingProtector protector = new MatchMakingSecretKeyRingProtector(); + private final List signingKeys = new ArrayList<>(); @Override public InlineSign mode(InlineSignAs mode) throws SOPGPException.UnsupportedOption { @@ -52,17 +50,17 @@ public class InlineSignImpl implements InlineSign { } @Override - public InlineSign key(InputStream keyIn) throws SOPGPException.KeyIsProtected, SOPGPException.BadData, IOException { + public InlineSign key(InputStream keyIn) throws SOPGPException.KeyCannotSign, SOPGPException.BadData, IOException { try { PGPSecretKeyRingCollection keys = PGPainless.readKeyRing().secretKeyRingCollection(keyIn); for (PGPSecretKeyRing key : keys) { - protector.addSecretKey(key); - if (mode == InlineSignAs.CleartextSigned) { - signingOptions.addDetachedSignature(protector, key, DocumentSignatureType.BINARY_DOCUMENT); - } else { - signingOptions.addInlineSignature(protector, key, modeToSigType(mode)); + KeyRingInfo info = PGPainless.inspectKeyRing(key); + if (!info.isUsableForSigning()) { + throw new SOPGPException.KeyCannotSign("Key " + info.getFingerprint() + " does not have valid, signing capable subkeys."); } + protector.addSecretKey(key); + signingKeys.add(key); } } catch (PGPException | KeyException e) { throw new SOPGPException.BadData(e); @@ -78,7 +76,20 @@ public class InlineSignImpl implements InlineSign { } @Override - public ReadyWithResult data(InputStream data) throws IOException, SOPGPException.ExpectedText { + public Ready data(InputStream data) throws SOPGPException.KeyIsProtected, IOException, SOPGPException.ExpectedText { + for (PGPSecretKeyRing key : signingKeys) { + try { + if (mode == InlineSignAs.CleartextSigned) { + signingOptions.addDetachedSignature(protector, key, DocumentSignatureType.BINARY_DOCUMENT); + } else { + signingOptions.addInlineSignature(protector, key, modeToSigType(mode)); + } + } catch (KeyException.UnacceptableSigningKeyException | KeyException.MissingSecretKeyException e) { + throw new SOPGPException.KeyCannotSign("Key " + OpenPgpFingerprint.of(key) + " cannot sign.", e); + } catch (PGPException e) { + throw new SOPGPException.KeyIsProtected("Key " + OpenPgpFingerprint.of(key) + " cannot be unlocked.", e); + } + } ProducerOptions producerOptions = ProducerOptions.sign(signingOptions); if (mode == InlineSignAs.CleartextSigned) { @@ -88,9 +99,9 @@ public class InlineSignImpl implements InlineSign { producerOptions.setAsciiArmor(armor); } - return new ReadyWithResult() { + return new Ready() { @Override - public SigningResult writeTo(OutputStream outputStream) throws IOException, SOPGPException.NoSignature { + public void writeTo(OutputStream outputStream) throws IOException, SOPGPException.NoSignature { try { EncryptionStream signingStream = PGPainless.encryptAndOrSign() .onOutputStream(outputStream) @@ -102,19 +113,9 @@ public class InlineSignImpl implements InlineSign { Streams.pipeAll(data, signingStream); signingStream.close(); - EncryptionResult encryptionResult = signingStream.getResult(); // forget passphrases protector.clear(); - - List signatures = new ArrayList<>(); - for (SubkeyIdentifier key : encryptionResult.getDetachedSignatures().keySet()) { - signatures.addAll(encryptionResult.getDetachedSignatures().get(key)); - } - - return SigningResult.builder() - .setMicAlg(micAlgFromSignatures(signatures)) - .build(); } catch (PGPException e) { throw new RuntimeException(e); } @@ -122,19 +123,6 @@ public class InlineSignImpl implements InlineSign { }; } - private MicAlg micAlgFromSignatures(Iterable signatures) { - int algorithmId = 0; - for (PGPSignature signature : signatures) { - int sigAlg = signature.getHashAlgorithm(); - if (algorithmId == 0 || algorithmId == sigAlg) { - algorithmId = sigAlg; - } else { - return MicAlg.empty(); - } - } - return algorithmId == 0 ? MicAlg.empty() : MicAlg.fromHashAlgorithmId(algorithmId); - } - private static DocumentSignatureType modeToSigType(InlineSignAs mode) { return mode == InlineSignAs.Binary ? DocumentSignatureType.BINARY_DOCUMENT : DocumentSignatureType.CANONICAL_TEXT_DOCUMENT; diff --git a/pgpainless-sop/src/test/java/org/pgpainless/sop/IncapableKeysTest.java b/pgpainless-sop/src/test/java/org/pgpainless/sop/IncapableKeysTest.java new file mode 100644 index 00000000..a50acee1 --- /dev/null +++ b/pgpainless-sop/src/test/java/org/pgpainless/sop/IncapableKeysTest.java @@ -0,0 +1,67 @@ +// SPDX-FileCopyrightText: 2022 Paul Schaub +// +// SPDX-License-Identifier: Apache-2.0 + +package org.pgpainless.sop; + +import org.bouncycastle.openpgp.PGPException; +import org.bouncycastle.openpgp.PGPSecretKeyRing; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.pgpainless.PGPainless; +import org.pgpainless.algorithm.KeyFlag; +import org.pgpainless.key.generation.KeySpec; +import org.pgpainless.key.generation.type.KeyType; +import org.pgpainless.key.generation.type.ecc.EllipticCurve; +import org.pgpainless.key.generation.type.eddsa.EdDSACurve; +import org.pgpainless.util.ArmorUtils; +import sop.SOP; +import sop.exception.SOPGPException; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.security.InvalidAlgorithmParameterException; +import java.security.NoSuchAlgorithmException; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class IncapableKeysTest { + + private static byte[] nonSigningKey; + private static byte[] nonEncryptionKey; + private static byte[] nonSigningCert; + private static byte[] nonEncryptionCert; + + private static final SOP sop = new SOPImpl(); + + @BeforeAll + public static void generateKeys() throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, IOException { + PGPSecretKeyRing key = PGPainless.buildKeyRing() + .addSubkey(KeySpec.getBuilder(KeyType.ECDH(EllipticCurve._P256), KeyFlag.ENCRYPT_COMMS, KeyFlag.ENCRYPT_STORAGE)) + .setPrimaryKey(KeySpec.getBuilder(KeyType.EDDSA(EdDSACurve._Ed25519), KeyFlag.CERTIFY_OTHER)) + .addUserId("Non Signing ") + .build(); + nonSigningKey = ArmorUtils.toAsciiArmoredString(key).getBytes(StandardCharsets.UTF_8); + nonSigningCert = sop.extractCert().key(nonSigningKey).getBytes(); + + key = PGPainless.buildKeyRing() + .addSubkey(KeySpec.getBuilder(KeyType.EDDSA(EdDSACurve._Ed25519), KeyFlag.SIGN_DATA)) + .setPrimaryKey(KeySpec.getBuilder(KeyType.EDDSA(EdDSACurve._Ed25519), KeyFlag.CERTIFY_OTHER)) + .addUserId("Non Encryption ") + .build(); + nonEncryptionKey = ArmorUtils.toAsciiArmoredString(key).getBytes(StandardCharsets.UTF_8); + nonEncryptionCert = sop.extractCert().key(nonEncryptionKey).getBytes(); + } + + @Test + public void encryptionToNonEncryptionKeyFails() { + assertThrows(SOPGPException.CertCannotEncrypt.class, () -> sop.encrypt().withCert(nonEncryptionCert)); + } + + @Test + public void signingWithNonSigningKeyFails() { + assertThrows(SOPGPException.KeyCannotSign.class, () -> sop.sign().key(nonSigningKey)); + assertThrows(SOPGPException.KeyCannotSign.class, () -> sop.detachedSign().key(nonSigningKey)); + assertThrows(SOPGPException.KeyCannotSign.class, () -> sop.inlineSign().key(nonSigningKey)); + } +}