Re: pgcrypto: PGP signatures

On Fri, Sep 5, 2014 at 4:38 AM, Marko Tiikkaja <marko(at)joh(dot)to> wrote:

> Hi all,
>
> I've updated the patch with a number of changes:
> 1) I've documented the current limitations of signatures
> 2) I've expanded section F.25.3 to add information about signatures
> (though I'm not sure why this part is in the user-facing documentation in
> the first place).
> 3) I've changed the code to use ntohl() and pg_time_t as per Thomas'
> comments.
> 4) I've changed the code to consistently use "while (1)" instead of "for
> (;;)" (except for the math library, but I didn't touch that at all)
>
> I've also changed the behaviour when passing a message with a signature to
> the decrypt functions which don't verify signatures. They now report
> "ERROR: Wrong key or corrupt data" instead of decrypting and silently
> ignoring the signature. The behaviour is now backwards compatible, but I
> see two ways we could possibly possibly improve this:
> 1) Produce a better error message (I'm sure most people don't know about
> the hidden debug=1 setting)
> 2) Provide an option to ignore the signature if decrypting the data is
> desirable even if the signature can't be verified
>

If i understand the sequence here: The current git HEAD is that
pgp_pub_decrypt would throw an error if given a signed and encrypted
message, and earlier version of your patch changed that to decrypt the
message and ignore the signature, and the current version went back to
throwing an error.

I think I prefer the middle of those behaviors. The original behavior
seems like a bug to me, and I don't think we need to be backwards
compatible with bugs. Why should a function called "decrypt" care if the
message is also signed? That is not its job.

If we decide to throw the error, a better error message certainly wouldn't
hurt. And the output of 'debug=1' is generally not comprehensible unless
you are familiar with the source code, so that is not a substitute.

(By the way, there are now 2 patches in this series named
pgcrypto_sigs.v3.patch--so be careful which one you look it.)

There seems to be a memory leak in pgp_sym_decrypt_verify that does not
exist in pgp_sym_decrypt. It is about 58 bytes per decryption.

Perl test script:

my $dbh=connect(...);
my $pub=`cat public.asc`;
my $pri=`cat private.asc`;

my $enc= $dbh->prepare("select
armor(pgp_sym_encrypt_sign('asdlkfjsldkfjsadf',?,dearmor(?),'debug=1'))");
my $dec= $dbh->prepare("select
pgp_sym_decrypt_verify(dearmor(?),?,dearmor(?),'debug=1')");
my $i=1;

$enc->execute("foobar$i",$pri);
my ($message)=$enc->fetchrow();

foreach my $ii (1..1000000) {
## my $i=$ii;
$dec->execute($message,"foobar$i",$pub);
my ($message2)=$dec->fetchrow();
die unless $message2 eq "asdlkfjsldkfjsadf";
warn "$i\t", time() if $i%1000 ==0;
};

Cheers,

Jeff