From f37aa1097c984079be5d07214dca76b8219794e9 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 | 19 +++++++++++++++++++ .../consumer/CertificateValidator.java | 6 +++--- .../java/org/pgpainless/util/ArmorUtils.java | 6 +++--- .../selection/keyring/impl/ExactUserId.java | 15 ++++++++------- 5 files changed, 37 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 1c20a06c..8353676d 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; @@ -362,8 +363,7 @@ public class KeyRingInfo { */ @Nonnull public List getUserIds() { - Iterator iterator = getPublicKey().getUserIDs(); - return iteratorToList(iterator); + return KeyRingUtils.getUserIdsIgnoringInvalidUTF8(keys.getPublicKey()); } /** @@ -1264,8 +1264,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 62ffd99a..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}. @@ -496,4 +501,18 @@ public final class KeyRingUtils { 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; }