From 8a0c8c52510d9c2d934f85159f04b666286b1786 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Thu, 17 May 2018 09:14:40 +0200 Subject: [PATCH] core: Always fail if an OpenPG message is not integrity protected. * src/decrypt.c (struct op_data_t): Add field not_integrity_protected. (parse_decryption_info): Set this. Also rename mode to aead_algo for clarity. (_gpgme_decrypt_status_handler): Force failure in case of a missing MDC. -- This extra check makes sure that a missing or stripped MDC in - gpg < 2.1 - or gpg 2.2 with an old cipher algorithm will lead to a decryption failure. gpg 2.3 will always fail in this case. Implementing this check here and not backporting the 2.3 change to 2.2 has the benefit that all GPGME using applications are protected but scripts relying on rfc2440 (i.e. without MDC) will only break when migrating to 2.3. Note that S/MIME has no integrity protection mechanism but gpgsm neither emits a DECRYPTION_INFO status line, so an error will not be triggered. If in the future gpgsm supports authenticated encryption it may issue a DECRYPTION_INFO line to force a failure here but it will in that case also emit a DECRYPTION_FAILED anyway. GnuPG-bug-id: 3981 Signed-off-by: Werner Koch --- NEWS | 3 +++ doc/gpgme.texi | 2 +- src/decrypt.c | 27 ++++++++++++++++++++------- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/NEWS b/NEWS index 80c01190..1eacccce 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,9 @@ Noteworthy changes in version 1.11.2 (unreleased) ------------------------------------------------- + * Even for old versions of gpg a missing MDC will now lead to a + decryption failure. + Noteworthy changes in version 1.11.1 (2018-04-20) ------------------------------------------------- diff --git a/doc/gpgme.texi b/doc/gpgme.texi index c4a29d54..c745675b 100644 --- a/doc/gpgme.texi +++ b/doc/gpgme.texi @@ -5408,7 +5408,7 @@ or @code{gpgme_get_ctx_flag (ctx, "export-session-key")} returns true @since{1.11.0} A string with the symmetric encryption algorithm and mode using the -format ".". Note that old non-MDC encryption mode of +format ".". Note that the deprecated non-MDC encryption mode of OpenPGP is given as "PGPCFB". @end table diff --git a/src/decrypt.c b/src/decrypt.c index 0fc7019c..ecd9c144 100644 --- a/src/decrypt.c +++ b/src/decrypt.c @@ -56,6 +56,11 @@ typedef struct * message that the general DECRYPTION_FAILED. */ int any_no_seckey; + /* If the engine emits a DECRYPTION_INFO status and that does not + * indicate that an integrity proetction mode is active, this flag + * is set. */ + int not_integrity_protected; + /* A pointer to the next pointer of the last recipient in the list. This makes appending new invalid signers painless while preserving the order. */ @@ -280,7 +285,7 @@ parse_decryption_info (char *args, op_data_t opd, gpgme_protocol_t protocol) char *field[3]; int nfields; char *args2; - int mdc, mode; + int mdc, aead_algo; const char *algostr, *modestr; if (!args) @@ -296,19 +301,22 @@ parse_decryption_info (char *args, op_data_t opd, gpgme_protocol_t protocol) mdc = atoi (field[0]); algostr = _gpgme_cipher_algo_name (atoi (field[1]), protocol); - mode = nfields < 3? 0 : atoi (field[2]); - modestr = _gpgme_cipher_mode_name (mode, protocol); + aead_algo = nfields < 3? 0 : atoi (field[2]); + modestr = _gpgme_cipher_mode_name (aead_algo, protocol); free (args2); free (opd->result.symkey_algo); - if (!mode && mdc != 2) + if (!aead_algo && mdc != 2) opd->result.symkey_algo = _gpgme_strconcat (algostr, ".PGPCFB", NULL); else opd->result.symkey_algo = _gpgme_strconcat (algostr, ".", modestr, NULL); if (!opd->result.symkey_algo) return gpg_error_from_syserror (); + if (!mdc && !aead_algo) + opd->not_integrity_protected = 1; + return 0; } @@ -338,13 +346,18 @@ _gpgme_decrypt_status_handler (void *priv, gpgme_status_code_t code, break; case GPGME_STATUS_EOF: - /* FIXME: These error values should probably be attributed to - the underlying crypto engine (as error source). */ + /* We force an encryption failure if we know that integrity + * protection is missing. For modern version of gpg using + * modern cipher algorithms this is not required because gpg + * will issue a failure anyway. However older gpg versions emit + * only a warning. + * Fixme: These error values should probably be attributed to + * the underlying crypto engine (as error source). */ if (opd->failed && opd->pkdecrypt_failed) return opd->pkdecrypt_failed; else if (opd->failed && opd->any_no_seckey) return gpg_error (GPG_ERR_NO_SECKEY); - else if (opd->failed) + else if (opd->failed || opd->not_integrity_protected) return gpg_error (GPG_ERR_DECRYPT_FAILED); else if (!opd->okay) return gpg_error (GPG_ERR_NO_DATA);