On 2024-05-27 Daniel Kahn Gillmor <d...@fifthhorseman.net> wrote:
[...]
> Would anyone be willing to try to backport the patches from upstream's
> fixes for T6481 to the 2.2.x series?
Hello Daniel,
the issue report refers to two patches, one of these is already part of
2.2.43. The other one[1] seemed pretty straightforward to backport
(functions moved to other files and cipher_filter_ocb instead of
cipher_filter_aead). I would appreciate a second pair of eyes.
cu Andreas
[1] https://dev.gnupg.org/rG2f872fa68c6576724b9dabee9fb0844266f55d0d
--
`What a good friend you are to him, Dr. Maturin. His other friends are
so grateful to you.'
`I sew his ears on from time to time, sure'
>From 2f872fa68c6576724b9dabee9fb0844266f55d0d Mon Sep 17 00:00:00 2001
From: NIIBE Yutaka <gni...@fsij.org>
Date: Wed, 24 May 2023 10:36:04 +0900
Subject: [PATCH] gpg: Report BEGIN_* status before examining the input.
* common/miscellaneous.c (is_openpgp_compressed_packet)
(is_file_compressed): Moved to ...
* common/iobuf.c: ... in this file.
(is_file_compressed): Change the argument to INP, the iobuf.
* common/util.h (is_file_compressed): Remove.
* common/iobuf.h (is_file_compressed): Add.
* g10/cipher-aead.c (write_header): Don't call write_status_printf
here.
(cipher_filter_aead): Call write_status_printf when called with
IOBUFCTRL_INIT.
* g10/cipher-cfb.c (write_header): Don't call write_status_printf
here.
(cipher_filter_cfb): Call write_status_printf when called with
IOBUFCTRL_INIT.
* g10/encrypt.c (encrypt_simple): Use new is_file_compressed function,
after call of iobuf_push_filter.
(encrypt_crypt): Likewise.
* g10/sign.c (sign_file): Likewise.
--
GnuPG-bug-id: 6481
--- a/common/iobuf.c
+++ b/common/iobuf.c
@@ -2750,5 +2750,125 @@ iobuf_skip_rest (iobuf_t a, unsigned lon
remaining -= count;
}
}
}
}
+
+
+/* Check whether (BUF,LEN) is valid header for an OpenPGP compressed
+ * packet. LEN should be at least 6. */
+static int
+is_openpgp_compressed_packet (const unsigned char *buf, size_t len)
+{
+ int c, ctb, pkttype;
+ int lenbytes;
+
+ ctb = *buf++; len--;
+ if (!(ctb & 0x80))
+ return 0; /* Invalid packet. */
+
+ if ((ctb & 0x40)) /* New style (OpenPGP) CTB. */
+ {
+ pkttype = (ctb & 0x3f);
+ if (!len)
+ return 0; /* Expected first length octet missing. */
+ c = *buf++; len--;
+ if (c < 192)
+ ;
+ else if (c < 224)
+ {
+ if (!len)
+ return 0; /* Expected second length octet missing. */
+ }
+ else if (c == 255)
+ {
+ if (len < 4)
+ return 0; /* Expected length octets missing */
+ }
+ }
+ else /* Old style CTB. */
+ {
+ pkttype = (ctb>>2)&0xf;
+ lenbytes = ((ctb&3)==3)? 0 : (1<<(ctb & 3));
+ if (len < lenbytes)
+ return 0; /* Not enough length bytes. */
+ }
+
+ return (pkttype == 8);
+}
+
+
+/*
+ * Check if the file is compressed, by peeking the iobuf. You need to
+ * pass the iobuf with INP. Returns true if the buffer seems to be
+ * compressed.
+ */
+int
+is_file_compressed (iobuf_t inp)
+{
+ int i;
+ char buf[32];
+ int buflen;
+
+ struct magic_compress_s
+ {
+ byte len;
+ byte extchk;
+ byte magic[5];
+ } magic[] =
+ {
+ { 3, 0, { 0x42, 0x5a, 0x68, 0x00 } }, /* bzip2 */
+ { 3, 0, { 0x1f, 0x8b, 0x08, 0x00 } }, /* gzip */
+ { 4, 0, { 0x50, 0x4b, 0x03, 0x04 } }, /* (pk)zip */
+ { 5, 0, { '%', 'P', 'D', 'F', '-'} }, /* PDF */
+ { 4, 1, { 0xff, 0xd8, 0xff, 0xe0 } }, /* Maybe JFIF */
+ { 5, 2, { 0x89, 'P','N','G', 0x0d} } /* Likely PNG */
+ };
+
+ if (!inp)
+ return 0;
+
+ for ( ; inp->chain; inp = inp->chain )
+ ;
+
+ buflen = iobuf_ioctl (inp, IOBUF_IOCTL_PEEK, sizeof buf, buf);
+ if (buflen < 0)
+ {
+ buflen = 0;
+ log_debug ("peeking at input failed\n");
+ }
+
+ if ( buflen < 6 )
+ {
+ return 0; /* Too short to check - assume uncompressed. */
+ }
+
+ for ( i = 0; i < DIM (magic); i++ )
+ {
+ if (!memcmp( buf, magic[i].magic, magic[i].len))
+ {
+ switch (magic[i].extchk)
+ {
+ case 0:
+ return 1; /* Is compressed. */
+ case 1:
+ if (buflen > 11 && !memcmp (buf + 6, "JFIF", 5))
+ return 1; /* JFIF: this likely a compressed JPEG. */
+ break;
+ case 2:
+ if (buflen > 8
+ && buf[5] == 0x0a && buf[6] == 0x1a && buf[7] == 0x0a)
+ return 1; /* This is a PNG. */
+ break;
+ default:
+ break;
+ }
+ }
+ }
+
+ if (buflen >= 6 && is_openpgp_compressed_packet (buf, buflen))
+ {
+ return 1; /* Already compressed. */
+ }
+
+ return 0; /* Not detected as compressed. */
+}
--- a/common/iobuf.h
+++ b/common/iobuf.h
@@ -595,10 +595,13 @@ void iobuf_set_partial_body_length_mode
Recall: a filter can return EOF. In this case, it and all
preceding filters are popped from the pipeline and the next read is
from the following filter (which may or may not return EOF). */
void iobuf_skip_rest (iobuf_t a, unsigned long n, int partial);
+/* Check if the file is compressed, by peeking the iobuf. */
+int is_file_compressed (iobuf_t inp);
+
#define iobuf_where(a) "[don't know]"
/* Each time a filter is allocated (via iobuf_alloc()), a
monotonically increasing counter is incremented and this field is
set to the new value. This macro returns that number. */
--- a/common/miscellaneous.c
+++ b/common/miscellaneous.c
@@ -461,116 +461,10 @@ decode_c_string (const char *src)
return buffer;
}
-/* Check whether (BUF,LEN) is valid header for an OpenPGP compressed
- * packet. LEN should be at least 6. */
-static int
-is_openpgp_compressed_packet (const unsigned char *buf, size_t len)
-{
- int c, ctb, pkttype;
- int lenbytes;
-
- ctb = *buf++; len--;
- if (!(ctb & 0x80))
- return 0; /* Invalid packet. */
-
- if ((ctb & 0x40)) /* New style (OpenPGP) CTB. */
- {
- pkttype = (ctb & 0x3f);
- if (!len)
- return 0; /* Expected first length octet missing. */
- c = *buf++; len--;
- if (c < 192)
- ;
- else if (c < 224)
- {
- if (!len)
- return 0; /* Expected second length octet missing. */
- }
- else if (c == 255)
- {
- if (len < 4)
- return 0; /* Expected length octets missing */
- }
- }
- else /* Old style CTB. */
- {
- pkttype = (ctb>>2)&0xf;
- lenbytes = ((ctb&3)==3)? 0 : (1<<(ctb & 3));
- if (len < lenbytes)
- return 0; /* Not enough length bytes. */
- }
-
- return (pkttype == 8);
-}
-
-
-
-/*
- * Check if the file is compressed. You need to pass the first bytes
- * of the file as (BUF,BUFLEN). Returns true if the buffer seems to
- * be compressed.
- */
-int
-is_file_compressed (const byte *buf, unsigned int buflen)
-{
- int i;
-
- struct magic_compress_s
- {
- byte len;
- byte extchk;
- byte magic[5];
- } magic[] =
- {
- { 3, 0, { 0x42, 0x5a, 0x68, 0x00 } }, /* bzip2 */
- { 3, 0, { 0x1f, 0x8b, 0x08, 0x00 } }, /* gzip */
- { 4, 0, { 0x50, 0x4b, 0x03, 0x04 } }, /* (pk)zip */
- { 5, 0, { '%', 'P', 'D', 'F', '-'} }, /* PDF */
- { 4, 1, { 0xff, 0xd8, 0xff, 0xe0 } }, /* Maybe JFIF */
- { 5, 2, { 0x89, 'P','N','G', 0x0d} } /* Likely PNG */
- };
-
- if ( buflen < 6 )
- {
- return 0; /* Too short to check - assume uncompressed. */
- }
-
- for ( i = 0; i < DIM (magic); i++ )
- {
- if (!memcmp( buf, magic[i].magic, magic[i].len))
- {
- switch (magic[i].extchk)
- {
- case 0:
- return 1; /* Is compressed. */
- case 1:
- if (buflen > 11 && !memcmp (buf + 6, "JFIF", 5))
- return 1; /* JFIF: this likely a compressed JPEG. */
- break;
- case 2:
- if (buflen > 8
- && buf[5] == 0x0a && buf[6] == 0x1a && buf[7] == 0x0a)
- return 1; /* This is a PNG. */
- break;
- default:
- break;
- }
- }
- }
-
- if (buflen >= 6 && is_openpgp_compressed_packet (buf, buflen))
- {
- return 1; /* Already compressed. */
- }
-
- return 0; /* Not detected as compressed. */
-}
-
-
/* Try match against each substring of multistr, delimited by | */
int
match_multistr (const char *multistr,const char *match)
{
do
--- a/common/util.h
+++ b/common/util.h
@@ -343,12 +343,10 @@ void print_hexstring (FILE *fp, const vo
int reserved);
char *try_make_printable_string (const void *p, size_t n, int delim);
char *make_printable_string (const void *p, size_t n, int delim);
char *decode_c_string (const char *src);
-int is_file_compressed (const byte *buf, unsigned int buflen);
-
int match_multistr (const char *multistr,const char *match);
int gnupg_compare_version (const char *a, const char *b);
struct debug_flags_s
--- a/g10/cipher.c
+++ b/g10/cipher.c
@@ -88,13 +88,10 @@ write_cfb_header (cipher_filter_context_
log_info (_("WARNING: "
"encrypting without integrity protection is dangerous\n"));
log_info (_("Hint: Do not use option %s\n"), "--rfc2440");
}
- write_status_printf (STATUS_BEGIN_ENCRYPTION, "%d %d",
- ed.mdc_method, cfx->dek->algo);
-
init_packet (&pkt);
pkt.pkttype = cfx->dek->use_mdc? PKT_ENCRYPTED_MDC : PKT_ENCRYPTED;
pkt.pkt.encrypted = &ed;
if (build_packet( a, &pkt))
log_bug ("build_packet(ENCR_DATA) failed\n");
@@ -198,10 +195,16 @@ cipher_filter_cfb (void *opaque, int con
}
else if (control == IOBUFCTRL_DESC)
{
mem2str (buf, "cipher_filter_cfb", *ret_len);
}
+ else if (control == IOBUFCTRL_INIT)
+ {
+ write_status_printf (STATUS_BEGIN_ENCRYPTION, "%d %d",
+ cfx->dek->use_mdc ? DIGEST_ALGO_SHA1 : 0,
+ cfx->dek->algo);
+ }
return rc;
}
@@ -313,12 +316,10 @@ write_ocb_header (cipher_filter_context_
if (DBG_FILTER)
log_debug ("aead packet: len=%lu extralen=%d\n",
(unsigned long)ed.len, ed.extralen);
- write_status_printf (STATUS_BEGIN_ENCRYPTION, "0 %d %d",
- cfx->dek->algo, ed.aead_algo);
print_cipher_algo_note (cfx->dek->algo);
if (build_packet( a, &pkt))
log_bug ("build_packet(ENCRYPTED_AEAD) failed\n");
@@ -627,8 +628,13 @@ cipher_filter_ocb (void *opaque, int con
}
else if (control == IOBUFCTRL_DESC)
{
mem2str (buf, "cipher_filter_ocb", *ret_len);
}
+ else if (control == IOBUFCTRL_INIT)
+ {
+ write_status_printf (STATUS_BEGIN_ENCRYPTION, "0 %d %d",
+ cfx->dek->algo, cfx->dek->use_aead);
+ }
return rc;
}
--- a/g10/encrypt.c
+++ b/g10/encrypt.c
@@ -335,12 +335,10 @@ encrypt_simple (const char *filename, in
armor_filter_context_t *afx = NULL;
compress_filter_context_t zfx;
text_filter_context_t tfx;
progress_filter_context_t *pfx;
int do_compress = !!default_compress_algo();
- char peekbuf[32];
- int peekbuflen;
if (!gnupg_rng_is_compliant (opt.compliance))
{
rc = gpg_error (GPG_ERR_FORBIDDEN);
log_error (_("%s is not compliant with %s mode\n"),
@@ -373,18 +371,10 @@ encrypt_simple (const char *filename, in
strerror(errno) );
release_progress_context (pfx);
return rc;
}
- peekbuflen = iobuf_ioctl (inp, IOBUF_IOCTL_PEEK, sizeof peekbuf, peekbuf);
- if (peekbuflen < 0)
- {
- peekbuflen = 0;
- if (DBG_FILTER)
- log_debug ("peeking at input failed\n");
- }
-
handle_progress (pfx, inp, filename);
if (opt.textmode)
iobuf_push_filter( inp, text_filter, &tfx );
@@ -442,21 +432,10 @@ encrypt_simple (const char *filename, in
openpgp_cipher_algo_name (cfx.dek->algo),
cfx.dek->use_aead? openpgp_aead_algo_name (cfx.dek->use_aead)
/**/ : "CFB");
}
- if (do_compress
- && cfx.dek
- && (cfx.dek->use_mdc || cfx.dek->use_aead)
- && !opt.explicit_compress_option
- && is_file_compressed (peekbuf, peekbuflen))
- {
- if (opt.verbose)
- log_info(_("'%s' already compressed\n"), filename? filename: "[stdin]");
- do_compress = 0;
- }
-
if ( rc || (rc = open_outfile (-1, filename, opt.armor? 1:0, 0, &out )))
{
iobuf_cancel (inp);
xfree (cfx.dek);
xfree (s2k);
@@ -522,10 +501,28 @@ encrypt_simple (const char *filename, in
filesize = 0;
}
else
filesize = opt.set_filesize ? opt.set_filesize : 0; /* stdin */
+ /* Register the cipher filter. */
+ if (mode)
+ iobuf_push_filter (out,
+ cfx.dek->use_aead? cipher_filter_ocb
+ /**/ : cipher_filter_cfb,
+ &cfx );
+
+ if (do_compress
+ && cfx.dek
+ && (cfx.dek->use_mdc || cfx.dek->use_aead)
+ && !opt.explicit_compress_option
+ && is_file_compressed (inp))
+ {
+ if (opt.verbose)
+ log_info(_("'%s' already compressed\n"), filename? filename: "[stdin]");
+ do_compress = 0;
+ }
+
if (!opt.no_literal)
{
/* Note that PT has been initialized above in !no_literal mode. */
pt->timestamp = make_timestamp();
pt->mode = opt.mimemode? 'm' : opt.textmode? 't' : 'b';
@@ -541,17 +538,10 @@ encrypt_simple (const char *filename, in
cfx.datalen = filesize && !do_compress ? filesize : 0;
pkt.pkttype = 0;
pkt.pkt.generic = NULL;
}
- /* Register the cipher filter. */
- if (mode)
- iobuf_push_filter (out,
- cfx.dek->use_aead? cipher_filter_ocb
- /**/ : cipher_filter_cfb,
- &cfx );
-
/* Register the compress filter. */
if ( do_compress )
{
if (cfx.dek && (cfx.dek->use_mdc || cfx.dek->use_aead))
zfx.new_ctb = 1;
@@ -779,21 +769,19 @@ encrypt_crypt (ctrl_t ctrl, int filefd,
iobuf_t out = NULL;
PACKET pkt;
PKT_plaintext *pt = NULL;
DEK *symkey_dek = NULL;
STRING2KEY *symkey_s2k = NULL;
- int rc = 0, rc2 = 0;
+ int rc = 0;
u32 filesize;
cipher_filter_context_t cfx;
armor_filter_context_t *afx = NULL;
compress_filter_context_t zfx;
text_filter_context_t tfx;
progress_filter_context_t *pfx;
PK_LIST pk_list;
int do_compress;
- char peekbuf[32];
- int peekbuflen;
if (filefd != -1 && filename)
return gpg_error (GPG_ERR_INV_ARG); /* Both given. */
do_compress = !!opt.compress_algo;
@@ -862,18 +850,10 @@ encrypt_crypt (ctrl_t ctrl, int filefd,
}
if (opt.verbose)
log_info (_("reading from '%s'\n"), iobuf_get_fname_nonnull (inp));
- peekbuflen = iobuf_ioctl (inp, IOBUF_IOCTL_PEEK, sizeof peekbuf, peekbuf);
- if (peekbuflen < 0)
- {
- peekbuflen = 0;
- if (DBG_FILTER)
- log_debug ("peeking at input failed\n");
- }
-
handle_progress (pfx, inp, filename);
if (opt.textmode)
iobuf_push_filter (inp, text_filter, &tfx);
@@ -897,30 +877,10 @@ encrypt_crypt (ctrl_t ctrl, int filefd,
cfx.dek->use_aead = use_aead (pk_list, cfx.dek->algo);
if (!cfx.dek->use_aead)
cfx.dek->use_mdc = !!use_mdc (pk_list, cfx.dek->algo);
- /* Only do the is-file-already-compressed check if we are using a
- MDC. This forces compressed files to be re-compressed if we do
- not have a MDC to give some protection against chosen ciphertext
- attacks. */
-
- if (do_compress
- && (cfx.dek->use_mdc || cfx.dek->use_aead)
- && !opt.explicit_compress_option
- && is_file_compressed (peekbuf, peekbuflen))
- {
- if (opt.verbose)
- log_info(_("'%s' already compressed\n"), filename? filename: "[stdin]");
- do_compress = 0;
- }
- if (rc2)
- {
- rc = rc2;
- goto leave;
- }
-
make_session_key (cfx.dek);
if (DBG_CRYPTO)
log_printhex (cfx.dek->key, cfx.dek->keylen, "DEK is: ");
rc = write_pubkey_enc_from_list (ctrl, pk_list, cfx.dek, out);
@@ -957,10 +917,30 @@ encrypt_crypt (ctrl_t ctrl, int filefd,
filesize = 0;
}
else
filesize = opt.set_filesize ? opt.set_filesize : 0; /* stdin */
+ /* Register the cipher filter. */
+ iobuf_push_filter (out,
+ cfx.dek->use_aead? cipher_filter_ocb
+ /**/ : cipher_filter_cfb,
+ &cfx);
+
+ /* Only do the is-file-already-compressed check if we are using a
+ * MDC or AEAD. This forces compressed files to be re-compressed if
+ * we do not have a MDC to give some protection against chosen
+ * ciphertext attacks. */
+ if (do_compress
+ && (cfx.dek->use_mdc || cfx.dek->use_aead)
+ && !opt.explicit_compress_option
+ && is_file_compressed (inp))
+ {
+ if (opt.verbose)
+ log_info(_("'%s' already compressed\n"), filename? filename: "[stdin]");
+ do_compress = 0;
+ }
+
if (!opt.no_literal)
{
pt->timestamp = make_timestamp();
pt->mode = opt.mimemode? 'm' : opt.textmode ? 't' : 'b';
pt->len = filesize;
@@ -971,16 +951,10 @@ encrypt_crypt (ctrl_t ctrl, int filefd,
cfx.datalen = filesize && !do_compress? calc_packet_length( &pkt ) : 0;
}
else
cfx.datalen = filesize && !do_compress ? filesize : 0;
- /* Register the cipher filter. */
- iobuf_push_filter (out,
- cfx.dek->use_aead? cipher_filter_ocb
- /**/ : cipher_filter_cfb,
- &cfx);
-
/* Register the compress filter. */
if (do_compress)
{
int compr_algo = opt.compress_algo;
--- a/g10/sign.c
+++ b/g10/sign.c
@@ -936,13 +936,10 @@ sign_file (ctrl_t ctrl, strlist_t filena
PK_LIST pk_list = NULL;
SK_LIST sk_list = NULL;
SK_LIST sk_rover = NULL;
int multifile = 0;
u32 duration=0;
- char peekbuf[32];
- int peekbuflen = 0;
-
pfx = new_progress_context ();
afx = new_armor_context ();
memset (&zfx, 0, sizeof zfx);
memset (&mfx, 0, sizeof mfx);
@@ -997,18 +994,10 @@ sign_file (ctrl_t ctrl, strlist_t filena
log_error (_("can't open '%s': %s\n"), fname? fname: "[stdin]",
strerror(errno) );
goto leave;
}
- peekbuflen = iobuf_ioctl (inp, IOBUF_IOCTL_PEEK, sizeof peekbuf, peekbuf);
- if (peekbuflen < 0)
- {
- peekbuflen = 0;
- if (DBG_FILTER)
- log_debug ("peeking at input failed\n");
- }
-
handle_progress (pfx, inp, fname);
}
if (outfile)
{
@@ -1157,11 +1146,11 @@ sign_file (ctrl_t ctrl, strlist_t filena
if (opt.compress_algo && !outfile && !detached)
{
int compr_algo = opt.compress_algo;
if (!opt.explicit_compress_option
- && is_file_compressed (peekbuf, peekbuflen))
+ && is_file_compressed (inp))
{
if (opt.verbose)
log_info(_("'%s' already compressed\n"), fname? fname: "[stdin]");
compr_algo = 0;
}
