Implement configuration option for SignerUserId subpacket verification level.

By default we ignore SignerUserId subpackets on signatures.
This behavior can be changed by calling Policy.setSignerUserIdValidationLevel().
Right now, STRICT and DISABLED are available as options, but it may make sense to implement
another option PARTIALLY, which will accept signatures made by key with user-id 'A <foo@bar>'
but where the sig contains a signer user id of value 'foo@bar' for example.
This commit is contained in:
Paul Schaub 2022-03-11 18:23:40 +01:00
parent 661c043cdc
commit ffdbd21491
3 changed files with 99 additions and 25 deletions

View File

@ -42,6 +42,25 @@ public final class Policy {
private AlgorithmSuite keyGenerationAlgorithmSuite = AlgorithmSuite.getDefaultAlgorithmSuite();
// Signers User-ID is soon to be deprecated.
private SignerUserIdValidationLevel signerUserIdValidationLevel = SignerUserIdValidationLevel.DISABLED;
public enum SignerUserIdValidationLevel {
/**
* PGPainless will verify {@link org.bouncycastle.bcpg.sig.SignerUserID} subpackets in signatures strictly.
* This means, that signatures with Signer's User-ID subpackets containing a value that does not match the signer key's
* user-id exactly, will be rejected.
* E.g. Signer's user-id "alice@pgpainless.org", User-ID: "Alice &lt;alice@pgpainless.org&gt;" does not
* match exactly and is therefore rejected.
*/
STRICT,
/**
* PGPainless will ignore {@link org.bouncycastle.bcpg.sig.SignerUserID} subpackets on signature.
*/
DISABLED
}
Policy() {
}
@ -468,4 +487,29 @@ public final class Policy {
public void setKeyGenerationAlgorithmSuite(@Nonnull AlgorithmSuite algorithmSuite) {
this.keyGenerationAlgorithmSuite = algorithmSuite;
}
/**
* Return the level of validation PGPainless shall do on {@link org.bouncycastle.bcpg.sig.SignerUserID} subpackets.
* By default, this value is {@link SignerUserIdValidationLevel#DISABLED}.
*
* @return the level of validation
*/
public SignerUserIdValidationLevel getSignerUserIdValidationLevel() {
return signerUserIdValidationLevel;
}
/**
* Specify, how {@link org.bouncycastle.bcpg.sig.SignerUserID} subpackets on signatures shall be validated.
*
* @param signerUserIdValidationLevel level of verification PGPainless shall do on
* {@link org.bouncycastle.bcpg.sig.SignerUserID} subpackets.
* @return policy instance
*/
public Policy setSignerUserIdValidationLevel(SignerUserIdValidationLevel signerUserIdValidationLevel) {
if (signerUserIdValidationLevel == null) {
throw new NullPointerException("SignerUserIdValidationLevel cannot be null.");
}
this.signerUserIdValidationLevel = signerUserIdValidationLevel;
return this;
}
}

View File

@ -143,7 +143,7 @@ public final class CertificateValidator {
// Specific signer user-id
SignerUserID signerUserID = SignatureSubpacketsUtil.getSignerUserID(signature);
if (signerUserID != null) {
if (signerUserID != null && policy.getSignerUserIdValidationLevel() == Policy.SignerUserIdValidationLevel.STRICT) {
List<PGPSignature> signerUserIdSigs = userIdSignatures.get(signerUserID.getID());
if (signerUserIdSigs == null || signerUserIdSigs.isEmpty()) {
throw new SignatureValidationException("Signature was allegedly made by user-id '" + signerUserID.getID() +

View File

@ -2,9 +2,11 @@
//
// SPDX-License-Identifier: Apache-2.0
package investigations;
package org.pgpainless.decryption_verification;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
@ -34,17 +36,17 @@ import org.bouncycastle.openpgp.operator.bc.BcPGPContentSignerBuilder;
import org.bouncycastle.openpgp.operator.bc.BcPGPDataEncryptorBuilder;
import org.bouncycastle.openpgp.operator.bc.BcPublicKeyKeyEncryptionMethodGenerator;
import org.bouncycastle.util.io.Streams;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.Test;
import org.pgpainless.PGPainless;
import org.pgpainless.algorithm.HashAlgorithm;
import org.pgpainless.decryption_verification.ConsumerOptions;
import org.pgpainless.decryption_verification.DecryptionStream;
import org.pgpainless.exception.SignatureValidationException;
import org.pgpainless.key.protection.UnlockSecretKey;
import org.pgpainless.policy.Policy;
import org.pgpainless.util.Passphrase;
public class WrongSignerUserIdTest {
private static final String CERT = "-----BEGIN PGP PRIVATE KEY BLOCK-----\n" +
private static final String KEY = "-----BEGIN PGP PRIVATE KEY BLOCK-----\n" +
" Comment: Alice's OpenPGP Transferable Secret Key\n" +
" Comment: https://www.ietf.org/id/draft-bre-openpgp-samples-01.html\n" +
"\n" +
@ -63,13 +65,54 @@ public class WrongSignerUserIdTest {
" -----END PGP PRIVATE KEY BLOCK-----";
private static final String USER_ID = "Alice Lovelace <alice@openpgp.example>";
public static void main(String[] args) throws Exception {
WrongSignerUserIdTest test = new WrongSignerUserIdTest();
test.execute();
@Test
public void verificationSucceedsWithDisabledCheck() throws PGPException, IOException {
executeTest(false, true);
}
public void execute() throws PGPException, IOException {
PGPSecretKeyRing secretKeys = PGPainless.readKeyRing().secretKeyRing(CERT);
@Test
public void verificationFailsWithEnabledCheck() throws PGPException, IOException {
executeTest(true, false);
}
@AfterAll
public static void resetDefault() {
PGPainless.getPolicy().setSignerUserIdValidationLevel(Policy.SignerUserIdValidationLevel.DISABLED);
}
public void executeTest(boolean enableCheck, boolean expectSucessfulVerification) throws IOException, PGPException {
PGPainless.getPolicy().setSignerUserIdValidationLevel(enableCheck ? Policy.SignerUserIdValidationLevel.STRICT : Policy.SignerUserIdValidationLevel.DISABLED);
PGPSecretKeyRing secretKeys = PGPainless.readKeyRing().secretKeyRing(KEY);
assertEquals(USER_ID, secretKeys.getPublicKey().getUserIDs().next());
String messageWithWrongUserId = generateTestMessage(secretKeys);
verifyTestMessage(messageWithWrongUserId, secretKeys, expectSucessfulVerification);
}
private void verifyTestMessage(String messageWithWrongUserId, PGPSecretKeyRing secretKeys, boolean expectSuccessfulVerification) throws IOException, PGPException {
PGPPublicKeyRing certificate = PGPainless.extractCertificate(secretKeys);
DecryptionStream decryptionStream = PGPainless.decryptAndOrVerify().onInputStream(
new ByteArrayInputStream(messageWithWrongUserId.getBytes(StandardCharsets.UTF_8)))
.withOptions(new ConsumerOptions()
.addDecryptionKey(secretKeys)
.addVerificationCert(certificate));
ByteArrayOutputStream out = new ByteArrayOutputStream();
Streams.pipeAll(decryptionStream, out);
decryptionStream.close();
OpenPgpMetadata metadata = decryptionStream.getResult();
if (expectSuccessfulVerification) {
assertTrue(metadata.isVerified());
} else {
assertFalse(metadata.isVerified());
}
}
private String generateTestMessage(PGPSecretKeyRing secretKeys) throws PGPException, IOException {
PGPPublicKeyRing certificate = PGPainless.extractCertificate(secretKeys);
assertEquals(USER_ID, certificate.getPublicKey().getUserIDs().next());
@ -126,19 +169,6 @@ public class WrongSignerUserIdTest {
encStream.close();
armorOut.close();
try {
DecryptionStream decryptionStream = PGPainless.decryptAndOrVerify().onInputStream(
new ByteArrayInputStream(cipherText.toByteArray()))
.withOptions(new ConsumerOptions()
.addDecryptionKey(secretKeys)
.addVerificationCert(certificate));
ByteArrayOutputStream out = new ByteArrayOutputStream();
Streams.pipeAll(decryptionStream, out);
decryptionStream.close();
} catch (SignatureValidationException e) {
// expected
}
return cipherText.toString();
}
}