Incorporate feedback from @IvanPizhenko. Thanks!

This commit is contained in:
Paul Schaub 2021-04-30 10:23:12 +02:00
parent 64cc9ecca4
commit ec85f53bb6
Signed by: vanitasvitae
GPG Key ID: 62BEE9264BF17311
21 changed files with 137 additions and 101 deletions

View File

@ -226,7 +226,7 @@ public enum SignatureType {
case CERTIFICATION_REVOCATION:
return true;
default:
throw new IllegalArgumentException("Unknown type: " + signatureType);
throw new IllegalArgumentException("Unknown signature type: " + signatureType);
}
}

View File

@ -26,7 +26,7 @@ import org.bouncycastle.openpgp.PGPPublicKeyRing;
import org.pgpainless.PGPainless;
import org.pgpainless.signature.DetachedSignature;
import org.pgpainless.signature.SignatureChainValidator;
import org.pgpainless.signature.SignatureValidationException;
import org.pgpainless.exception.SignatureValidationException;
import org.pgpainless.util.IntegrityProtectedInputStream;
/**

View File

@ -38,7 +38,7 @@ import org.pgpainless.key.OpenPgpV4Fingerprint;
import org.pgpainless.policy.Policy;
import org.pgpainless.signature.OnePassSignature;
import org.pgpainless.signature.SignatureChainValidator;
import org.pgpainless.signature.SignatureValidationException;
import org.pgpainless.exception.SignatureValidationException;
public class SignatureVerifyingInputStream extends FilterInputStream {

View File

@ -33,6 +33,7 @@ import org.bouncycastle.openpgp.PGPPublicKeyRing;
import org.bouncycastle.openpgp.PGPPublicKeyRingCollection;
import org.bouncycastle.openpgp.PGPSecretKey;
import org.bouncycastle.openpgp.PGPSecretKeyRing;
import org.bouncycastle.openpgp.PGPSecretKeyRingCollection;
import org.bouncycastle.openpgp.operator.PBESecretKeyDecryptor;
import org.pgpainless.PGPainless;
import org.pgpainless.algorithm.CompressionAlgorithm;
@ -176,7 +177,7 @@ public class EncryptionBuilder implements EncryptionBuilderInterface {
}
}
if (encryptionKeys.isEmpty()) {
throw new IllegalArgumentException("No suitable encryption key found in key ring " + new OpenPgpV4Fingerprint(ring));
throw new IllegalArgumentException("No suitable encryption key found in the key ring " + new OpenPgpV4Fingerprint(ring));
}
EncryptionBuilder.this.encryptionKeys.putAll(encryptionKeys);
}
@ -194,7 +195,7 @@ public class EncryptionBuilder implements EncryptionBuilderInterface {
}
}
if (encryptionKeys.isEmpty()) {
throw new IllegalArgumentException("No suitable encryption key found in key ring " + new OpenPgpV4Fingerprint(ring));
throw new IllegalArgumentException("No suitable encryption key found in the key ring " + new OpenPgpV4Fingerprint(ring));
}
EncryptionBuilder.this.encryptionKeys.putAll(encryptionKeys);
}
@ -246,6 +247,11 @@ public class EncryptionBuilder implements EncryptionBuilderInterface {
return new SignWithImpl().signWith(decryptor, keyRings);
}
@Override
public DocumentType signWith(@Nonnull SecretKeyRingProtector decryptor, @Nonnull PGPSecretKeyRingCollection keyRings) {
return new SignWithImpl().signWith(decryptor, keyRings);
}
}
class SignWithImpl implements SignWith {
@ -254,7 +260,7 @@ public class EncryptionBuilder implements EncryptionBuilderInterface {
public DocumentType signWith(@Nonnull SecretKeyRingProtector decryptor,
@Nonnull PGPSecretKeyRing... keyRings) {
if (keyRings.length == 0) {
throw new IllegalArgumentException("Recipient list MUST NOT be empty.");
throw new IllegalArgumentException("Signing key list MUST NOT be empty.");
}
for (PGPSecretKeyRing ring : keyRings) {
Map<SubkeyIdentifier, PGPSecretKeyRing> signingKeys = new ConcurrentHashMap<>();
@ -266,7 +272,7 @@ public class EncryptionBuilder implements EncryptionBuilderInterface {
}
if (signingKeys.isEmpty()) {
throw new IllegalArgumentException("No suitable signing key found in key ring " + new OpenPgpV4Fingerprint(ring));
throw new IllegalArgumentException("No suitable signing key found in the key ring " + new OpenPgpV4Fingerprint(ring));
}
EncryptionBuilder.this.signingKeys.putAll(signingKeys);
@ -274,6 +280,33 @@ public class EncryptionBuilder implements EncryptionBuilderInterface {
EncryptionBuilder.this.signingKeysDecryptor = decryptor;
return new DocumentTypeImpl();
}
@Override
public DocumentType signWith(@Nonnull SecretKeyRingProtector decryptor, @Nonnull PGPSecretKeyRingCollection keyRings) {
Iterator<PGPSecretKeyRing> iterator = keyRings.iterator();
if (!iterator.hasNext()) {
throw new IllegalArgumentException("Signing key collection MUST NOT be empty.");
}
while (iterator.hasNext()) {
PGPSecretKeyRing ring = iterator.next();
Map<SubkeyIdentifier, PGPSecretKeyRing> signingKeys = new ConcurrentHashMap<>();
for (Iterator<PGPSecretKey> i = ring.getSecretKeys(); i.hasNext(); ) {
PGPSecretKey s = i.next();
if (EncryptionBuilder.this.signingKeySelector().accept(s)) {
signingKeys.put(new SubkeyIdentifier(ring, s.getKeyID()), ring);
}
}
if (signingKeys.isEmpty()) {
throw new IllegalArgumentException("No suitable signing key found in the key ring " + new OpenPgpV4Fingerprint(ring));
}
EncryptionBuilder.this.signingKeys.putAll(signingKeys);
}
EncryptionBuilder.this.signingKeysDecryptor = decryptor;
return new DocumentTypeImpl();
}
}
class DocumentTypeImpl implements DocumentType {

View File

@ -24,6 +24,7 @@ import org.bouncycastle.openpgp.PGPException;
import org.bouncycastle.openpgp.PGPPublicKeyRing;
import org.bouncycastle.openpgp.PGPPublicKeyRingCollection;
import org.bouncycastle.openpgp.PGPSecretKeyRing;
import org.bouncycastle.openpgp.PGPSecretKeyRingCollection;
import org.pgpainless.algorithm.CompressionAlgorithm;
import org.pgpainless.algorithm.HashAlgorithm;
import org.pgpainless.algorithm.StreamEncoding;
@ -191,6 +192,8 @@ public interface EncryptionBuilderInterface {
*/
DocumentType signWith(@Nonnull SecretKeyRingProtector decryptor, @Nonnull PGPSecretKeyRing... keyRings);
DocumentType signWith(@Nonnull SecretKeyRingProtector decryptor, @Nonnull PGPSecretKeyRingCollection keyRings);
}
interface DocumentType {

View File

@ -213,7 +213,7 @@ public final class EncryptionStream extends OutputStream {
private void prepareCompression() throws IOException {
compressedDataGenerator = new PGPCompressedDataGenerator(
compressionAlgorithm.getAlgorithmId());
compressionAlgorithm.getAlgorithmId());
if (compressionAlgorithm == CompressionAlgorithm.UNCOMPRESSED) {
return;
}
@ -290,7 +290,11 @@ public final class EncryptionStream extends OutputStream {
literalDataStream.close();
literalDataGenerator.close();
writeSignatures();
try {
writeSignatures();
} catch (PGPException e) {
throw new IOException("Exception while writing signatures.", e);
}
// Compressed Data
compressedDataGenerator.close();
@ -309,20 +313,16 @@ public final class EncryptionStream extends OutputStream {
closed = true;
}
private void writeSignatures() throws IOException {
private void writeSignatures() throws PGPException, IOException {
for (SubkeyIdentifier signingKey : signatureGenerators.keySet()) {
PGPSignatureGenerator signatureGenerator = signatureGenerators.get(signingKey).getSecond();
try {
PGPSignature signature = signatureGenerator.generate();
if (!detachedSignature) {
signature.encode(outermostStream);
}
DetachedSignature detachedSignature = new DetachedSignature(
signature, signatureGenerators.get(signingKey).getFirst(), signingKey);
resultBuilder.addDetachedSignature(detachedSignature);
} catch (PGPException e) {
throw new IOException(e);
PGPSignature signature = signatureGenerator.generate();
if (!detachedSignature) {
signature.encode(outermostStream);
}
DetachedSignature detachedSignature = new DetachedSignature(
signature, signatureGenerators.get(signingKey).getFirst(), signingKey);
resultBuilder.addDetachedSignature(detachedSignature);
}
}

View File

@ -13,20 +13,21 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.pgpainless.signature;
package org.pgpainless.exception;
import java.util.Map;
import org.bouncycastle.openpgp.PGPException;
import org.bouncycastle.openpgp.PGPSignature;
import org.pgpainless.algorithm.SignatureType;
public class SignatureValidationException extends Exception {
public class SignatureValidationException extends PGPException {
public SignatureValidationException(String message) {
super(message);
}
public SignatureValidationException(String message, Throwable underlying) {
public SignatureValidationException(String message, Exception underlying) {
super(message, underlying);
}
@ -35,11 +36,13 @@ public class SignatureValidationException extends Exception {
}
private static String exceptionMapToString(Map<PGPSignature, Exception> rejections) {
String out = "";
out += rejections.size() + " Rejected signatures:\n";
StringBuilder sb = new StringBuilder();
sb.append(rejections.size()).append(" rejected signatures:\n");
for (PGPSignature signature : rejections.keySet()) {
out += SignatureType.valueOf(signature.getSignatureType()) + " " + signature.getCreationTime() + ": " + rejections.get(signature).getMessage() + "\n";
sb.append(SignatureType.valueOf(signature.getSignatureType())).append(' ')
.append(signature.getCreationTime()).append(": ")
.append(rejections.get(signature).getMessage()).append('\n');
}
return out;
};
return sb.toString();
}
}

View File

@ -36,7 +36,7 @@ import org.pgpainless.key.util.KeyRingUtils;
import org.pgpainless.policy.Policy;
import org.pgpainless.signature.SelectSignatureFromKey;
import org.pgpainless.signature.SignatureCreationDateComparator;
import org.pgpainless.signature.SignatureValidationException;
import org.pgpainless.exception.SignatureValidationException;
import org.pgpainless.signature.SignatureValidator;
import org.pgpainless.util.CollectionUtils;
@ -74,7 +74,7 @@ public class KeyRingValidator {
Iterator<PGPSignature> directKeyIterator = primaryKey.getSignaturesOfType(SignatureType.DIRECT_KEY.getCode());
List<PGPSignature> directKeyCertifications = CollectionUtils.iteratorToList(directKeyIterator);
Collections.sort(directKeyCertifications, new SignatureCreationDateComparator(SignatureCreationDateComparator.Order.new_to_old));
Collections.sort(directKeyCertifications, new SignatureCreationDateComparator(SignatureCreationDateComparator.Order.NEW_TO_OLD));
for (PGPSignature signature : directKeyCertifications) {
try {
if (SignatureValidator.verifyDirectKeySignature(signature, blank, policy, validationDate)) {
@ -87,7 +87,7 @@ public class KeyRingValidator {
Iterator<PGPSignature> revocationIterator = primaryKey.getSignaturesOfType(SignatureType.KEY_REVOCATION.getCode());
List<PGPSignature> directKeyRevocations = CollectionUtils.iteratorToList(revocationIterator);
Collections.sort(directKeyRevocations, new SignatureCreationDateComparator(SignatureCreationDateComparator.Order.new_to_old));
Collections.sort(directKeyRevocations, new SignatureCreationDateComparator(SignatureCreationDateComparator.Order.NEW_TO_OLD));
for (PGPSignature signature : directKeyRevocations) {
try {
if (SignatureValidator.verifyKeyRevocationSignature(signature, primaryKey, policy, validationDate)) {
@ -103,7 +103,7 @@ public class KeyRingValidator {
String userId = userIdIterator.next();
Iterator<PGPSignature> userIdSigs = primaryKey.getSignaturesForID(userId);
List<PGPSignature> signatures = CollectionUtils.iteratorToList(userIdSigs);
Collections.sort(signatures, new SignatureCreationDateComparator(SignatureCreationDateComparator.Order.new_to_old));
Collections.sort(signatures, new SignatureCreationDateComparator(SignatureCreationDateComparator.Order.NEW_TO_OLD));
for (PGPSignature signature : signatures) {
try {
if (SignatureType.valueOf(signature.getSignatureType()) == SignatureType.CERTIFICATION_REVOCATION) {

View File

@ -143,11 +143,11 @@ public class KeyRingInfo {
if (publicKey == getPublicKey()) {
return revocationSelfSignature == null;
} else {
PGPSignature binding = mostRecentSubkeyBindings.get(keyId);
PGPSignature revocation = mostRecentSubkeyRevocations.get(keyId);
return binding != null && revocation == null;
}
PGPSignature binding = mostRecentSubkeyBindings.get(keyId);
PGPSignature revocation = mostRecentSubkeyRevocations.get(keyId);
return binding != null && revocation == null;
}
/**
@ -264,10 +264,7 @@ public class KeyRingInfo {
PGPSignature certification = mostRecentUserIdSignatures.get(userId);
PGPSignature revocation = mostRecentUserIdRevocations.get(userId);
if (certification == null) {
return false;
}
return revocation == null;
return certification != null && revocation == null;
}
/**

View File

@ -31,6 +31,7 @@ import org.bouncycastle.openpgp.PGPPublicKeyRing;
import org.bouncycastle.openpgp.PGPSignature;
import org.pgpainless.algorithm.KeyFlag;
import org.pgpainless.algorithm.SignatureType;
import org.pgpainless.exception.SignatureValidationException;
import org.pgpainless.policy.Policy;
import org.pgpainless.signature.subpackets.SignatureSubpacketsUtil;
@ -76,7 +77,7 @@ public class SignatureChainValidator {
}
}
Collections.sort(directKeySignatures, new SignatureValidityComparator(SignatureCreationDateComparator.Order.new_to_old));
Collections.sort(directKeySignatures, new SignatureValidityComparator(SignatureCreationDateComparator.Order.NEW_TO_OLD));
if (directKeySignatures.isEmpty()) {
} else {
@ -102,7 +103,7 @@ public class SignatureChainValidator {
LOGGER.log(Level.INFO, "Rejecting user-id signature.", e);
}
}
Collections.sort(signaturesOnUserId, new SignatureValidityComparator(SignatureCreationDateComparator.Order.new_to_old));
Collections.sort(signaturesOnUserId, new SignatureValidityComparator(SignatureCreationDateComparator.Order.NEW_TO_OLD));
userIdSignatures.put(userId, signaturesOnUserId);
}
@ -150,7 +151,7 @@ public class SignatureChainValidator {
}
}
Collections.sort(subkeySigs, new SignatureValidityComparator(SignatureCreationDateComparator.Order.new_to_old));
Collections.sort(subkeySigs, new SignatureValidityComparator(SignatureCreationDateComparator.Order.NEW_TO_OLD));
if (subkeySigs.isEmpty()) {
throw new SignatureValidationException("Subkey is not bound.", rejections);
}

View File

@ -21,11 +21,11 @@ import org.bouncycastle.openpgp.PGPSignature;
public class SignatureCreationDateComparator implements Comparator<PGPSignature> {
public static final Order DEFAULT_ORDER = Order.old_to_new;
public static final Order DEFAULT_ORDER = Order.OLD_TO_NEW;
public enum Order {
old_to_new,
new_to_old
OLD_TO_NEW,
NEW_TO_OLD
}
private final Order order;
@ -40,7 +40,7 @@ public class SignatureCreationDateComparator implements Comparator<PGPSignature>
@Override
public int compare(PGPSignature one, PGPSignature two) {
return order == Order.old_to_new
return order == Order.OLD_TO_NEW
? one.getCreationTime().compareTo(two.getCreationTime())
: two.getCreationTime().compareTo(one.getCreationTime());
}

View File

@ -170,6 +170,16 @@ public class SignatureUtils {
return datePlusSeconds(creationDate, expiresInSecs);
}
/**
* Return a new date which represents the given date plus the given amount of seconds added.
*
* Since '0' is a special value in the OpenPGP specification when it comes to dates
* (e.g. '0' means no expiration for expiration dates), this method will return 'null' if seconds is 0.
*
* @param date date
* @param seconds number of seconds to be added
* @return date plus seconds or null if seconds is '0'
*/
public static Date datePlusSeconds(Date date, long seconds) {
if (seconds == 0) {
return null;
@ -183,10 +193,7 @@ public class SignatureUtils {
public static boolean isSignatureExpired(PGPSignature signature, Date comparisonDate) {
Date expirationDate = getSignatureExpirationDate(signature);
if (expirationDate == null) {
return false;
}
return comparisonDate.after(expirationDate);
return expirationDate != null && comparisonDate.after(expirationDate);
}
public static void sortByCreationTimeAscending(List<PGPSignature> signatures) {

View File

@ -36,6 +36,7 @@ import org.pgpainless.algorithm.HashAlgorithm;
import org.pgpainless.algorithm.PublicKeyAlgorithm;
import org.pgpainless.algorithm.SignatureSubpacket;
import org.pgpainless.algorithm.SignatureType;
import org.pgpainless.exception.SignatureValidationException;
import org.pgpainless.implementation.ImplementationFactory;
import org.pgpainless.policy.Policy;
import org.pgpainless.signature.subpackets.SignatureSubpacketsUtil;

View File

@ -35,13 +35,12 @@ public class SignatureValidityComparator implements Comparator<PGPSignature> {
@Override
public int compare(PGPSignature one, PGPSignature two) {
int compareByCreationTime = creationDateComparator.compare(one, two);
boolean oneIsHard = SignatureUtils.isHardRevocation(one);
boolean twoIsHard = SignatureUtils.isHardRevocation(two);
// both have same "hardness", so compare creation time
if (oneIsHard == twoIsHard) {
return compareByCreationTime;
return creationDateComparator.compare(one, two);
}
// favor the "harder" signature
return oneIsHard ? -1 : 1;

View File

@ -259,8 +259,15 @@ public class BCUtil {
return ring.getSecretKey(keyId) != null;
}
public static PGPSignatureList readSignatures(String encoding) throws IOException {
InputStream inputStream = getPgpDecoderInputStream(encoding.getBytes(Charset.forName("UTF8")));
/**
* Parse an ASCII encoded list of OpenPGP signatures into a {@link PGPSignatureList}.
*
* @param encodedSignatures ASCII armored signature list
* @return signature list
* @throws IOException if the signatures cannot be read
*/
public static PGPSignatureList readSignatures(String encodedSignatures) throws IOException {
InputStream inputStream = getPgpDecoderInputStream(encodedSignatures.getBytes(Charset.forName("UTF8")));
PGPObjectFactory objectFactory = new PGPObjectFactory(inputStream, ImplementationFactory.getInstance().getKeyFingerprintCalculator());
Object next = objectFactory.nextObject();
while (next != null) {
@ -271,5 +278,5 @@ public class BCUtil {
return (PGPSignatureList) next;
}
return null;
};
}
}

View File

@ -16,19 +16,19 @@
package org.pgpainless.util;
public class Tuple<A, B> {
private final A a;
private final B b;
private final A first;
private final B second;
public Tuple(A a, B b) {
this.a = a;
this.b = b;
public Tuple(A first, B second) {
this.first = first;
this.second = second;
}
public A getFirst() {
return a;
return first;
}
public B getSecond() {
return b;
return second;
}
}

View File

@ -17,16 +17,9 @@ package org.junit;
import static org.junit.jupiter.api.Assertions.assertTrue;
import org.junit.jupiter.api.Test;
public class JUtils {
public static void assertEquals(long a, long b, long delta) {
assertTrue(a - delta <= b && a + delta >= b);
}
@Test
public void comparatorLearningTest() {
assertEquals(-1, Integer.compare(5,6), 0);
}
}

View File

@ -28,6 +28,7 @@ import org.bouncycastle.openpgp.PGPPublicKeyRing;
import org.bouncycastle.openpgp.PGPSignature;
import org.junit.jupiter.api.Test;
import org.pgpainless.PGPainless;
import org.pgpainless.exception.SignatureValidationException;
import org.pgpainless.policy.Policy;
import org.pgpainless.util.BCUtil;

View File

@ -26,6 +26,7 @@ import org.bouncycastle.openpgp.PGPPublicKeyRing;
import org.bouncycastle.openpgp.PGPSignature;
import org.junit.jupiter.api.Test;
import org.pgpainless.PGPainless;
import org.pgpainless.exception.SignatureValidationException;
import org.pgpainless.util.BCUtil;
public class KeyRevocationTest {

View File

@ -28,6 +28,7 @@ import org.bouncycastle.openpgp.PGPPublicKeyRing;
import org.bouncycastle.openpgp.PGPSignature;
import org.junit.jupiter.api.Test;
import org.pgpainless.PGPainless;
import org.pgpainless.exception.SignatureValidationException;
import org.pgpainless.policy.Policy;
import org.pgpainless.util.BCUtil;

View File

@ -15,10 +15,13 @@
*/
package org.pgpainless.util.selection.signature;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import java.io.IOException;
import java.util.Iterator;
import org.bouncycastle.openpgp.PGPException;
import org.bouncycastle.openpgp.PGPPublicKey;
import org.bouncycastle.openpgp.PGPPublicKeyRing;
import org.bouncycastle.openpgp.PGPSignature;
@ -29,7 +32,7 @@ import org.pgpainless.signature.SelectSignatureFromKey;
public class SelectSignatureFromKeyTest {
@Test
public void validKeyTest() throws IOException, PGPException {
public void validKeyTest() throws IOException {
String key = "-----BEGIN PGP PUBLIC KEY BLOCK-----\n" +
"\n" +
"xsDNBF2lnPIBDAC5cL9PQoQLTMuhjbYvb4Ncuuo0bfmgPRFywX53jPhoFf4Zg6mv\n" +
@ -90,19 +93,19 @@ public class SelectSignatureFromKeyTest {
PGPPublicKey primaryKey = publicKeys.getPublicKey();
while (keyIt.hasNext()) {
PGPPublicKey publicKey = keyIt.next();
// CHECKSTYLE:OFF
System.out.println(publicKey.getKeyID());
// CHECKSTYLE:ON
if (publicKey == primaryKey) {
continue;
}
boolean validBinding = false;
Iterator<PGPSignature> signatures = publicKey.getSignatures();
while (signatures.hasNext()) {
PGPSignature signature = signatures.next();
if (SelectSignatureFromKey.isValidSubkeyBindingSignature(primaryKey, publicKey).accept(signature, publicKey, publicKeys)) {
// CHECKSTYLE:OFF
System.out.println("Valid subkey binding signature");
// CHECKSTYLE:ON
validBinding = true;
}
}
assertTrue(validBinding);
}
}
@ -156,31 +159,17 @@ public class SelectSignatureFromKeyTest {
PGPPublicKey primaryKey = publicKeys.getPublicKey();
while (keyIt.hasNext()) {
PGPPublicKey publicKey = keyIt.next();
// CHECKSTYLE:OFF
System.out.println(publicKey.getKeyID());
// CHECKSTYLE:ON
if (publicKey.isMasterKey()) {
Iterator<PGPSignature> signatures = publicKey.getSignatures();
boolean isValidPrimaryKey = false;
boolean isRevokedPrimaryKey = false;
while (signatures.hasNext()) {
PGPSignature signature = signatures.next();
}
} else {
Iterator<PGPSignature> signatures = publicKey.getSignatures();
while (signatures.hasNext()) {
PGPSignature signature = signatures.next();
if (SelectSignatureFromKey.isValidSubkeyBindingSignature(primaryKey, publicKey).accept(signature, publicKey, publicKeys)) {
// CHECKSTYLE:OFF
System.out.println("Valid subkey binding signature");
// CHECKSTYLE:ON
}
}
if (publicKey == primaryKey) {
continue;
}
Iterator<PGPSignature> signatures = publicKey.getSignatures();
while (signatures.hasNext()) {
PGPSignature signature = signatures.next();
if (SelectSignatureFromKey.isValidSubkeyBindingSignature(primaryKey, publicKey).accept(signature, publicKey, publicKeys)) {
fail("Implementation MUST NOT accept this subkey as bound valid since the backsig is missing.");
}
}
}
}
}