Re: pgcrypto: PGP signatures - Mailing list pgsql-hackers

From Jeff Janes
Subject Re: pgcrypto: PGP signatures
Date
Msg-id CAMkU=1w76Ki8KS2E315q9v7gcdLh0_3YTKchYhwCk9qYaVVSxA@mail.gmail.com
Whole thread Raw
In response to Re: pgcrypto: PGP signatures  (Marko Tiikkaja <marko@joh.to>)
Responses Re: pgcrypto: PGP signatures
Re: pgcrypto: PGP signatures
List pgsql-hackers

On Fri, Sep 5, 2014 at 4:38 AM, Marko Tiikkaja <marko@joh.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

pgsql-hackers by date:

Previous
From: Merlin Moncure
Date:
Subject: Re: Scaling shared buffer eviction
Next
From: "Erik Rijkers"
Date:
Subject: Re: BRIN indexes - TRAP: BadArgument