Thread: pgcrypto: PGP signatures

Hi hackers,

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

Currently, the list of limitations is the following:

     - It only knows how to generate one signature per message.  I don't
see that as a problem.
     - If a message has been signed with multiple keys which have the
same keyid as the one specified to verify the message, an error is
returned.  Naively, it seems that we should try all of them and return
"OK" if even one of them matches, but that seems icky.
     - Only RSA signatures are supported.  It wouldn't be too hard for
someone familiar with DSA to add it in, but I'm not volunteering to do
it.  Personally I think supporting RSA is better than no support at all.

As per usual, I'll also add this to the upcoming commitfest.  Any
feedback appreciated before that, of course.


.marko

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

Hi Marko,

I took a quick look at your patch at
http://www.postgresql.org/message-id/53EDBCF0.9070205@joh.to (sorry I
didn't reply directly as I didn't have the message).  It applies
cleanly, builds, and the tests pass.  I will hopefully have more to
say after I've poked at it and understood it better, but in the
meantime a couple of trivial things I spotted:

In pgp-pgsql.c, function pgp_sym_decrypt_verify_bytea:

+       if (PG_NARGS() > 3)
+               PG_FREE_IF_COPY(arg, 3);
+       if (PG_NARGS() > 4)
+               PG_FREE_IF_COPY(arg, 4);

I think the first 'arg' should be 'psw'.

I think the same mistake appears in pgp_sym_decrypt_verify_text.

+       if (!sig->onepass)
+       {
+               time_t t;
+
+               isnull[3] = false;
+               /* unsigned big endian */
+               t = sig->creation_time[0] << 24;
+               t += sig->creation_time[1] << 16;
+               t += sig->creation_time[2] << 8;
+               t += sig->creation_time[3];
+               values[3] = time_t_to_timestamptz(t);
+       }

Should t be of type pg_time_t rather than time_t?  Would it be better
if PGP_Signature's member creation_time were of type uint32_t and you
could use ntohl(sig->creation_time) instead of the bitshifting?

In pgp.h:

+#define PGP_MAX_KEY                                    (256/8)
+#define PGP_MAX_BLOCK                          (256/8)
+#define PGP_MAX_DIGEST                         (512/8)
+#define PGP_MAX_DIGEST_ASN1_PREFIX     20
+#define PGP_S2K_SALT                           8

The PGP_MAX_DIGEST_ASN1_PREFIX line is new but the others just have
whitespace changes, and I gather the done thing is not to reformat
existing lines like that to distract from the changes in a patch.

(Just curious, why do you use while (1) for an infinite loop in
extract_signatures, but for (;;) in pullf_discard?  It looks like the
latter is much more common in the source tree.)

Best regards,

Thomas Munro

On 8/22/14, 2:57 AM, Thomas Munro wrote:
> I took a quick look at your patch at
> http://www.postgresql.org/message-id/53EDBCF0.9070205@joh.to (sorry I
> didn't reply directly as I didn't have the message).  It applies
> cleanly, builds, and the tests pass.  I will hopefully have more to
> say after I've poked at it and understood it better, but in the
> meantime a couple of trivial things I spotted:

Thanks!

> In pgp-pgsql.c, function pgp_sym_decrypt_verify_bytea:
>
> I think the first 'arg' should be 'psw'.
>
> I think the same mistake appears in pgp_sym_decrypt_verify_text.

Yeah, these look like copypaste-os.  Will fix.

> Should t be of type pg_time_t rather than time_t?  Would it be better
> if PGP_Signature's member creation_time were of type uint32_t and you
> could use ntohl(sig->creation_time) instead of the bitshifting?

I tried to make the code look like the existing code in 
init_litdata_packet().  I don't oppose to writing it this way, but I 
think we should change both instances if so (or perhaps move the code 
into a new function).

