1
0
Fork 0
mirror of https://github.com/pgpainless/pgpainless.git synced 2024-12-25 12:27:58 +01:00

SIGNATURE VERIFICATION IN OPENPGP SUCKS BIG TIME

This commit is contained in:
Paul Schaub 2022-09-29 00:15:18 +02:00
parent e8b5787557
commit 2dbc998890
2 changed files with 128 additions and 92 deletions

View file

@ -48,6 +48,7 @@ import org.pgpainless.key.info.KeyRingInfo;
import org.pgpainless.key.protection.SecretKeyRingProtector; import org.pgpainless.key.protection.SecretKeyRingProtector;
import org.pgpainless.key.protection.UnlockSecretKey; import org.pgpainless.key.protection.UnlockSecretKey;
import org.pgpainless.signature.SignatureUtils; import org.pgpainless.signature.SignatureUtils;
import org.pgpainless.util.ArmorUtils;
import org.pgpainless.util.Passphrase; import org.pgpainless.util.Passphrase;
import org.pgpainless.util.Tuple; import org.pgpainless.util.Tuple;
import org.slf4j.Logger; import org.slf4j.Logger;
@ -138,19 +139,25 @@ public class OpenPgpMessageInputStream extends InputStream {
// Literal Data - the literal data content is the new input stream // Literal Data - the literal data content is the new input stream
case LIT: case LIT:
automaton.next(InputAlphabet.LiteralData); automaton.next(InputAlphabet.LiteralData);
signatures.commitNested();
processLiteralData(); processLiteralData();
break loop; break loop;
// Compressed Data - the content contains another OpenPGP message // Compressed Data - the content contains another OpenPGP message
case COMP: case COMP:
automaton.next(InputAlphabet.CompressedData); automaton.next(InputAlphabet.CompressedData);
signatures.commitNested();
processCompressedData(); processCompressedData();
break loop; break loop;
// One Pass Signature // One Pass Signature
case OPS: case OPS:
automaton.next(InputAlphabet.OnePassSignatures); automaton.next(InputAlphabet.OnePassSignatures);
signatures.addOnePassSignature(readOnePassSignature()); // signatures.addOnePassSignature(readOnePassSignature());
PGPOnePassSignatureList onePassSignatureList = readOnePassSignatures();
for (PGPOnePassSignature ops : onePassSignatureList) {
signatures.addOnePassSignature(ops);
}
// signatures.addOnePassSignatures(readOnePassSignatures()); // signatures.addOnePassSignatures(readOnePassSignatures());
break; break;
@ -158,12 +165,15 @@ public class OpenPgpMessageInputStream extends InputStream {
case SIG: case SIG:
boolean isSigForOPS = automaton.peekStack() == StackAlphabet.ops; boolean isSigForOPS = automaton.peekStack() == StackAlphabet.ops;
automaton.next(InputAlphabet.Signatures); automaton.next(InputAlphabet.Signatures);
PGPSignature signature = readSignature();
processSignature(signature, isSigForOPS); // PGPSignature signature = readSignature();
/* // processSignature(signature, isSigForOPS);
PGPSignatureList signatureList = readSignatures(); PGPSignatureList signatureList = readSignatures();
processSignatures(signatureList, isSigForOPS); for (PGPSignature signature : signatureList) {
*/ processSignature(signature, isSigForOPS);
}
break; break;
// Encrypted Data (ESKs and SED/SEIPD are parsed the same by BC) // Encrypted Data (ESKs and SED/SEIPD are parsed the same by BC)
@ -178,7 +188,7 @@ public class OpenPgpMessageInputStream extends InputStream {
throw new MissingDecryptionMethodException("No working decryption method found."); throw new MissingDecryptionMethodException("No working decryption method found.");
// Marker Packets need to be skipped and ignored // Marker Packets need to be skipped and ignored
case MARKER: case MARKER:
packetInputStream.readPacket(); // skip packetInputStream.readPacket(); // skip
break; break;
@ -193,8 +203,8 @@ public class OpenPgpMessageInputStream extends InputStream {
case UATTR: case UATTR:
throw new MalformedOpenPgpMessageException("Illegal Packet in Stream: " + nextPacket); throw new MalformedOpenPgpMessageException("Illegal Packet in Stream: " + nextPacket);
// MDC packet is usually processed by PGPEncryptedDataList, so it is very likely we encounter this // MDC packet is usually processed by PGPEncryptedDataList, so it is very likely we encounter this
// packet out of order // packet out of order
case MDC: case MDC:
throw new MalformedOpenPgpMessageException("Unexpected Packet in Stream: " + nextPacket); throw new MalformedOpenPgpMessageException("Unexpected Packet in Stream: " + nextPacket);
@ -210,20 +220,12 @@ public class OpenPgpMessageInputStream extends InputStream {
private void processSignature(PGPSignature signature, boolean isSigForOPS) { private void processSignature(PGPSignature signature, boolean isSigForOPS) {
if (isSigForOPS) { if (isSigForOPS) {
signatures.addOnePassCorrespondingSignature(signature); signatures.addCorrespondingOnePassSignature(signature);
} else { } else {
signatures.addPrependedSignature(signature); signatures.addPrependedSignature(signature);
} }
} }
private void processSignatures(PGPSignatureList signatureList, boolean isSigForOPS) throws IOException {
if (isSigForOPS) {
signatures.addOnePassCorrespondingSignatures(signatureList);
} else {
signatures.addPrependedSignatures(signatureList);
}
}
private void processCompressedData() throws IOException, PGPException { private void processCompressedData() throws IOException, PGPException {
PGPCompressedData compressedData = new PGPCompressedData(packetInputStream); PGPCompressedData compressedData = new PGPCompressedData(packetInputStream);
MessageMetadata.CompressedData compressionLayer = new MessageMetadata.CompressedData( MessageMetadata.CompressedData compressionLayer = new MessageMetadata.CompressedData(
@ -380,16 +382,17 @@ public class OpenPgpMessageInputStream extends InputStream {
private PGPOnePassSignature readOnePassSignature() private PGPOnePassSignature readOnePassSignature()
throws PGPException, IOException { throws PGPException, IOException {
return new PGPOnePassSignature(packetInputStream); //return new PGPOnePassSignature(packetInputStream);
return null;
} }
private PGPSignature readSignature() private PGPSignature readSignature()
throws PGPException, IOException { throws PGPException, IOException {
return new PGPSignature(packetInputStream); //return new PGPSignature(packetInputStream);
return null;
} }
private PGPOnePassSignatureListWrapper readOnePassSignatures() throws IOException { private PGPOnePassSignatureList readOnePassSignatures() throws IOException {
List<Boolean> encapsulating = new ArrayList<>();
ByteArrayOutputStream buf = new ByteArrayOutputStream(); ByteArrayOutputStream buf = new ByteArrayOutputStream();
BCPGOutputStream bcpgOut = new BCPGOutputStream(buf); BCPGOutputStream bcpgOut = new BCPGOutputStream(buf);
int tag; int tag;
@ -398,7 +401,6 @@ public class OpenPgpMessageInputStream extends InputStream {
if (tag == PacketTags.ONE_PASS_SIGNATURE) { if (tag == PacketTags.ONE_PASS_SIGNATURE) {
OnePassSignaturePacket sigPacket = (OnePassSignaturePacket) packet; OnePassSignaturePacket sigPacket = (OnePassSignaturePacket) packet;
byte[] bytes = sigPacket.getEncoded(); byte[] bytes = sigPacket.getEncoded();
encapsulating.add(bytes[bytes.length - 1] == 1);
bcpgOut.write(bytes); bcpgOut.write(bytes);
} }
} }
@ -406,7 +408,7 @@ public class OpenPgpMessageInputStream extends InputStream {
PGPObjectFactory objectFactory = ImplementationFactory.getInstance().getPGPObjectFactory(buf.toByteArray()); PGPObjectFactory objectFactory = ImplementationFactory.getInstance().getPGPObjectFactory(buf.toByteArray());
PGPOnePassSignatureList signatureList = (PGPOnePassSignatureList) objectFactory.nextObject(); PGPOnePassSignatureList signatureList = (PGPOnePassSignatureList) objectFactory.nextObject();
return new PGPOnePassSignatureListWrapper(signatureList, encapsulating); return signatureList;
} }
private PGPSignatureList readSignatures() throws IOException { private PGPSignatureList readSignatures() throws IOException {
@ -449,6 +451,7 @@ public class OpenPgpMessageInputStream extends InputStream {
nestedInputStream.close(); nestedInputStream.close();
collectMetadata(); collectMetadata();
nestedInputStream = null; nestedInputStream = null;
signatures.popNested();
try { try {
consumePackets(); consumePackets();
@ -474,6 +477,7 @@ public class OpenPgpMessageInputStream extends InputStream {
nestedInputStream.close(); nestedInputStream.close();
collectMetadata(); collectMetadata();
nestedInputStream = null; nestedInputStream = null;
signatures.popNested();
try { try {
consumePackets(); consumePackets();
@ -556,41 +560,29 @@ public class OpenPgpMessageInputStream extends InputStream {
} }
} }
/** // TODO: In 'OPS LIT("Foo") SIG', OPS is only updated with "Foo"
* Workaround for BC not exposing, whether an OPS is encapsulating or not. // In 'OPS[1] OPS LIT("Foo") SIG SIG', OPS[1] (nested) is updated with OPS LIT("Foo") SIG.
* TODO: Remove once our PR is merged // Therefore, we need to handle the innermost signature layer differently when updating with Literal data.
* // For this we might want to provide two update entries into the Signatures class, one for OpenPGP packets and one
* @see <a href="https://github.com/bcgit/bc-java/pull/1232">PR against BC</a> // for literal data. UUUUUGLY!!!!
*/
private static class PGPOnePassSignatureListWrapper {
private final PGPOnePassSignatureList list;
private final List<Boolean> encapsulating;
PGPOnePassSignatureListWrapper(PGPOnePassSignatureList signatures, List<Boolean> encapsulating) {
this.list = signatures;
this.encapsulating = encapsulating;
}
public int size() {
return list.size();
}
}
private static final class Signatures extends OutputStream { private static final class Signatures extends OutputStream {
final ConsumerOptions options; final ConsumerOptions options;
final List<PGPSignature> detachedSignatures; final List<PGPSignature> detachedSignatures;
final List<PGPSignature> prependedSignatures; final List<PGPSignature> prependedSignatures;
final Stack<List<PGPOnePassSignature>> onePassSignatures; final List<PGPOnePassSignature> onePassSignatures;
final Stack<List<PGPOnePassSignature>> opsUpdateStack;
final List<PGPSignature> correspondingSignatures; final List<PGPSignature> correspondingSignatures;
boolean lastOpsIsContaining = true; ByteArrayOutputStream out = new ByteArrayOutputStream();
List<PGPOnePassSignature> opsCurrentNesting = new ArrayList<>();
private Signatures(ConsumerOptions options) { private Signatures(ConsumerOptions options) {
this.options = options; this.options = options;
this.detachedSignatures = new ArrayList<>(); this.detachedSignatures = new ArrayList<>();
this.prependedSignatures = new ArrayList<>(); this.prependedSignatures = new ArrayList<>();
this.onePassSignatures = new Stack<>(); this.onePassSignatures = new ArrayList<>();
onePassSignatures.push(new ArrayList<>()); this.opsUpdateStack = new Stack<>();
this.correspondingSignatures = new ArrayList<>(); this.correspondingSignatures = new ArrayList<>();
} }
@ -607,12 +599,6 @@ public class OpenPgpMessageInputStream extends InputStream {
this.detachedSignatures.add(signature); this.detachedSignatures.add(signature);
} }
void addPrependedSignatures(PGPSignatureList signatures) {
for (PGPSignature signature : signatures) {
addPrependedSignature(signature);
}
}
void addPrependedSignature(PGPSignature signature) { void addPrependedSignature(PGPSignature signature) {
long keyId = SignatureUtils.determineIssuerKeyId(signature); long keyId = SignatureUtils.determineIssuerKeyId(signature);
PGPPublicKeyRing certificate = findCertificate(keyId); PGPPublicKeyRing certificate = findCertificate(keyId);
@ -621,27 +607,61 @@ public class OpenPgpMessageInputStream extends InputStream {
} }
void addOnePassSignature(PGPOnePassSignature signature) { void addOnePassSignature(PGPOnePassSignature signature) {
List<PGPOnePassSignature> list;
if (lastOpsIsContaining) {
list = new ArrayList<>();
onePassSignatures.add(list);
} else {
list = onePassSignatures.get(onePassSignatures.size() - 1);
}
PGPPublicKeyRing certificate = findCertificate(signature.getKeyID()); PGPPublicKeyRing certificate = findCertificate(signature.getKeyID());
initialize(signature, certificate); initialize(signature, certificate);
list.add(signature); onePassSignatures.add(signature);
// lastOpsIsContaining = signature.isContaining(); opsCurrentNesting.add(signature);
if (isContaining(signature)) {
commitNested();
}
} }
void addOnePassCorrespondingSignatures(PGPSignatureList signatures) { boolean isContaining(PGPOnePassSignature ops) {
for (PGPSignature signature : signatures) { try {
correspondingSignatures.add(signature); byte[] bytes = ops.getEncoded();
return bytes[bytes.length - 1] == 1;
} catch (IOException e) {
return false;
} }
} }
void addCorrespondingOnePassSignature(PGPSignature signature) {
for (PGPOnePassSignature onePassSignature : onePassSignatures) {
if (onePassSignature.getKeyID() != signature.getKeyID()) {
continue;
}
boolean verified = false;
try {
verified = onePassSignature.verify(signature);
} catch (PGPException e) {
log("Cannot verify OPS signature.", e);
}
log("One-Pass-Signature by " + Long.toHexString(onePassSignature.getKeyID()) + " is " + (verified ? "verified" : "unverified"));
try {
log(ArmorUtils.toAsciiArmoredString(out.toByteArray()));
} catch (IOException e) {
throw new RuntimeException(e);
}
}
}
void commitNested() {
if (opsCurrentNesting.isEmpty()) {
return;
}
log("Committing " + opsCurrentNesting.size() + " OPS sigs for updating");
opsUpdateStack.push(opsCurrentNesting);
opsCurrentNesting = new ArrayList<>();
}
void popNested() {
log("Popping nested");
opsUpdateStack.pop();
}
private void initialize(PGPSignature signature, PGPPublicKeyRing certificate, long keyId) { private void initialize(PGPSignature signature, PGPPublicKeyRing certificate, long keyId) {
if (certificate == null) { if (certificate == null) {
// SHIT // SHIT
@ -680,14 +700,21 @@ public class OpenPgpMessageInputStream extends InputStream {
} }
public void update(byte b) { public void update(byte b) {
if (!opsUpdateStack.isEmpty()) {
log("Update");
out.write(b);
}
for (PGPSignature prepended : prependedSignatures) { for (PGPSignature prepended : prependedSignatures) {
prepended.update(b); prepended.update(b);
} }
for (List<PGPOnePassSignature> opss : onePassSignatures) {
for (List<PGPOnePassSignature> opss : opsUpdateStack) {
for (PGPOnePassSignature ops : opss) { for (PGPOnePassSignature ops : opss) {
ops.update(b); ops.update(b);
} }
} }
for (PGPSignature detached : detachedSignatures) { for (PGPSignature detached : detachedSignatures) {
detached.update(b); detached.update(b);
} }
@ -699,9 +726,9 @@ public class OpenPgpMessageInputStream extends InputStream {
try { try {
verified = detached.verify(); verified = detached.verify();
} catch (PGPException e) { } catch (PGPException e) {
LOGGER.debug("Cannot verify detached signature.", e); log("Cannot verify detached signature.", e);
} }
LOGGER.debug("Detached Signature by " + Long.toHexString(detached.getKeyID()) + " is " + (verified ? "verified" : "unverified")); log("Detached Signature by " + Long.toHexString(detached.getKeyID()) + " is " + (verified ? "verified" : "unverified"));
} }
for (PGPSignature prepended : prependedSignatures) { for (PGPSignature prepended : prependedSignatures) {
@ -709,22 +736,9 @@ public class OpenPgpMessageInputStream extends InputStream {
try { try {
verified = prepended.verify(); verified = prepended.verify();
} catch (PGPException e) { } catch (PGPException e) {
LOGGER.debug("Cannot verify prepended signature.", e); log("Cannot verify prepended signature.", e);
} }
LOGGER.debug("Prepended Signature by " + Long.toHexString(prepended.getKeyID()) + " is " + (verified ? "verified" : "unverified")); log("Prepended Signature by " + Long.toHexString(prepended.getKeyID()) + " is " + (verified ? "verified" : "unverified"));
}
for (int i = 0; i < onePassSignatures.size(); i++) {
PGPOnePassSignature ops = onePassSignatures.get(i);
PGPSignature signature = correspondingSignatures.get(correspondingSignatures.size() - i - 1);
boolean verified = false;
try {
verified = ops.verify(signature);
} catch (PGPException e) {
LOGGER.debug("Cannot verify OPS signature.", e);
}
LOGGER.debug("One-Pass-Signature by " + Long.toHexString(ops.getKeyID()) + " is " + (verified ? "verified" : "unverified"));
} }
} }
@ -733,4 +747,18 @@ public class OpenPgpMessageInputStream extends InputStream {
update((byte) b); update((byte) b);
} }
} }
static void log(String message) {
LOGGER.debug(message);
// CHECKSTYLE:OFF
System.out.println(message);
// CHECKSTYLE:ON
}
static void log(String message, Throwable e) {
log(message);
// CHECKSTYLE:OFF
e.printStackTrace();
// CHECKSTYLE:ON
}
} }

View file

@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.ByteArrayInputStream; import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream; import java.io.ByteArrayOutputStream;
@ -377,7 +378,8 @@ public class OpenPgpMessageInputStreamTest {
@ParameterizedTest(name = "Process SIG COMP(LIT) using {0}") @ParameterizedTest(name = "Process SIG COMP(LIT) using {0}")
@MethodSource("provideMessageProcessors") @MethodSource("provideMessageProcessors")
public void testProcessSIG_COMP_LIT(Processor processor) throws PGPException, IOException { public void testProcessSIG_COMP_LIT(Processor processor)
throws PGPException, IOException {
PGPPublicKeyRing cert = PGPainless.extractCertificate( PGPPublicKeyRing cert = PGPainless.extractCertificate(
PGPainless.readKeyRing().secretKeyRing(KEY)); PGPainless.readKeyRing().secretKeyRing(KEY));
@ -392,7 +394,8 @@ public class OpenPgpMessageInputStreamTest {
@ParameterizedTest(name = "Process SENC(LIT) using {0}") @ParameterizedTest(name = "Process SENC(LIT) using {0}")
@MethodSource("provideMessageProcessors") @MethodSource("provideMessageProcessors")
public void testProcessSENC_LIT(Processor processor) throws PGPException, IOException { public void testProcessSENC_LIT(Processor processor)
throws PGPException, IOException {
Tuple<String, MessageMetadata> result = processor.process(SENC_LIT, ConsumerOptions.get() Tuple<String, MessageMetadata> result = processor.process(SENC_LIT, ConsumerOptions.get()
.addDecryptionPassphrase(Passphrase.fromPassword(PASSPHRASE))); .addDecryptionPassphrase(Passphrase.fromPassword(PASSPHRASE)));
String plain = result.getA(); String plain = result.getA();
@ -404,7 +407,8 @@ public class OpenPgpMessageInputStreamTest {
@ParameterizedTest(name = "Process PENC(COMP(LIT)) using {0}") @ParameterizedTest(name = "Process PENC(COMP(LIT)) using {0}")
@MethodSource("provideMessageProcessors") @MethodSource("provideMessageProcessors")
public void testProcessPENC_COMP_LIT(Processor processor) throws IOException, PGPException { public void testProcessPENC_COMP_LIT(Processor processor)
throws IOException, PGPException {
PGPSecretKeyRing secretKeys = PGPainless.readKeyRing().secretKeyRing(KEY); PGPSecretKeyRing secretKeys = PGPainless.readKeyRing().secretKeyRing(KEY);
Tuple<String, MessageMetadata> result = processor.process(PENC_COMP_LIT, ConsumerOptions.get() Tuple<String, MessageMetadata> result = processor.process(PENC_COMP_LIT, ConsumerOptions.get()
.addDecryptionKey(secretKeys)); .addDecryptionKey(secretKeys));
@ -417,7 +421,8 @@ public class OpenPgpMessageInputStreamTest {
@ParameterizedTest(name = "Process OPS LIT SIG using {0}") @ParameterizedTest(name = "Process OPS LIT SIG using {0}")
@MethodSource("provideMessageProcessors") @MethodSource("provideMessageProcessors")
public void testProcessOPS_LIT_SIG(Processor processor) throws IOException, PGPException { public void testProcessOPS_LIT_SIG(Processor processor)
throws IOException, PGPException {
PGPPublicKeyRing cert = PGPainless.extractCertificate(PGPainless.readKeyRing().secretKeyRing(KEY)); PGPPublicKeyRing cert = PGPainless.extractCertificate(PGPainless.readKeyRing().secretKeyRing(KEY));
Tuple<String, MessageMetadata> result = processor.process(OPS_LIT_SIG, ConsumerOptions.get() Tuple<String, MessageMetadata> result = processor.process(OPS_LIT_SIG, ConsumerOptions.get()
.addVerificationCert(cert)); .addVerificationCert(cert));
@ -428,7 +433,8 @@ public class OpenPgpMessageInputStreamTest {
assertNull(metadata.getCompressionAlgorithm()); assertNull(metadata.getCompressionAlgorithm());
} }
private static Tuple<String, MessageMetadata> processReadBuffered(String armoredMessage, ConsumerOptions options) throws PGPException, IOException { private static Tuple<String, MessageMetadata> processReadBuffered(String armoredMessage, ConsumerOptions options)
throws PGPException, IOException {
OpenPgpMessageInputStream in = get(armoredMessage, options); OpenPgpMessageInputStream in = get(armoredMessage, options);
ByteArrayOutputStream out = new ByteArrayOutputStream(); ByteArrayOutputStream out = new ByteArrayOutputStream();
Streams.pipeAll(in, out); Streams.pipeAll(in, out);
@ -437,7 +443,8 @@ public class OpenPgpMessageInputStreamTest {
return new Tuple<>(out.toString(), metadata); return new Tuple<>(out.toString(), metadata);
} }
private static Tuple<String, MessageMetadata> processReadSequential(String armoredMessage, ConsumerOptions options) throws PGPException, IOException { private static Tuple<String, MessageMetadata> processReadSequential(String armoredMessage, ConsumerOptions options)
throws PGPException, IOException {
OpenPgpMessageInputStream in = get(armoredMessage, options); OpenPgpMessageInputStream in = get(armoredMessage, options);
ByteArrayOutputStream out = new ByteArrayOutputStream(); ByteArrayOutputStream out = new ByteArrayOutputStream();
@ -451,7 +458,8 @@ public class OpenPgpMessageInputStreamTest {
return new Tuple<>(out.toString(), metadata); return new Tuple<>(out.toString(), metadata);
} }
private static OpenPgpMessageInputStream get(String armored, ConsumerOptions options) throws IOException, PGPException { private static OpenPgpMessageInputStream get(String armored, ConsumerOptions options)
throws IOException, PGPException {
ByteArrayInputStream bytesIn = new ByteArrayInputStream(armored.getBytes(StandardCharsets.UTF_8)); ByteArrayInputStream bytesIn = new ByteArrayInputStream(armored.getBytes(StandardCharsets.UTF_8));
ArmoredInputStream armorIn = ArmoredInputStreamFactory.get(bytesIn); ArmoredInputStream armorIn = ArmoredInputStreamFactory.get(bytesIn);
OpenPgpMessageInputStream pgpIn = new OpenPgpMessageInputStream(armorIn, options); OpenPgpMessageInputStream pgpIn = new OpenPgpMessageInputStream(armorIn, options);