Re: pgcrypto: PGP signatures

From: Marko Tiikkaja <marko(at)joh(dot)to>
To: Jeff Janes <jeff(dot)janes(at)gmail(dot)com>
Cc: Joel Jacobson <joel(at)trustly(dot)com>, Thomas Munro <munro(at)ip9(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pgcrypto: PGP signatures
Date: 2014-09-12 14:53:06
Message-ID: 541308D2.3000402@joh.to
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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

Attachment Content-Type Size
pgcrypto_sigs.v5.patch text/plain 151.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2014-09-12 15:07:33 Re: pgbench throttling latency limit
Previous Message Fujii Masao 2014-09-12 14:44:27 Re: ALTER SYSTEM RESET?