> In pgp.h:
>
> +#define PGP_MAX_KEY                                    (256/8)
> +#define PGP_MAX_BLOCK                          (256/8)
> +#define PGP_MAX_DIGEST                         (512/8)
> +#define PGP_MAX_DIGEST_ASN1_PREFIX     20
> +#define PGP_S2K_SALT                           8
>
> The PGP_MAX_DIGEST_ASN1_PREFIX line is new but the others just have
> whitespace changes, and I gather the done thing is not to reformat
> existing lines like that to distract from the changes in a patch.

Right.  Sorry about that.  I can revert the whitespace fixes.

> (Just curious, why do you use while (1) for an infinite loop in
> extract_signatures, but for (;;) in pullf_discard?  It looks like the
> latter is much more common in the source tree.)

In the postgres source tree?  Yeah.  But while(1) is all over pgcrypto, 
so I've tried to make the new code match that.  If there are any 
instances of "for (;;)" in the patch, those must be because of me typing 
without thinking.  I could search-and-replace those to "while (1)" to 
make it consistent.


.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;       ^

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."

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

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

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

Marko, et al,

This is a review of the pgcrypto PGP signatures patch:
http://www.postgresql.org/message-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
>

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

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

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

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

(I have't read the patch, or even earlier correspondence in this
thread, so I apologise for just jumping in.)

At 2014-09-12 12:50:45 -0300, alvherre@2ndquadrant.com wrote:
>
> +1 for ignoring sigs.  If somebody want to check sigs, that's a
> separate step. 

For what it's worth, although it seems logical to split up cryptographic
primitives like this, I think it's widely recognised these days to have
contributed to plenty of bad crypto implementations. These seems to be
general trend of moving towards higher-level interfaces that require
fewer decisions and can be relied upon do the Right Thing.

I don't like the idea of ignoring signature verification errors any more
than I would like "if somebody wants to check the HMAC before decypting,
that's a separate step".

Of course, all that is an aside. If the function ever threw an error on
signature verification failures, I would strongly object to changing it
to ignore such errors for exactly the reasons you mention already.

-- Abhijit

On 9/12/14, 8:22 PM, Abhijit Menon-Sen wrote:
> (I have't read the patch, or even earlier correspondence in this
> thread, so I apologise for just jumping in.)
>
> At 2014-09-12 12:50:45 -0300, alvherre@2ndquadrant.com wrote:
>>
>> +1 for ignoring sigs.  If somebody want to check sigs, that's a
>> separate step.
>
> For what it's worth, although it seems logical to split up cryptographic
> primitives like this, I think it's widely recognised these days to have
> contributed to plenty of bad crypto implementations. These seems to be
> general trend of moving towards higher-level interfaces that require
> fewer decisions and can be relied upon do the Right Thing.
>
> I don't like the idea of ignoring signature verification errors any more
> than I would like "if somebody wants to check the HMAC before decypting,
> that's a separate step".
>
> Of course, all that is an aside. If the function ever threw an error on
> signature verification failures, I would strongly object to changing it
> to ignore such errors for exactly the reasons you mention already.

I'm not sure we're talking about the same thing.  Currently, we throw an
error if *any* signature was present, valid or otherwise.  The "decrypt
only" functions don't have enough information to verify the validity of
the signature, so we must either ignore the signatures or throw an error
in their presence.

The only downside of ignoring signatures here as far as I can tell is a
scenario where you're sending messages to someone, and they accept your
signed messages.  You might get the impression that the receiving party
is actually validating the signature, but I guess that's trivial to
test, and relying on such unwritten contracts is a bit suspicious anyway
when it comes to cryptography.

I've changed the patch back to ignore signatures when not using the
decrypt_verify() functions in the attached.


.marko

At 2014-09-15 13:37:48 +0200, marko@joh.to wrote:
>
> I'm not sure we're talking about the same thing.

No, we weren't. I was under the impression that the signatures
could be validated. Sorry for the noise.

-- Abhijit

