[PATCH GnuPG] Disallow compressed signatures and certificates
Demi Marie Obenour
demiobenour at gmail.com
Thu Feb 2 07:19:50 CET 2023
Compressed packets have significant attack surface, due to the potential
for both denial of service (zip bombs and the like) and for code
execution via memory corruption vulnerabilities in the decompressor.
Furthermore, I am not aware of any implementation that uses them in keys
or detached signatures. Therefore, disallow their use in such contexts
entirely. This includes signatures that are part of a cleartext-signed
message.
When parsing detached signatures, forbid any packet that is not a
signature or marker packet. When parsing keys, return an error when
encountering a compressed packet, instead of decompressing the packet.
When parsing a cleartext-signed message, the signature (and any data
that follows it) is treated as a detached signature.
Furthermore, certificates, keys, and signatures are not allowed to
contain partial-length or indeterminate-length packets. Reject those in
parse_packet, rather than activating the partial-length filter code.
Tests for changes to signature processing are included. Tests for
changes to key and certificate processing have not yet been implemented.
GnuPG-bug-id: T5993
Signed-off-by: Demi Marie Obenour <demiobenour at gmail.com>
---
g10/import.c | 18 +-------
g10/mainproc.c | 35 ++++++++++++++-
g10/packet.h | 2 +
g10/parse-packet.c | 45 ++++++++++++++++++-
tests/openpgp/compressed-sig-inline.asc | 15 +++++++
tests/openpgp/compressed-sig.asc | 11 +++++
tests/openpgp/indeterminate-length-inline.asc | 14 ++++++
tests/openpgp/indeterminate-length.asc | 10 +++++
tests/openpgp/partial-len-inline.asc | 14 ++++++
tests/openpgp/partial-len.asc | 10 +++++
tests/openpgp/verify.scm | 32 +++++++++++++
11 files changed, 187 insertions(+), 19 deletions(-)
create mode 100644 tests/openpgp/compressed-sig-inline.asc
create mode 100644 tests/openpgp/compressed-sig.asc
create mode 100644 tests/openpgp/indeterminate-length-inline.asc
create mode 100644 tests/openpgp/indeterminate-length.asc
create mode 100644 tests/openpgp/partial-len-inline.asc
create mode 100644 tests/openpgp/partial-len.asc
diff --git a/g10/import.c b/g10/import.c
index 9fab46ca65a8bd81ac9f1a240a60eeb5f4b2b987..20fdfedf12c46b7f6bf0c047ac277337ed606f15 100644
--- a/g10/import.c
+++ b/g10/import.c
@@ -1043,22 +1043,8 @@ read_block( IOBUF a, unsigned int options,
switch (pkt->pkttype)
{
case PKT_COMPRESSED:
- if (check_compress_algo (pkt->pkt.compressed->algorithm))
- {
- rc = GPG_ERR_COMPR_ALGO;
- goto ready;
- }
- else
- {
- compress_filter_context_t *cfx = xmalloc_clear( sizeof *cfx );
- pkt->pkt.compressed->buf = NULL;
- if (push_compress_filter2 (a, cfx,
- pkt->pkt.compressed->algorithm, 1))
- xfree (cfx); /* e.g. in case of compression_algo NONE. */
- }
- free_packet (pkt, &parsectx);
- init_packet(pkt);
- break;
+ rc = GPG_ERR_UNEXPECTED;
+ goto ready;
case PKT_RING_TRUST:
/* Skip those packets unless we are in restore mode. */
diff --git a/g10/mainproc.c b/g10/mainproc.c
index 4710386eafee7cc3894698cde4bb2537a322c658..992e3d088985efee717280dd4666bc2970af7647 100644
--- a/g10/mainproc.c
+++ b/g10/mainproc.c
@@ -105,6 +105,7 @@ struct mainproc_context
has been seen. */
unsigned int data:1; /* Any data packet seen */
unsigned int uncompress_failed:1;
+ unsigned int in_cleartext:1; /* In cleartext of cleartext signature */
} any;
};
@@ -170,6 +171,7 @@ add_onepass_sig (CTX c, PACKET *pkt)
{
kbnode_t node;
+ log_assert(!(c->sigs_only && c->signed_data.used));
if (c->list) /* Add another packet. */
add_kbnode (c->list, new_kbnode (pkt));
else /* Insert the first one. */
@@ -187,6 +189,7 @@ add_gpg_control (CTX c, PACKET *pkt)
/* New clear text signature.
* Process the last one and reset everything */
release_list(c);
+ c->any.in_cleartext = 1;
}
if (c->list) /* Add another packet. */
@@ -1118,8 +1121,16 @@ proc_compressed (CTX c, PACKET *pkt)
int rc;
/*printf("zip: compressed data packet\n");*/
- if (c->sigs_only)
- rc = handle_compressed (c->ctrl, c, zd, proc_compressed_cb, c);
+ if( literals_seen )
+ {
+ log_error ("Compressed packet follows literal data packet\n");
+ rc = gpg_error (GPG_ERR_BAD_DATA);
+ }
+ else if( c->sigs_only )
+ {
+ log_assert(!c->signed_data.used);
+ rc = handle_compressed (c->ctrl, c, zd, proc_compressed_cb, c);
+ }
else if( c->encrypt_only )
rc = handle_compressed (c->ctrl, c, zd, proc_encrypt_cb, c);
else
@@ -1638,6 +1649,7 @@ do_proc_packets (CTX c, iobuf_t a)
c->iobuf = a;
init_packet(pkt);
init_parse_packet (&parsectx, a);
+ parsectx.sigs_only = c->sigs_only && c->signed_data.used;
while ((rc=parse_packet (&parsectx, pkt)) != -1)
{
any_data = 1;
@@ -1649,11 +1661,27 @@ do_proc_packets (CTX c, iobuf_t a)
if (gpg_err_code (rc) == GPG_ERR_INV_PACKET
&& opt.list_packets == 0)
break;
+
+ if (gpg_err_code (rc) == GPG_ERR_UNEXPECTED)
+ {
+ write_status_text( STATUS_UNEXPECTED, "0" );
+ goto leave;
+ }
continue;
}
newpkt = -1;
+ if (c->any.in_cleartext)
+ {
+ /* Just finished a clear-text signature's plaintext */
+ if (pkt->pkttype != PKT_PLAINTEXT)
+ log_bug("Armor parser produced packet type %d where %d expected",
+ pkt->pkttype, PKT_PLAINTEXT);
+ c->any.in_cleartext = 0;
+ parsectx.sigs_only = 1;
+ }
if (opt.list_packets)
{
+ log_assert(!c->sigs_only);
switch (pkt->pkttype)
{
case PKT_PUBKEY_ENC: proc_pubkey_enc (c, pkt); break;
@@ -1667,6 +1695,9 @@ do_proc_packets (CTX c, iobuf_t a)
}
else if (c->sigs_only)
{
+ log_assert(pkt->pkttype == PKT_SIGNATURE ||
+ pkt->pkttype == PKT_MARKER ||
+ !c->signed_data.used);
switch (pkt->pkttype)
{
case PKT_PUBLIC_KEY:
diff --git a/g10/packet.h b/g10/packet.h
index eeea9b450facad03188d4b87485b8fb25398f48d..6de0685b61e0098cc92f8ae0e98d897614a736fe 100644
--- a/g10/packet.h
+++ b/g10/packet.h
@@ -660,6 +660,7 @@ struct parse_packet_ctx_s
int free_last_pkt; /* Indicates that LAST_PKT must be freed. */
int skip_meta; /* Skip ring trust packets. */
unsigned int n_parsed_packets; /* Number of parsed packets. */
+ int sigs_only; /* Only accept detached signature packets */
};
typedef struct parse_packet_ctx_s *parse_packet_ctx_t;
@@ -670,6 +671,7 @@ typedef struct parse_packet_ctx_s *parse_packet_ctx_t;
(a)->free_last_pkt = 0; \
(a)->skip_meta = 0; \
(a)->n_parsed_packets = 0; \
+ (a)->sigs_only = 0; \
} while (0)
#define deinit_parse_packet(a) do { \
diff --git a/g10/parse-packet.c b/g10/parse-packet.c
index a033732ecf97dfd14ad55d940de6ff5936829caa..af641694797a19881dde40435d9712d8ecc603cd 100644
--- a/g10/parse-packet.c
+++ b/g10/parse-packet.c
@@ -738,6 +738,20 @@ parse (parse_packet_ctx_t ctx, PACKET *pkt, int onlykeypkts, off_t * retpos,
case PKT_ENCRYPTED_MDC:
case PKT_ENCRYPTED_AEAD:
case PKT_COMPRESSED:
+ if (ctx->sigs_only)
+ {
+ log_error (_("partial length packet of type %d in detached "
+ "or cleartext signature\n"), pkttype);
+ rc = gpg_error (GPG_ERR_INV_PACKET);
+ goto leave;
+ }
+ if (onlykeypkts)
+ {
+ log_error (_("partial length packet of type %d in keyring\n"),
+ pkttype);
+ rc = gpg_error (GPG_ERR_INV_PACKET);
+ goto leave;
+ }
iobuf_set_partial_body_length_mode (inp, c & 0xff);
pktlen = 0; /* To indicate partial length. */
partial = 1;
@@ -767,6 +781,20 @@ parse (parse_packet_ctx_t ctx, PACKET *pkt, int onlykeypkts, off_t * retpos,
/* This isn't really partial, but we can treat it the same
in a "read until the end" sort of way. */
partial = 1;
+ if (ctx->sigs_only)
+ {
+ log_error (_("indeterminate length packet of type %d in detached "
+ "or cleartext signature\n"), pkttype);
+ rc = gpg_error (GPG_ERR_INV_PACKET);
+ goto leave;
+ }
+ if (onlykeypkts)
+ {
+ log_error (_("indeterminate length packet of type %d in"
+ " keyring\n"), pkttype);
+ rc = gpg_error (GPG_ERR_INV_PACKET);
+ goto leave;
+ }
if (pkttype != PKT_ENCRYPTED && pkttype != PKT_PLAINTEXT
&& pkttype != PKT_COMPRESSED)
{
@@ -828,7 +856,22 @@ parse (parse_packet_ctx_t ctx, PACKET *pkt, int onlykeypkts, off_t * retpos,
goto leave;
}
- if (with_uid && pkttype == PKT_USER_ID)
+ if (ctx->sigs_only)
+ switch (pkttype)
+ {
+ case PKT_SIGNATURE:
+ case PKT_MARKER:
+ break;
+ default:
+ log_error(_("Packet type %d not allowed in detached "
+ "or cleartext signature\n"),
+ pkttype);
+ iobuf_skip_rest (inp, pktlen, partial);
+ *skip = 1;
+ rc = gpg_error (GPG_ERR_BAD_DATA);
+ goto leave;
+ }
+ else if (with_uid && pkttype == PKT_USER_ID)
/* If ONLYKEYPKTS is set to 2, then we never skip user id packets,
even if DO_SKIP is set. */
;
diff --git a/tests/openpgp/compressed-sig-inline.asc b/tests/openpgp/compressed-sig-inline.asc
new file mode 100644
index 0000000000000000000000000000000000000000..3420ee1b8c50da4b7da9140d1089f06cacf18610
--- /dev/null
+++ b/tests/openpgp/compressed-sig-inline.asc
@@ -0,0 +1,15 @@
>
diff --git a/tests/openpgp/compressed-sig.asc b/tests/openpgp/compressed-sig.asc
new file mode 100644
index 0000000000000000000000000000000000000000..15d6e73b696c38aff7591366046b01c28a7e9f74
--- /dev/null
+++ b/tests/openpgp/compressed-sig.asc
@@ -0,0 +1,11 @@
+-----BEGIN PGP SIGNATURE-----
+Comment: This is a signature (of the empty file) wrapped in a compressed
+Comment: packet. Such signatures must be rejected to prevent DoS
+Comment: attacks.
+
+ogAAAJYB65jIwiDGwWAppsjC0P0yfP1vewn2ee0/jNjKJorezGBhZUrexSUuLZOV
+n5Gnl52ampeY51CSWlyil55XWpCul1+UzsDFKQBTnfiV4X/G+ysmcvsLn7lX3Dof
+zPNjjWhRXtyV3IP/q81YuR+HnjjOyPD+7f3Y7Dmfr/+beFCV/WvAc9YFDetCgv5F
+fz/kbHxF+QILAA==
+=3bBP
+-----END PGP SIGNATURE-----
diff --git a/tests/openpgp/indeterminate-length-inline.asc b/tests/openpgp/indeterminate-length-inline.asc
new file mode 100644
index 0000000000000000000000000000000000000000..8674063028c732a9b1e5b87d87b2cadd25622839
--- /dev/null
+++ b/tests/openpgp/indeterminate-length-inline.asc
@@ -0,0 +1,14 @@
+-----BEGIN PGP SIGNED MESSAGE-----
+Hash: SHA256
+
+
+-----BEGIN PGP SIGNATURE-----
+Comment: This signature is encoded as an indeterminate length packet,
+Comment: which is forbidden.
+
+iwQAFggAORYhBACL6Vev+z8YB56H+DIGdpEV2WgEBQJjyfwrGxxqb2huLmtlZW5h
+bkB0ZXN0LmdudXBnLm9yZwAKCRAyBnaRFdloBMWaAQC9wiaOsM5QxSkoAFLypqdN
+KXS0SWXpmmvK5h5aIE6igQD+PLEBHomZltQkNR/V/TYOBmT4G153RfHV5tHooLy0
+9Q0=
+=Xhaf
+-----END PGP SIGNATURE-----
diff --git a/tests/openpgp/indeterminate-length.asc b/tests/openpgp/indeterminate-length.asc
new file mode 100644
index 0000000000000000000000000000000000000000..e79dcc42afd267ff1be16a25135d926636e6044b
--- /dev/null
+++ b/tests/openpgp/indeterminate-length.asc
@@ -0,0 +1,10 @@
+-----BEGIN PGP SIGNATURE-----
+Comment: This signature is encoded as an indeterminate length packet,
+Comment: which is forbidden.
+
+iwQAFggAORYhBACL6Vev+z8YB56H+DIGdpEV2WgEBQJjyfwrGxxqb2huLmtlZW5h
+bkB0ZXN0LmdudXBnLm9yZwAKCRAyBnaRFdloBMWaAQC9wiaOsM5QxSkoAFLypqdN
+KXS0SWXpmmvK5h5aIE6igQD+PLEBHomZltQkNR/V/TYOBmT4G153RfHV5tHooLy0
+9Q0=
+=Xhaf
+-----END PGP SIGNATURE-----
diff --git a/tests/openpgp/partial-len-inline.asc b/tests/openpgp/partial-len-inline.asc
new file mode 100644
index 0000000000000000000000000000000000000000..d3a66ec389abca8d9c27c842f152d07d651a1c2d
--- /dev/null
+++ b/tests/openpgp/partial-len-inline.asc
@@ -0,0 +1,14 @@
+-----BEGIN PGP SIGNED MESSAGE-----
+Hash: SHA256
+
+
+-----BEGIN PGP SIGNATURE-----
+Comment: This signature uses partial-length encoding, which is
+Comment: not allowed.
+
+yOcB65jIwiDGwWAppsjC0P0yfP1vewn2ee0/jNjKJorezGBhZUrexSUuLZOVn5Gn
+l52ampeY51CSWlyil55XWpCul1+UzsDFKQBTnfiV4X/G+ysmcvsLn7lX3DofzPNj
+jWhRXtyV3IP/q81YuR+HnjjOyPD+7f3Y7Dmfr/+beFCV/RVrwHPWBQ3rQoL+RX8/
+5Gx8RfkCCwA=
+=Ph+8
+-----END PGP SIGNATURE-----
diff --git a/tests/openpgp/partial-len.asc b/tests/openpgp/partial-len.asc
new file mode 100644
index 0000000000000000000000000000000000000000..24c966d1bf5a30a3731c8be02cfc00bd01dfc07d
--- /dev/null
+++ b/tests/openpgp/partial-len.asc
@@ -0,0 +1,10 @@
+-----BEGIN PGP SIGNATURE-----
+Comment: This signature uses partial-length encoding, which is
+Comment: not allowed.
+
+yOcB65jIwiDGwWAppsjC0P0yfP1vewn2ee0/jNjKJorezGBhZUrexSUuLZOVn5Gn
+l52ampeY51CSWlyil55XWpCul1+UzsDFKQBTnfiV4X/G+ysmcvsLn7lX3DofzPNj
+jWhRXtyV3IP/q81YuR+HnjjOyPD+7f3Y7Dmfr/+beFCV/RVrwHPWBQ3rQoL+RX8/
+5Gx8RfkCCwA=
+=Ph+8
+-----END PGP SIGNATURE-----
diff --git a/tests/openpgp/verify.scm b/tests/openpgp/verify.scm
index afa6b6a213cc583aca38417c426695786bd16f53..57d02a81e9cc2b781fb86c996d36e503545cfa6e 100755
--- a/tests/openpgp/verify.scm
+++ b/tests/openpgp/verify.scm
@@ -70,6 +70,38 @@
(fail "verification succeeded but should not")))
'(bad_ls_asc bad_fols_asc bad_olsf_asc bad_ools_asc))
+(info "Checking that compressed signatures are rejected")
+(call-check `(,(tool 'gpg) --quiet --yes --import
+ ,(in-srcdir "tests" "openpgp" "samplekeys" "silent-running.asc")))
+
+(let ((check-stderr
+ (lambda (msg file expected detached)
+ (info "Checking that" msg "are rejected")
+ (let* ((what `(,(tool 'gpg) --verify
+ ,(in-srcdir "tests" "openpgp" file)
+ ,@(if detached (list "/dev/null") '())))
+ (result (call-with-io what "")))
+ (if (and (= 2 (:retcode result))
+ (string-contains? (:stderr result) expected))
+ (:stderr result)
+ (throw (string-append (stringify what) " returned unexpected status "
+ (number->string (:retcode result)))
+ (:stderr result)))))))
+ (check-stderr "indeterminate length packets in detached signatures" "indeterminate-length.asc"
+ "gpg: indeterminate length packet of type 2 in detached or cleartext signature\n" #t)
+ (check-stderr "indeterminate length packets in cleartext signatures" "indeterminate-length-inline.asc"
+ "gpg: indeterminate length packet of type 2 in detached or cleartext signature\n" #f)
+ (check-stderr "compressed detached signatures" "compressed-sig.asc"
+ "gpg: Packet type 8 not allowed in detached or cleartext signature\n" #t)
+ (check-stderr "compressed cleartext signatures" "compressed-sig-inline.asc"
+ "gpg: Packet type 8 not allowed in detached or cleartext signature\n" #f)
+ (check-stderr "partial-length packets in detached signatures" "partial-len.asc"
+ "gpg: partial length packet of type 8 in detached or cleartext signature\n"
+ #t)
+ (check-stderr "partial-length packets in cleartext signatures" "partial-len-inline.asc"
+ "gpg: partial length packet of type 8 in detached or cleartext signature\n"
+ #f))
+
;;; Need to import the ed25519 sample key used for
;;; the next two tests.
--
2.39.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xB288B55FFF9C22C1.asc
Type: application/pgp-keys
Size: 4885 bytes
Desc: OpenPGP public key
URL: <https://lists.gnupg.org/pipermail/gnupg-devel/attachments/20230202/0f53adf9/attachment.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.gnupg.org/pipermail/gnupg-devel/attachments/20230202/0f53adf9/attachment.sig>
More information about the Gnupg-devel
mailing list