Re: pgcrypto: PGP signatures

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