public inbox for [email protected]  
help / color / mirror / Atom feed
From: cary huang <[email protected]>
To: [email protected]
Cc: shihao zhong <[email protected]>
Subject: Re: Encoding protection for pgcrypto
Date: Fri, 09 Feb 2024 22:33:17 +0000
Message-ID: <170751799724.1131.10362204887069158737.pgcf@coridan.postgresql.org> (raw)
In-Reply-To: <CAGRkXqRZyo0gLxPJqUsDqtWYBbgM14betsHiLRPj9mo2=z9VvA@mail.gmail.com>
References: <CAGRkXqRZyo0gLxPJqUsDqtWYBbgM14betsHiLRPj9mo2=z9VvA@mail.gmail.com>
	<CAGRkXqRZyo0gLxPJqUsDqtWYBbgM14betsHiLRPj9mo2=z9VvA@mail.gmail.com>

The following review has been posted through the commitfest application:
make installcheck-world:  tested, failed
Implements feature:       not tested
Spec compliant:           not tested
Documentation:            not tested

Hello

I had a look at your patch, which applies fine to PostgreSQL master. I noticed that the new regression tests you have added to test utf-8 encoding fails on my setup (make check) with the following diffs:

---------------------------------------
@ -13,6 +13,7 @@
 =ivrD
 -----END PGP MESSAGE-----
 '), '0123456789abcdefghij'), 'sha1');
+ERROR:  invalid byte sequence for encoding "UTF8": 0x91
 -- Database encoding protection.  Ciphertext source:
 -- printf '\xe0\xe0\xbff' | gpg --batch --passphrase mykey --textmode --armor --symmetric
 select pgp_sym_decrypt(dearmor('
@@ -23,5 +24,5 @@
 =QKy4
 -----END PGP MESSAGE-----
 '), 'mykey', 'debug=1');
+ERROR:  invalid byte sequence for encoding "UTF8": 0xe0 0xe0 0xbf
 \quit
-\endif
---------------------------------------

I am not sure but it seems that you intentionally provide a text input that would produce a non-utf-8 compliant decrypted output, which triggers the error from within "pg_verifymbstr()" call that you have added in pgp-pgsql.c? Are the errors expected in your new test case? If so, then the tests shall pass instead because it has caught a invalid encoding in decrypted output.

Generally, I am ok with the extra encoding check after text decryption but do not think if it is a good idea to just error out and abort the transaction when it detects invalid encoding character. text decryption routines may be used quite frequently and users generally do not expect them to abort transaction. It may be ok to just give them a warning about invalid character encoding. 

thanks
--------------------
Cary Huang
Highgo Software - Canada
www.highgo.ca

view thread (3+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Encoding protection for pgcrypto
  In-Reply-To: <170751799724.1131.10362204887069158737.pgcf@coridan.postgresql.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox