pgcrypto: Report errant decryption as "Wrong key or corrupt data".

This has been the predominant outcome.  When the output of decrypting
with a wrong key coincidentally resembled an OpenPGP packet header,
pgcrypto could instead report "Corrupt data", "Not text data" or
"Unsupported compression algorithm".  The distinct "Corrupt data"
message added no value.  The latter two error messages misled when the
decrypted payload also exhibited fundamental integrity problems.  Worse,
error message variance in other systems has enabled cryptologic attacks;
see RFC 4880 section "14. Security Considerations".  Whether these
pgcrypto behaviors are likewise exploitable is unknown.

In passing, document that pgcrypto does not resist side-channel attacks.
Back-patch to 9.0 (all supported versions).

Security: CVE-2015-3167

9 files changed, 162 insertions, 27 deletions

diff --git a/contrib/pgcrypto/expected/pgp-decrypt.out b/contrib/pgcrypto/expected/pgp-decrypt.out
index 7193dca026..2dabfaf7b0 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 d290a1349f..b4b6810a3c 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 c59691ed2c..44d9adcd2a 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 c0c5773e66..5c69745156 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:
@@ -1168,8 +1174,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 398f21bca2..2ce429d1b2 100644
--- a/contrib/pgcrypto/pgp.h
+++ b/contrib/pgcrypto/pgp.h
@@ -153,7 +153,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 93c436daa0..cfb3b50985 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 7255ebf04c..0f6bbd7a8d 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 5457152ccf..f46a18f8cf 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 d409446c37..bfcbe02f85 100644
--- a/doc/src/sgml/pgcrypto.sgml
+++ b/doc/src/sgml/pgcrypto.sgml
@@ -1270,6 +1270,14 @@ gen_random_uuid() returns uuid
    <para>
     If you cannot, then better do crypto inside client application.
    </para>
+
+   <para>
+    The implementation does not resist
+    <ulink url="http://en.wikipedia.org/wiki/Side-channel_attack">side-channel
+    attacks</ulink>.  For example, the time required for
+    a <filename>pgcrypto</> decryption function to complete varies among
+    ciphertexts of a given size.
+   </para>
   </sect3>
 
   <sect3>