I looked at this briefly, and was surprised that there is no support for 
signing a message without encrypting it. Is that intentional? Instead of 
adding a function to encrypt and sign a message, I would have expected 
this to just add a new function for signing, and you could then pass it 
an already-encrypted blob, or plaintext.

- Heikki

On 10/2/14 1:47 PM, Heikki Linnakangas wrote:
> I looked at this briefly, and was surprised that there is no support for
> signing a message without encrypting it. Is that intentional? Instead of
> adding a function to encrypt and sign a message, I would have expected
> this to just add a new function for signing, and you could then pass it
> an already-encrypted blob, or plaintext.

Yes, that's intentional.  The signatures are part of the encrypted data 
here, so you can't look at a message and determine who sent it.

There was brief discussion about this upthread (though no one probably 
added any links to those discussions into the commit fest app), and I 
still think that both types of signing would probably be valuable.  But 
this patch is already quite big, and I really have no desire to work on 
this "sign anything" functionality.  The pieces are there, though, so if 
someone wants to do it, I don't see why they couldn't.


.marko

Hi,

On 10/17/14, 9:56 PM, Jeff Janes wrote:
> This patch needs a rebase now that the armor header patch has been
> committed.

Thanks.  Will fix that shortly.

I'm guessing there's no need to bump the pgcrypto version to 1.3, since 
there hasn't been a release with the 1.2 version?


.marko

<div dir="ltr"><br /><div class="gmail_extra"><br /><div class="gmail_quote">On Mon, Oct 20, 2014 at 6:27 AM, Marko
Tiikkaja<span dir="ltr"><<a href="mailto:marko@joh.to" target="_blank">marko@joh.to</a>></span> wrote:<br
/><blockquoteclass="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span
class=""></span>I'mguessing there's no need to bump the pgcrypto version to 1.3, since there hasn't been a release with
the1.2 version?<span class="HOEnZb"></span><br /></blockquote></div>Yep. One version bump by major release is fine for
acontrib module.<br />-- <br />Michael<br /></div></div> 

Hi,

Here's the rebased patch -- as promised -- in a v7.


.marko

On 10/27/14, 11:57 PM, Jeff Janes wrote:
> Using the same script as for the memory leak, I am getting seg faults using
> this patch.
>
> gdb backtrace:
>
> #0  pfree (pointer=0x0) at mcxt.c:749
> #1  0x00007f617d7973e7 in pgp_sym_decrypt_verify_text (fcinfo=0x1d7f1f0) at
> pgp-pgsql.c:1047

Huh.  That's weird.  I seem to have somehow rebased an older version of
the patch from my git history.  This is the copy-paste mistake Thomas
(IIRC) pointed out and I fixed weeks, if not months, ago.

I have no idea what happened so I just threw away the git history and
rebased v6 of the patch on top of master.  I've also verified that the
script runs and does not leak memory.  The resulting patch, v8, attached.

My apologies for wasting your time, but thanks for testing!


.marko

Hi,

I discovered a problem with the lack of MDC handling in the signature
info extraction code, so I've fixed that and added a test message.  v9 here.


.marko

On Wed, Nov 12, 2014 at 7:05 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
> On Sat, Nov 1, 2014 at 7:52 AM, Marko Tiikkaja <marko@joh.to> wrote:
>>
>> Hi,
>>
>> I discovered a problem with the lack of MDC handling in the signature info
>> extraction code, so I've fixed that and added a test message.  v9 here.
>>
>>
>>
>
> Hi Marko,
>
> I get a segfault when the length of the message is exactly 16308 bytes, see
> attached perl script.
>
> I can't get a backtrace, for some reason it acts as if there were no debug
> symbols despite that I built with them.  I've not seen that before.
>
> I get this whether or not the bug 11905 patch is applied, so the problem
> seems to be related but different.
This patch status was "Ready for committer" but it still has visibly
some bugs, and has not been updated in a while as pointed out by Jeff.
So I am switching it as "Returned with feedback".
Regards,
-- 
Michael