diff --git a/contrib/pgcrypto/expected/pgp-decrypt.out b/contrib/pgcrypto/expected/pgp-decrypt.out index 7193dca0262..2dabfaf7b0e 100644 --- a/contrib/pgcrypto/expected/pgp-decrypt.out +++ b/contrib/pgcrypto/expected/pgp-decrypt.out @@ -372,3 +372,54 @@ select pgp_sym_decrypt(pgp_sym_encrypt(repeat('x',65530),'1'),'1') = repeat('x', (1 row) -- expected: true +-- Negative tests +-- Decryption with a certain incorrect key yields an apparent Literal Data +-- packet reporting its content to be binary data. Ciphertext source: +-- iterative pgp_sym_encrypt('secret', 'key') until the random prefix gave +-- rise to that property. +select pgp_sym_decrypt(dearmor(' +-----BEGIN PGP MESSAGE----- + +ww0EBwMCxf8PTrQBmJdl0jcB6y2joE7GSLKRv7trbNsF5Z8ou5NISLUg31llVH/S0B2wl4bvzZjV +VsxxqLSPzNLAeIspJk5G +=mSd/ +-----END PGP MESSAGE----- +'), 'wrong-key', 'debug=1'); +NOTICE: dbg: prefix_init: corrupt prefix +NOTICE: dbg: parse_literal_data: data type=b +NOTICE: dbg: mdcbuf_finish: bad MDC pkt hdr +ERROR: Wrong key or corrupt data +-- Routine text/binary mismatch. +select pgp_sym_decrypt(pgp_sym_encrypt_bytea('P', 'key'), 'key', 'debug=1'); +NOTICE: dbg: parse_literal_data: data type=b +ERROR: Not text data +-- Decryption with a certain incorrect key yields an apparent BZip2-compressed +-- plaintext. Ciphertext source: iterative pgp_sym_encrypt('secret', 'key') +-- until the random prefix gave rise to that property. +select pgp_sym_decrypt(dearmor(' +-----BEGIN PGP MESSAGE----- + +ww0EBwMC9rK/dMkF5Zlt0jcBlzAQ1mQY2qYbKYbw8h3EZ5Jk0K2IiY92R82TRhWzBIF/8cmXDPtP +GXsd65oYJZp3Khz0qfyn +=Nmpq +-----END PGP MESSAGE----- +'), 'wrong-key', 'debug=1'); +NOTICE: dbg: prefix_init: corrupt prefix +NOTICE: dbg: parse_compressed_data: bzip2 unsupported +NOTICE: dbg: mdcbuf_finish: bad MDC pkt hdr +ERROR: Wrong key or corrupt data +-- Routine use of BZip2 compression. Ciphertext source: +-- echo x | gpg --homedir /nonexistent --personal-compress-preferences bzip2 \ +-- --personal-cipher-preferences aes --no-emit-version --batch \ +-- --symmetric --passphrase key --armor +select pgp_sym_decrypt(dearmor(' +-----BEGIN PGP MESSAGE----- + +jA0EBwMCRhFrAKNcLVJg0mMBLJG1cCASNk/x/3dt1zJ+2eo7jHfjgg3N6wpB3XIe +QCwkWJwlBG5pzbO5gu7xuPQN+TbPJ7aQ2sLx3bAHhtYb0i3vV9RO10Gw++yUyd4R +UCAAw2JRIISttRHMfDpDuZJpvYo= +=AZ9M +-----END PGP MESSAGE----- +'), 'key', 'debug=1'); +NOTICE: dbg: parse_compressed_data: bzip2 unsupported +ERROR: Unsupported compression algorithm diff --git a/contrib/pgcrypto/expected/pgp-pubkey-decrypt.out b/contrib/pgcrypto/expected/pgp-pubkey-decrypt.out index d290a1349f9..b4b6810a3c5 100644 --- a/contrib/pgcrypto/expected/pgp-pubkey-decrypt.out +++ b/contrib/pgcrypto/expected/pgp-pubkey-decrypt.out @@ -625,7 +625,7 @@ ERROR: No encryption key found -- rsa: password-protected secret key, wrong password select pgp_pub_decrypt(dearmor(data), dearmor(seckey), '123') from keytbl, encdata where keytbl.id=7 and encdata.id=4; -ERROR: Corrupt data +ERROR: Wrong key or corrupt data -- rsa: password-protected secret key, right password select pgp_pub_decrypt(dearmor(data), dearmor(seckey), 'parool') from keytbl, encdata where keytbl.id=7 and encdata.id=4; @@ -641,7 +641,7 @@ ERROR: Need password for secret key -- password-protected secret key, wrong password select pgp_pub_decrypt(dearmor(data), dearmor(seckey), 'foo') from keytbl, encdata where keytbl.id=5 and encdata.id=1; -ERROR: Corrupt data +ERROR: Wrong key or corrupt data -- password-protected secret key, right password select pgp_pub_decrypt(dearmor(data), dearmor(seckey), 'parool') from keytbl, encdata where keytbl.id=5 and encdata.id=1; diff --git a/contrib/pgcrypto/mbuf.c b/contrib/pgcrypto/mbuf.c index c59691ed2cc..44d9adcd2ab 100644 --- a/contrib/pgcrypto/mbuf.c +++ b/contrib/pgcrypto/mbuf.c @@ -325,7 +325,7 @@ pullf_read_fixed(PullFilter *src, int len, uint8 *dst) if (res != len) { px_debug("pullf_read_fixed: need=%d got=%d", len, res); - return PXE_MBUF_SHORT_READ; + return PXE_PGP_CORRUPT_DATA; } if (p != dst) memcpy(dst, p, len); diff --git a/contrib/pgcrypto/pgp-decrypt.c b/contrib/pgcrypto/pgp-decrypt.c index 2c744b73a30..55119e1e112 100644 --- a/contrib/pgcrypto/pgp-decrypt.c +++ b/contrib/pgcrypto/pgp-decrypt.c @@ -236,6 +236,8 @@ pgp_create_pkt_reader(PullFilter **pf_p, PullFilter *src, int len, /* * Prefix check filter + * https://tools.ietf.org/html/rfc4880#section-5.7 + * https://tools.ietf.org/html/rfc4880#section-5.13 */ static int @@ -264,20 +266,7 @@ prefix_init(void **priv_p, void *arg, PullFilter *src) if (buf[len - 2] != buf[len] || buf[len - 1] != buf[len + 1]) { px_debug("prefix_init: corrupt prefix"); - - /* - * The original purpose of the 2-byte check was to show user a - * friendly "wrong key" message. This made following possible: - * - * "An Attack on CFB Mode Encryption As Used By OpenPGP" by Serge - * Mister and Robert Zuccherato - * - * To avoid being 'oracle', we delay reporting, which basically means - * we prefer to run into corrupt packet header. - * - * We _could_ throw PXE_PGP_CORRUPT_DATA here, but there is - * possibility of attack via timing, so we don't. - */ + /* report error in pgp_decrypt() */ ctx->corrupt_prefix = 1; } px_memset(tmpbuf, 0, sizeof(tmpbuf)); @@ -788,12 +777,15 @@ parse_literal_data(PGP_Context *ctx, MBuf *dst, PullFilter *pkt) } px_memset(tmpbuf, 0, 4); - /* check if text */ + /* + * If called from an SQL function that returns text, pgp_decrypt() rejects + * inputs not self-identifying as text. + */ if (ctx->text_mode) if (type != 't' && type != 'u') { px_debug("parse_literal_data: data type=%c", type); - return PXE_PGP_NOT_TEXT; + ctx->unexpected_binary = true; } ctx->unicode_mode = (type == 'u') ? 1 : 0; @@ -827,6 +819,7 @@ parse_compressed_data(PGP_Context *ctx, MBuf *dst, PullFilter *pkt) int res; uint8 type; PullFilter *pf_decompr; + uint8 *discard_buf; GETBYTE(pkt, type); @@ -850,7 +843,20 @@ parse_compressed_data(PGP_Context *ctx, MBuf *dst, PullFilter *pkt) case PGP_COMPR_BZIP2: px_debug("parse_compressed_data: bzip2 unsupported"); - res = PXE_PGP_UNSUPPORTED_COMPR; + /* report error in pgp_decrypt() */ + ctx->unsupported_compr = 1; + + /* + * Discard the compressed data, allowing it to first affect any + * MDC digest computation. + */ + while (1) + { + res = pullf_read(pkt, 32 * 1024, &discard_buf); + if (res <= 0) + break; + } + break; default: @@ -1171,8 +1177,36 @@ pgp_decrypt(PGP_Context *ctx, MBuf *msrc, MBuf *mdst) if (res < 0) return res; + /* + * Report a failure of the prefix_init() "quick check" now, rather than + * upon detection, to hinder timing attacks. pgcrypto is not generally + * secure against timing attacks, but this helps. + */ if (!got_data || ctx->corrupt_prefix) - res = PXE_PGP_CORRUPT_DATA; + return PXE_PGP_CORRUPT_DATA; + + /* + * Code interpreting purportedly-decrypted data prior to this stage shall + * report no error other than PXE_PGP_CORRUPT_DATA. (PXE_BUG is okay so + * long as it remains unreachable.) This ensures that an attacker able to + * choose a ciphertext and receive a corresponding decryption error + * message cannot use that oracle to gather clues about the decryption + * key. See "An Attack on CFB Mode Encryption As Used By OpenPGP" by + * Serge Mister and Robert Zuccherato. + * + * A problematic value in the first octet of a Literal Data or Compressed + * Data packet may indicate a simple user error, such as the need to call + * pgp_sym_decrypt_bytea instead of pgp_sym_decrypt. Occasionally, + * though, it is the first symptom of the encryption key not matching the + * decryption key. When this was the only problem encountered, report a + * specific error to guide the user; otherwise, we will have reported + * PXE_PGP_CORRUPT_DATA before now. A key mismatch makes the other errors + * into red herrings, and this avoids leaking clues to attackers. + */ + if (ctx->unsupported_compr) + return PXE_PGP_UNSUPPORTED_COMPR; + if (ctx->unexpected_binary) + return PXE_PGP_NOT_TEXT; return res; } diff --git a/contrib/pgcrypto/pgp.h b/contrib/pgcrypto/pgp.h index efcfa943bd1..dad7c8d8790 100644 --- a/contrib/pgcrypto/pgp.h +++ b/contrib/pgcrypto/pgp.h @@ -148,7 +148,9 @@ struct PGP_Context * internal variables */ int mdc_checked; - int corrupt_prefix; + int corrupt_prefix; /* prefix failed RFC 4880 "quick check" */ + int unsupported_compr; /* has bzip2 compression */ + int unexpected_binary; /* binary data seen in text_mode */ int in_mdc_pkt; int use_mdcbuf_filter; PX_MD *mdc_ctx; diff --git a/contrib/pgcrypto/px.c b/contrib/pgcrypto/px.c index 93c436daa0d..cfb3b50985a 100644 --- a/contrib/pgcrypto/px.c +++ b/contrib/pgcrypto/px.c @@ -87,9 +87,6 @@ static const struct error_desc px_err_list[] = { {PXE_PGP_UNSUPPORTED_PUBALGO, "Unsupported public key algorithm"}, {PXE_PGP_MULTIPLE_SUBKEYS, "Several subkeys not supported"}, - /* fake this as PXE_PGP_CORRUPT_DATA */ - {PXE_MBUF_SHORT_READ, "Corrupt data"}, - {0, NULL}, }; diff --git a/contrib/pgcrypto/px.h b/contrib/pgcrypto/px.h index a01a58e29c0..d237d97017d 100644 --- a/contrib/pgcrypto/px.h +++ b/contrib/pgcrypto/px.h @@ -80,8 +80,6 @@ void px_free(void *p); #define PXE_NO_RANDOM -17 #define PXE_DECRYPT_FAILED -18 -#define PXE_MBUF_SHORT_READ -50 - #define PXE_PGP_CORRUPT_DATA -100 #define PXE_PGP_CORRUPT_ARMOR -101 #define PXE_PGP_UNSUPPORTED_COMPR -102 diff --git a/contrib/pgcrypto/sql/pgp-decrypt.sql b/contrib/pgcrypto/sql/pgp-decrypt.sql index 5457152ccf6..f46a18f8cfd 100644 --- a/contrib/pgcrypto/sql/pgp-decrypt.sql +++ b/contrib/pgcrypto/sql/pgp-decrypt.sql @@ -268,3 +268,48 @@ a3nsOzKTXUfS9VyaXo8IrncM6n7fdaXpwba/3tNsAhJG4lDv1k4g9v8Ix2dfv6Rs -- check BUG #11905, problem with messages 6 less than a power of 2. select pgp_sym_decrypt(pgp_sym_encrypt(repeat('x',65530),'1'),'1') = repeat('x',65530); -- expected: true + + +-- Negative tests + +-- Decryption with a certain incorrect key yields an apparent Literal Data +-- packet reporting its content to be binary data. Ciphertext source: +-- iterative pgp_sym_encrypt('secret', 'key') until the random prefix gave +-- rise to that property. +select pgp_sym_decrypt(dearmor(' +-----BEGIN PGP MESSAGE----- + +ww0EBwMCxf8PTrQBmJdl0jcB6y2joE7GSLKRv7trbNsF5Z8ou5NISLUg31llVH/S0B2wl4bvzZjV +VsxxqLSPzNLAeIspJk5G +=mSd/ +-----END PGP MESSAGE----- +'), 'wrong-key', 'debug=1'); + +-- Routine text/binary mismatch. +select pgp_sym_decrypt(pgp_sym_encrypt_bytea('P', 'key'), 'key', 'debug=1'); + +-- Decryption with a certain incorrect key yields an apparent BZip2-compressed +-- plaintext. Ciphertext source: iterative pgp_sym_encrypt('secret', 'key') +-- until the random prefix gave rise to that property. +select pgp_sym_decrypt(dearmor(' +-----BEGIN PGP MESSAGE----- + +ww0EBwMC9rK/dMkF5Zlt0jcBlzAQ1mQY2qYbKYbw8h3EZ5Jk0K2IiY92R82TRhWzBIF/8cmXDPtP +GXsd65oYJZp3Khz0qfyn +=Nmpq +-----END PGP MESSAGE----- +'), 'wrong-key', 'debug=1'); + +-- Routine use of BZip2 compression. Ciphertext source: +-- echo x | gpg --homedir /nonexistent --personal-compress-preferences bzip2 \ +-- --personal-cipher-preferences aes --no-emit-version --batch \ +-- --symmetric --passphrase key --armor +select pgp_sym_decrypt(dearmor(' +-----BEGIN PGP MESSAGE----- + +jA0EBwMCRhFrAKNcLVJg0mMBLJG1cCASNk/x/3dt1zJ+2eo7jHfjgg3N6wpB3XIe +QCwkWJwlBG5pzbO5gu7xuPQN+TbPJ7aQ2sLx3bAHhtYb0i3vV9RO10Gw++yUyd4R +UCAAw2JRIISttRHMfDpDuZJpvYo= +=AZ9M +-----END PGP MESSAGE----- +'), 'key', 'debug=1'); diff --git a/doc/src/sgml/pgcrypto.sgml b/doc/src/sgml/pgcrypto.sgml index 1e7e402a569..254fc0776bd 100644 --- a/doc/src/sgml/pgcrypto.sgml +++ b/doc/src/sgml/pgcrypto.sgml @@ -1224,6 +1224,14 @@ gen_random_bytes(count integer) returns bytea If you cannot, then better do crypto inside client application. + + + The implementation does not resist + side-channel + attacks. For example, the time required for + a pgcrypto decryption function to complete varies among + ciphertexts of a given size. +