pgcrypto: PGP signatures

On 8/6/14 2:46 PM, I wrote:

Attached is a patch to add support for PGP signatures in encrypted
messages into pgcrypto.

Here's v2 of the patch. I've changed the info-extracting code to not
look for signatures beyond the data, which also meant that it had to
parse one-pass signatures (which it didn't do before). This matches the
behaviour of the main decryption code.

.marko

Hi,

On 8/7/14 12:15 PM, I wrote:

Here's v2 of the patch. I've changed the info-extracting code to not
look for signatures beyond the data, which also meant that it had to
parse one-pass signatures (which it didn't do before). This matches the
behaviour of the main decryption code.

Here's the latest version where I've added the option to extract the
creation time from the signatures.

.marko

On Thu, 2014-08-07 at 12:15 +0200, Marko Tiikkaja wrote:

On 8/6/14 2:46 PM, I wrote:

Attached is a patch to add support for PGP signatures in encrypted
messages into pgcrypto.

Here's v2 of the patch. I've changed the info-extracting code to not
look for signatures beyond the data, which also meant that it had to
parse one-pass signatures (which it didn't do before). This matches the
behaviour of the main decryption code.

There is a compiler warning:

pgp-sig.c: In function ‘pgp_parse_onepass_signature’:
pgp-sig.c:715:8: error: variable ‘last’ set but not used [-Werror=unused-but-set-variable]
uint8 last;
^

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Aug 6, 2014 at 2:46 PM, Marko Tiikkaja <marko@joh.to> wrote:

Hi hackers,

Attached is a patch to add support for PGP signatures in encrypted messages
into pgcrypto.

I noticed Heikki wanted to check if there is any interested for the
patches in the current commitfest.

Yes, our company Trustly are very interested in the two PGP additions
to pgcrypto.

We currently use these patches in production in a separate database,
but if they would be part of standard postgres, we wouldn't need to
run the application using the functionality in a separate database
server, which would simplify things a lot.

Without these patches, there is no way to deal with PGP signatures.
Since signatures is a crucial component of OpenPGP, the existing
encryption/decryption features are useful, but not nearly as useful as
if you also have the capabilities to generate and verify PGP
signatures.

We use the PGP functionality in a system called BankAPI, which is open
source and available here: https://github.com/trustly/bankapi

Also, in the documentation, it has already been acknowledged the lack
of signing is a current limitation:
"F.25.3.9. Limitations of PGP Code
No support for signing. That also means that it is not checked whether
the encryption subkey belongs to the master key."

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 09/03/2014 02:51 PM, Joel Jacobson wrote:

On Wed, Aug 6, 2014 at 2:46 PM, Marko Tiikkaja <marko@joh.to> wrote:

Hi hackers,

Attached is a patch to add support for PGP signatures in encrypted messages
into pgcrypto.

I noticed Heikki wanted to check if there is any interested for the
patches in the current commitfest.

Yes, our company Trustly are very interested in the two PGP additions
to pgcrypto.

Cool. Please sign up as a reviewer then, so that we can get these
patches reviewed and committed.

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Aug 15, 2014 at 12:55 AM, Marko Tiikkaja <marko@joh.to> wrote:

Hi,

On 8/7/14 12:15 PM, I wrote:

Here's v2 of the patch. I've changed the info-extracting code to not
look for signatures beyond the data, which also meant that it had to
parse one-pass signatures (which it didn't do before). This matches the
behaviour of the main decryption code.

Here's the latest version where I've added the option to extract the
creation time from the signatures.

There is trivial sgml patch application conflict due to a grammar
correction in 05258761bf12a64befc9caec1947b254cdeb74c5

I wanted to start simple so I have a file which is signed, but not
encrypted. I can't figure out what to do with it. All of the functions
seem to require that it also be encrypted. I tried providing an empty
password for pgp_sym_signatures but it didn't work.

Is there a way to deal with this situation?

Thanks

Jeff

On 2014-09-03 9:36 PM, Jeff Janes wrote:

I wanted to start simple so I have a file which is signed, but not
encrypted. I can't figure out what to do with it. All of the functions
seem to require that it also be encrypted. I tried providing an empty
password for pgp_sym_signatures but it didn't work.

Right. This patch only adds support for signing data when encrypting it
at the same time. There's no support for detached signatures, nor is
there support for anything other than signatures of encrypted data. I
should have been more clear on that in my initial email. :-(

.marko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Sep 3, 2014 at 12:43 PM, Marko Tiikkaja <marko@joh.to> wrote:

On 2014-09-03 9:36 PM, Jeff Janes wrote:

I wanted to start simple so I have a file which is signed, but not
encrypted. I can't figure out what to do with it. All of the functions
seem to require that it also be encrypted. I tried providing an empty
password for pgp_sym_signatures but it didn't work.

Right. This patch only adds support for signing data when encrypting it
at the same time. There's no support for detached signatures, nor is there
support for anything other than signatures of encrypted data. I should
have been more clear on that in my initial email. :-(

OK, thanks. How hard do you think it would to allow NULL (or empty
string?) passwords to gpg_sym_signatures and gpg_sym_decrypt_verify to
accommodate this?

I think docs section F.25.3 needs to be re-titled and expanded to reflect
signatures as well as encryption, and an explanation added about signatures
only being processed on encrypted data if that restriction can't be removed.

I've switched to using a signed plus symmetrically encrypted message for
testing.

One surprising thing so far is that the 3rd argument to
gpg_sym_decrypt_verify must be dearmored. I thought it would detect and
dearmor automatically.

Once I wrap it in dearmor, I get the ERROR: No signature matching the key
id present in the message

The public key block I am giving it is for the keyid that is reported
by pgp_sym_signatures, so I don't know what the problem might be.

When I get more time, I'll look at your examples from the regression tests
to see if I can figure it out.

Thanks,

Jeff

On 2014-09-03 10:33 PM, Jeff Janes wrote:

On Wed, Sep 3, 2014 at 12:43 PM, Marko Tiikkaja <marko@joh.to> wrote:

Right. This patch only adds support for signing data when encrypting it
at the same time. There's no support for detached signatures, nor is there
support for anything other than signatures of encrypted data. I should
have been more clear on that in my initial email. :-(

OK, thanks. How hard do you think it would to allow NULL (or empty
string?) passwords to gpg_sym_signatures and gpg_sym_decrypt_verify to
accommodate this?

To sign without encrypting? I think those should really be a different
set of functions altogether. But this patch is already humongous (on my
standards, at least), so I think that should be done separately.

I think docs section F.25.3 needs to be re-titled and expanded to reflect
signatures as well as encryption, and an explanation added about signatures
only being processed on encrypted data if that restriction can't be removed.

I don't have an objection to that. I fully acknowledge that the
documentation doesn't state the limitations of signing should this patch
be applied.

I've switched to using a signed plus symmetrically encrypted message for
testing.

One surprising thing so far is that the 3rd argument to
gpg_sym_decrypt_verify must be dearmored. I thought it would detect and
dearmor automatically.

I can't see that as an improvement to be honest.

Once I wrap it in dearmor, I get the ERROR: No signature matching the key
id present in the message

The public key block I am giving it is for the keyid that is reported
by pgp_sym_signatures, so I don't know what the problem might be.

Have you tried with the debug=1 option? (It's undocumented, but it was
like that before this patch and I didn't touch it).

When I get more time, I'll look at your examples from the regression tests
to see if I can figure it out.

Thanks! I'm happy to help if you run into any trouble!

.marko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Marko, et al,

This is a review of the pgcrypto PGP signatures patch:
/messages/by-id/53EDBCF0.9070205@joh.to

There hasn't been any discussion, at least that I've been able to find.

Contents & Purpose
==================
This patch add functions to create, verify and extract infromation
from OpenPGP signatures. Previously pgcrypto only peformed
PGP encrypt/decrypt, not sign/verify. This is a painful limitation
since a very common use-case for OpenPGP is the signature-part,
where two parties want to verify messages originate from each other,
and not only encrypt the messages.

Included in the patch are updated regression test cases and documentation.

Initial Run
===========
The patch applies cleanly to HEAD after changing a single line in the patch:
< ! Giving this function a secret key will produce an error.
---

! Giving this function a secret key will produce a error.

This grammar fix was already fixed in 05258761bf12a64befc9caec1947b254cdeb74c5,
and therefore caused the conflict.

The 144 regression tests all pass successfully against the new patch.

Conclusion
==========
Since I'm using these functions in the BankAPI project,
https://github.com/trustly/bankapi, I have tested them
by actually using them in production, in addition to the provided
regression tests, which is a good sign they are working not just
in theory.

+1 for committer review after the changes suggested by Jeff Janes and
Thomas Munro.

On Fri, Aug 15, 2014 at 9:55 AM, Marko Tiikkaja <marko@joh.to> wrote:

Hi,

On 8/7/14 12:15 PM, I wrote:

Here's v2 of the patch. I've changed the info-extracting code to not
look for signatures beyond the data, which also meant that it had to
parse one-pass signatures (which it didn't do before). This matches the
behaviour of the main decryption code.

Here's the latest version where I've added the option to extract the
creation time from the signatures.

.marko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

Any thoughts, comments appreciated.

.marko

On 2014-09-05 1:38 PM, I wrote:

3) I've changed the code to use ntohl() and pg_time_t as per Thomas'
comments.

sig->creation_time = ntohl(*((uint32_t *) creation_time));

This is probably a horrible idea due to strict aliasing rules and
alignment, though. I think I'll just hide the bit shifts behind a
function instead.

.marko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Sep 3, 2014 at 2:13 PM, Marko Tiikkaja <marko@joh.to> wrote:

On 2014-09-03 10:33 PM, Jeff Janes wrote:

On Wed, Sep 3, 2014 at 12:43 PM, Marko Tiikkaja <marko@joh.to> wrote:

Right. This patch only adds support for signing data when encrypting it
at the same time. There's no support for detached signatures, nor is
there
support for anything other than signatures of encrypted data. I should
have been more clear on that in my initial email. :-(

OK, thanks. How hard do you think it would to allow NULL (or empty

string?) passwords to gpg_sym_signatures and gpg_sym_decrypt_verify to
accommodate this?

To sign without encrypting?

To verify signatures of things that are not encrypted. I'm not really
interested in storing private keys in PostgreSQL, just things that can be
done with public keys. (But I will make a dummy private key for testing if
I get that far.)

...

Once I wrap it in dearmor, I get the ERROR: No signature matching the key

id present in the message

The public key block I am giving it is for the keyid that is reported
by pgp_sym_signatures, so I don't know what the problem might be.

Have you tried with the debug=1 option? (It's undocumented, but it was
like that before this patch and I didn't touch it).

I have now, but it didn't produce any output for this situation. I have
two theories for the problem. My test signed message was signed with a
keyring that had a signing subkey, so it was signed with that, not with the
master. Maybe it doesn't like that. Also, I created the signed message in
gpg, then imported it to PostgreSQL, and maybe it doesn't like that.

I've never used the pgp functions of pgcrypto before, so I decided to take
a step back and try some of the functions that predate the proposed patch.
And I can't get them to work well, either.

If I use pgp_sym_encrypt to encrypt a message with AES, then
pgp_sym_decrypt will decrypt, and so will gpg command line tool. But if I
use gpg to encrypt a message, pgp_sym_decrypt will not decrypt it.

select pgp_sym_decrypt(dearmor('-----BEGIN PGP MESSAGE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Password: foobar

jA0EBwMCqywsAv/hXJ7D0j8BWsD+9H7DY4KhrIIw2oV/6tBueVQ28+VDjBw9rGiy
3JRPmyXNN4wRTZXIyTVzK3LylWLomD9pQkao4hrQwSs=
=02RI
-----END PGP MESSAGE-----
'),'foobar','debug=1');
NOTICE: dbg: parse_literal_data: data type=b
ERROR: Not text data

So I don't know if I am doing something wrong, or if the PostgreSQL
implementation of pgp is just not interoperable with other implementations.
That makes it hard to test the new features if I can't make the old ones
work.

The two messages I am working with are:

Created: echo -n 'a message'|gpg -c --armor --cipher-algo AES -
-----BEGIN PGP MESSAGE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Password: foobar

jA0EBwMCqywsAv/hXJ7D0j8BWsD+9H7DY4KhrIIw2oV/6tBueVQ28+VDjBw9rGiy
3JRPmyXNN4wRTZXIyTVzK3LylWLomD9pQkao4hrQwSs=
=02RI
-----END PGP MESSAGE-----

and

Created: select armor(pgp_sym_encrypt('a message','foobar'));
-----BEGIN PGP MESSAGE-----

ww0EBwMCYzgp4dU3zCJ30joBViH28prwc9jIHhzUyXt31omiHao7NeOuLhCR0/uhAB6GRfYAXWVa
x+FTsW27F46/W7dlRjxCuzcu
=jQGZ
-----END PGP MESSAGE-----

Cheers,

Jeff

On 2014-09-07 19:28, Jeff Janes wrote:

On Wed, Sep 3, 2014 at 2:13 PM, Marko Tiikkaja <marko@joh.to> wrote:

To sign without encrypting?

To verify signatures of things that are not encrypted. I'm not really
interested in storing private keys in PostgreSQL, just things that can be
done with public keys. (But I will make a dummy private key for testing if
I get that far.)

Right. That functionality might be useful, but I think it should be a
separate patch completely. (And I doubt I have any interest in
implementing it).

Once I wrap it in dearmor, I get the ERROR: No signature matching the key

id present in the message

The public key block I am giving it is for the keyid that is reported
by pgp_sym_signatures, so I don't know what the problem might be.

Have you tried with the debug=1 option? (It's undocumented, but it was
like that before this patch and I didn't touch it).

I have now, but it didn't produce any output for this situation. I have
two theories for the problem. My test signed message was signed with a
keyring that had a signing subkey, so it was signed with that, not with the
master. Maybe it doesn't like that.

Yeah, this patch only supports signing and verifying signatures with
main keys.

Also, I created the signed message in
gpg, then imported it to PostgreSQL, and maybe it doesn't like that.

That should not be a problem. I used gpg extensively when testing the
patch.

I've never used the pgp functions of pgcrypto before, so I decided to take
a step back and try some of the functions that predate the proposed patch.
And I can't get them to work well, either.

If I use pgp_sym_encrypt to encrypt a message with AES, then
pgp_sym_decrypt will decrypt, and so will gpg command line tool. But if I
use gpg to encrypt a message, pgp_sym_decrypt will not decrypt it.

select pgp_sym_decrypt(dearmor('-----BEGIN PGP MESSAGE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Password: foobar

jA0EBwMCqywsAv/hXJ7D0j8BWsD+9H7DY4KhrIIw2oV/6tBueVQ28+VDjBw9rGiy
3JRPmyXNN4wRTZXIyTVzK3LylWLomD9pQkao4hrQwSs=
=02RI
-----END PGP MESSAGE-----
'),'foobar','debug=1');
NOTICE: dbg: parse_literal_data: data type=b
ERROR: Not text data

So I don't know if I am doing something wrong, or if the PostgreSQL
implementation of pgp is just not interoperable with other implementations.
That makes it hard to test the new features if I can't make the old ones
work.

The NOTICE here says what's wrong: the message has been marked to
contain binary data, not text. You should be able to decrypt it with
pgp_sym_decrypt_bytea() (and you can use convert_from() to get a text
value out).

.marko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sun, Sep 7, 2014 at 10:36 AM, Marko Tiikkaja <marko@joh.to> wrote:

On 2014-09-07 19:28, Jeff Janes wrote:

select pgp_sym_decrypt(dearmor('-----BEGIN PGP MESSAGE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Password: foobar

jA0EBwMCqywsAv/hXJ7D0j8BWsD+9H7DY4KhrIIw2oV/6tBueVQ28+VDjBw9rGiy
3JRPmyXNN4wRTZXIyTVzK3LylWLomD9pQkao4hrQwSs=
=02RI
-----END PGP MESSAGE-----
'),'foobar','debug=1');
NOTICE: dbg: parse_literal_data: data type=b
ERROR: Not text data

So I don't know if I am doing something wrong, or if the PostgreSQL
implementation of pgp is just not interoperable with other
implementations.
That makes it hard to test the new features if I can't make the old ones
work.

The NOTICE here says what's wrong: the message has been marked to contain
binary data, not text. You should be able to decrypt it with
pgp_sym_decrypt_bytea() (and you can use convert_from() to get a text value
out).

OK, thanks. That is obvious in retrospect. I'll put it on my todo list to
try to clean up some of documentation and error messages to make it more
obvious to the naive user, but that is not part of this patch.

One problem I've run into now is that if I try to sign a message
with pgp_pub_encrypt_sign but give it the public, not private, key as the
3rd argument, it generates this message:

ERROR: Cannot decrypt with public key

Should be 'sign', not 'decrypt'.

Similarly for verification:

ERROR: Refusing to encrypt with secret key

'encrypt' should be 'verify signature'.

Cheers,

Jeff

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

On 2014-09-08 7:30 PM, Jeff Janes wrote:

On Fri, Sep 5, 2014 at 4:38 AM, Marko Tiikkaja <marko@joh.to> wrote:

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.

You got that right, yes.

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.

Yeah, that seems reasonable, I guess. I'm kind of torn between the two
behaviours to be honest. But perhaps it would make sense to change the
previous behaviour (i.e. go back to way this patch was earlier) and
document that somewhere.

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.

Interesting. Thanks! I'll have a look.

.marko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi Jeff,

On 9/8/14 7:30 PM, Jeff Janes wrote:

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.

Thanks. There seemed to have been a small confusion about the ownership
of ctx->sig_digest_ctx. I've fixed that now and the test case you
provided doesn't appear to be leaking memory anymore. I also added some
other missing free calls to pgp_free().

I've attached a patch with this problem fixed, in case you still want to
keep testing (all your work so far very much appreciated, btw!) The
attached also fixes the ntohl() problem I pointed out in my previous
patch, and now AFAIK there aren't any outstanding technical issues.

However..

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.

I haven't updated the patch yet because I don't want to waste my time
going back and forth until we have a consensus, but I think I prefer
Jeff's suggestion here to make the _decrypt() functions ignore
signatures. Does anyone else want to voice their opinion?

.marko

Marko Tiikkaja wrote:

On 9/8/14 7:30 PM, Jeff Janes wrote:

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.

I haven't updated the patch yet because I don't want to waste my
time going back and forth until we have a consensus, but I think I
prefer Jeff's suggestion here to make the _decrypt() functions
ignore signatures. Does anyone else want to voice their opinion?

+1 for ignoring sigs. If somebody want to check sigs, that's a
separate step. Maybe we could have an optional boolean flag, default
false, to enable checking sigs, but that seems material for a future
patch.

That said, I do wonder if it's a behavior change with security
implications: if somebody is relying on the current behavior of throwing
an error when sigs don't match, they wouldn't be thrilled to hear that
their security checks now fail to detect a problem because we don't
verify signatures when decrypting. In other words, is this established
practice already? If not, it's okay; otherwise, hmm.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers