From 7d0dc5e40dcb92e60ddef5f77638346ef2ccb29d Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Tue, 4 Jul 2023 14:45:09 +0200 Subject: [PATCH] Prevent IAE when encountering non-UTF8 User-ID on a public key Fixes #392 --- .../org/pgpainless/key/info/KeyRingInfo.java | 8 ++--- .../org/pgpainless/key/util/KeyRingUtils.java | 30 +++++++++++++++++++ .../consumer/CertificateValidator.java | 6 ++-- .../java/org/pgpainless/util/ArmorUtils.java | 6 ++-- .../selection/keyring/impl/ExactUserId.java | 15 +++++----- 5 files changed, 48 insertions(+), 17 deletions(-) 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 d906ab14..77277622 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 @@ -42,6 +42,7 @@ import org.pgpainless.exception.KeyException; import org.pgpainless.key.OpenPgpFingerprint; import org.pgpainless.key.SubkeyIdentifier; import org.pgpainless.key.util.KeyIdUtil; +import org.pgpainless.key.util.KeyRingUtils; import org.pgpainless.key.util.RevocationAttributes; import org.pgpainless.policy.Policy; import org.pgpainless.signature.SignatureUtils; @@ -382,8 +383,7 @@ public class KeyRingInfo { */ @Nonnull public List getUserIds() { - Iterator iterator = getPublicKey().getUserIDs(); - return iteratorToList(iterator); + return KeyRingUtils.getUserIdsIgnoringInvalidUTF8(keys.getPublicKey()); } /** @@ -1284,8 +1284,8 @@ public class KeyRingInfo { subkeyRevocations = new HashMap<>(); subkeyBindings = new HashMap<>(); - for (Iterator it = keyRing.getPublicKey().getUserIDs(); it.hasNext(); ) { - String userId = it.next(); + List userIds = KeyRingUtils.getUserIdsIgnoringInvalidUTF8(keyRing.getPublicKey()); + for (String userId : userIds) { PGPSignature revocation = SignaturePicker.pickCurrentUserIdRevocationSignature(keyRing, userId, policy, referenceDate); if (revocation != null) { userIdRevocations.put(userId, revocation); diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/util/KeyRingUtils.java b/pgpainless-core/src/main/java/org/pgpainless/key/util/KeyRingUtils.java index 23361c13..4689535d 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/util/KeyRingUtils.java +++ b/pgpainless-core/src/main/java/org/pgpainless/key/util/KeyRingUtils.java @@ -25,10 +25,13 @@ import org.bouncycastle.openpgp.PGPSecretKeyRing; import org.bouncycastle.openpgp.PGPSecretKeyRingCollection; import org.bouncycastle.openpgp.PGPSignature; import org.bouncycastle.openpgp.PGPUserAttributeSubpacketVector; +import org.bouncycastle.util.Strings; import org.pgpainless.PGPainless; import org.pgpainless.implementation.ImplementationFactory; import org.pgpainless.key.protection.SecretKeyRingProtector; import org.pgpainless.key.protection.UnlockSecretKey; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public final class KeyRingUtils { @@ -36,6 +39,8 @@ public final class KeyRingUtils { } + private static final Logger LOGGER = LoggerFactory.getLogger(KeyRingUtils.class); + /** * Return the primary {@link PGPSecretKey} from the provided {@link PGPSecretKeyRing}. * If it has no primary secret key, throw a {@link NoSuchElementException}. @@ -485,4 +490,29 @@ public final class KeyRingUtils { // Parse the key back into an object return new PGPSecretKeyRing(encoded.toByteArray(), ImplementationFactory.getInstance().getKeyFingerprintCalculator()); } + + /** + * Strip all user-ids, user-attributes and signatures from the given public key. + * + * @param bloatedKey public key + * @return stripped public key + * @throws PGPException if the packet is faulty or the required calculations fail + */ + public static PGPPublicKey getStrippedDownPublicKey(PGPPublicKey bloatedKey) throws PGPException { + return new PGPPublicKey(bloatedKey.getPublicKeyPacket(), ImplementationFactory.getInstance().getKeyFingerprintCalculator()); + } + + public static List getUserIdsIgnoringInvalidUTF8(PGPPublicKey key) { + List userIds = new ArrayList<>(); + Iterator it = key.getRawUserIDs(); + while (it.hasNext()) { + byte[] rawUserId = it.next(); + try { + userIds.add(Strings.fromUTF8ByteArray(rawUserId)); + } catch (IllegalArgumentException e) { + LOGGER.warn("Invalid UTF-8 user-ID encountered: " + new String(rawUserId)); + } + } + return userIds; + } } diff --git a/pgpainless-core/src/main/java/org/pgpainless/signature/consumer/CertificateValidator.java b/pgpainless-core/src/main/java/org/pgpainless/signature/consumer/CertificateValidator.java index 6cd3e328..f22e057a 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/signature/consumer/CertificateValidator.java +++ b/pgpainless-core/src/main/java/org/pgpainless/signature/consumer/CertificateValidator.java @@ -23,6 +23,7 @@ import org.bouncycastle.openpgp.PGPSignature; import org.pgpainless.algorithm.KeyFlag; import org.pgpainless.algorithm.SignatureType; import org.pgpainless.exception.SignatureValidationException; +import org.pgpainless.key.util.KeyRingUtils; import org.pgpainless.policy.Policy; import org.pgpainless.signature.SignatureUtils; import org.pgpainless.signature.subpackets.SignatureSubpacketsUtil; @@ -113,11 +114,10 @@ public final class CertificateValidator { } // User-ID signatures (certifications, revocations) - Iterator userIds = primaryKey.getUserIDs(); + List userIds = KeyRingUtils.getUserIdsIgnoringInvalidUTF8(primaryKey); Map> userIdSignatures = new ConcurrentHashMap<>(); - while (userIds.hasNext()) { + for (String userId : userIds) { List signaturesOnUserId = new ArrayList<>(); - String userId = userIds.next(); Iterator userIdSigs = primaryKey.getSignaturesForID(userId); while (userIdSigs.hasNext()) { PGPSignature userIdSig = userIdSigs.next(); diff --git a/pgpainless-core/src/main/java/org/pgpainless/util/ArmorUtils.java b/pgpainless-core/src/main/java/org/pgpainless/util/ArmorUtils.java index cf19d273..976f6ab2 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/util/ArmorUtils.java +++ b/pgpainless-core/src/main/java/org/pgpainless/util/ArmorUtils.java @@ -32,6 +32,7 @@ import org.bouncycastle.util.io.Streams; import org.pgpainless.algorithm.HashAlgorithm; import org.pgpainless.decryption_verification.OpenPgpInputStream; import org.pgpainless.key.OpenPgpFingerprint; +import org.pgpainless.key.util.KeyRingUtils; /** * Utility class for dealing with ASCII armored OpenPGP data. @@ -388,13 +389,12 @@ public final class ArmorUtils { // Quickly determine the primary user-id + number of total user-ids // NOTE: THIS METHOD DOES NOT CRYPTOGRAPHICALLY VERIFY THE SIGNATURES // DO NOT RELY ON IT! - Iterator userIds = publicKey.getUserIDs(); + List userIds = KeyRingUtils.getUserIdsIgnoringInvalidUTF8(publicKey); int countIdentities = 0; String first = null; String primary = null; - while (userIds.hasNext()) { + for (String userId : userIds) { countIdentities++; - String userId = userIds.next(); // remember the first user-id if (first == null) { first = userId; diff --git a/pgpainless-core/src/main/java/org/pgpainless/util/selection/keyring/impl/ExactUserId.java b/pgpainless-core/src/main/java/org/pgpainless/util/selection/keyring/impl/ExactUserId.java index 29c43c41..ffda8867 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/util/selection/keyring/impl/ExactUserId.java +++ b/pgpainless-core/src/main/java/org/pgpainless/util/selection/keyring/impl/ExactUserId.java @@ -4,10 +4,11 @@ package org.pgpainless.util.selection.keyring.impl; -import java.util.Iterator; +import java.util.List; import org.bouncycastle.openpgp.PGPPublicKeyRing; import org.bouncycastle.openpgp.PGPSecretKeyRing; +import org.pgpainless.key.util.KeyRingUtils; import org.pgpainless.util.selection.keyring.PublicKeyRingSelectionStrategy; import org.pgpainless.util.selection.keyring.SecretKeyRingSelectionStrategy; @@ -29,9 +30,9 @@ public final class ExactUserId { @Override public boolean accept(String identifier, PGPPublicKeyRing keyRing) { - Iterator userIds = keyRing.getPublicKey().getUserIDs(); - while (userIds.hasNext()) { - if (userIds.next().equals(identifier)) return true; + List userIds = KeyRingUtils.getUserIdsIgnoringInvalidUTF8(keyRing.getPublicKey()); + for (String userId : userIds) { + if (userId.equals(identifier)) return true; } return false; } @@ -45,9 +46,9 @@ public final class ExactUserId { @Override public boolean accept(String identifier, PGPSecretKeyRing keyRing) { - Iterator userIds = keyRing.getPublicKey().getUserIDs(); - while (userIds.hasNext()) { - if (userIds.next().equals(identifier)) return true; + List userIds = KeyRingUtils.getUserIdsIgnoringInvalidUTF8(keyRing.getPublicKey()); + for (String userId : userIds) { + if (userId.equals(identifier)) return true; } return false; }