pgcrypto: add s2k-count
pgcrypto supports s2k-mode for key-stretching during symmetric
encryption, and even defaults to s2k-mode=3, which means configurable
iterations. But it doesn't support s2k-count to actually set those
iterations to be anything other than the default. If you are
interested in key-stretching, the default is not going to cut it.
(You could argue that pgp's s2k doesn't cut it either even at the max,
but at least we should offer the maximum that the pgp spec makes
available.)
This patch implements s2k-count as an option to pgp_sym_encrypt.
Demo (note the password is intentionally wrong in the last character):
select pgp_sym_decrypt(
pgp_sym_encrypt('foobar','acf86729b6b0289f4d1909db8c1aaf0c','s2k-mode=3'),
'acf86729b6b0289f4d1909db8c1aaf0d');
ERROR: Wrong key or corrupt data
Time: 1.606 ms
select pgp_sym_decrypt(
pgp_sym_encrypt('foobar','acf86729b6b0289f4d1909db8c1aaf0c','s2k-mode=3,s2k-count=65000000'),
'acf86729b6b0289f4d1909db8c1aaf0d');
ERROR: Wrong key or corrupt data
Time: 615.720 ms
I did not bump the extension version. I realized the migration file
would be empty, as there no change to SQL-level functionality (the new
s2k-count is parsed out of a string down in the C code). Since only
one version of contrib extensions binary object files are installed in
any given postgres installation, people using the newer binary gets
the feature even if they have not updated the extension version. So I
don't know if it makes sense to bump the version if people inherently
get the feature anyway.
Cheers,
Jeff
Attachments:
pgp_s2k_count_v1.patchtext/x-patch; charset=US-ASCII; name=pgp_s2k_count_v1.patchDownload
diff --git a/contrib/pgcrypto/expected/pgp-encrypt.out b/contrib/pgcrypto/expected/pgp-encrypt.out
new file mode 100644
index b35de79..2bf999f
*** a/contrib/pgcrypto/expected/pgp-encrypt.out
--- b/contrib/pgcrypto/expected/pgp-encrypt.out
*************** select pgp_sym_decrypt(
*** 103,108 ****
--- 103,126 ----
Secret.
(1 row)
+ -- s2k count change
+ select pgp_sym_decrypt(
+ pgp_sym_encrypt('Secret.', 'key', 's2k-count=1024'),
+ 'key', 'expect-s2k-count=1024');
+ pgp_sym_decrypt
+ -----------------
+ Secret.
+ (1 row)
+
+ select pgp_sym_decrypt(
+ pgp_sym_encrypt('Secret.', 'key', 's2k-count=65000000'),
+ 'key', 'expect-s2k-count=65000000'); -- rounded up
+ NOTICE: pgp_decrypt: unexpected s2k_count: expected 65000000 got 65011712
+ pgp_sym_decrypt
+ -----------------
+ Secret.
+ (1 row)
+
-- s2k digest change
select pgp_sym_decrypt(
pgp_sym_encrypt('Secret.', 'key', 's2k-digest-algo=md5'),
diff --git a/contrib/pgcrypto/pgp-decrypt.c b/contrib/pgcrypto/pgp-decrypt.c
new file mode 100644
index 5c69745..3d16033
*** a/contrib/pgcrypto/pgp-decrypt.c
--- b/contrib/pgcrypto/pgp-decrypt.c
*************** parse_symenc_sesskey(PGP_Context *ctx, P
*** 643,648 ****
--- 643,649 ----
if (res < 0)
return res;
ctx->s2k_mode = ctx->s2k.mode;
+ ctx->s2k_count = iter_to_count(ctx->s2k.iter);
ctx->s2k_digest_algo = ctx->s2k.digest_algo;
/*
diff --git a/contrib/pgcrypto/pgp-encrypt.c b/contrib/pgcrypto/pgp-encrypt.c
new file mode 100644
index 2320c75..c9148fd
*** a/contrib/pgcrypto/pgp-encrypt.c
--- b/contrib/pgcrypto/pgp-encrypt.c
*************** init_s2k_key(PGP_Context *ctx)
*** 567,573 ****
if (ctx->s2k_cipher_algo < 0)
ctx->s2k_cipher_algo = ctx->cipher_algo;
! res = pgp_s2k_fill(&ctx->s2k, ctx->s2k_mode, ctx->s2k_digest_algo);
if (res < 0)
return res;
--- 567,573 ----
if (ctx->s2k_cipher_algo < 0)
ctx->s2k_cipher_algo = ctx->cipher_algo;
! res = pgp_s2k_fill(&ctx->s2k, ctx->s2k_mode, ctx->s2k_digest_algo, ctx->s2k_count);
if (res < 0)
return res;
diff --git a/contrib/pgcrypto/pgp-pgsql.c b/contrib/pgcrypto/pgp-pgsql.c
new file mode 100644
index 1842985..1b55bc9
*** a/contrib/pgcrypto/pgp-pgsql.c
--- b/contrib/pgcrypto/pgp-pgsql.c
*************** struct debug_expect
*** 181,186 ****
--- 181,187 ----
int expect;
int cipher_algo;
int s2k_mode;
+ int s2k_count;
int s2k_cipher_algo;
int s2k_digest_algo;
int compress_algo;
*************** fill_expect(struct debug_expect * ex, in
*** 196,201 ****
--- 197,203 ----
ex->expect = 0;
ex->cipher_algo = -1;
ex->s2k_mode = -1;
+ ex->s2k_count= -1;
ex->s2k_cipher_algo = -1;
ex->s2k_digest_algo = -1;
ex->compress_algo = -1;
*************** check_expect(PGP_Context *ctx, struct de
*** 218,223 ****
--- 220,226 ----
{
EX_CHECK(cipher_algo);
EX_CHECK(s2k_mode);
+ EX_CHECK(s2k_count);
EX_CHECK(s2k_digest_algo);
EX_CHECK(use_sess_key);
if (ctx->use_sess_key)
*************** set_arg(PGP_Context *ctx, char *key, cha
*** 247,252 ****
--- 250,257 ----
res = pgp_set_sess_key(ctx, atoi(val));
else if (strcmp(key, "s2k-mode") == 0)
res = pgp_set_s2k_mode(ctx, atoi(val));
+ else if (strcmp(key, "s2k-count") == 0)
+ res = pgp_set_s2k_count(ctx, atoi(val));
else if (strcmp(key, "s2k-digest-algo") == 0)
res = pgp_set_s2k_digest_algo(ctx, val);
else if (strcmp(key, "s2k-cipher-algo") == 0)
*************** set_arg(PGP_Context *ctx, char *key, cha
*** 286,291 ****
--- 291,301 ----
ex->expect = 1;
ex->s2k_mode = atoi(val);
}
+ else if (ex != NULL && strcmp(key, "expect-s2k-count") == 0)
+ {
+ ex->expect = 1;
+ ex->s2k_count = atoi(val);
+ }
else if (ex != NULL && strcmp(key, "expect-s2k-digest-algo") == 0)
{
ex->expect = 1;
diff --git a/contrib/pgcrypto/pgp-s2k.c b/contrib/pgcrypto/pgp-s2k.c
new file mode 100644
index 193dd95..baa8e88
*** a/contrib/pgcrypto/pgp-s2k.c
--- b/contrib/pgcrypto/pgp-s2k.c
*************** calc_s2k_iter_salted(PGP_S2K *s2k, PX_MD
*** 137,143 ****
count;
cval = s2k->iter;
! count = ((unsigned) 16 + (cval & 15)) << ((cval >> 4) + 6);
md_rlen = px_md_result_size(md);
--- 137,143 ----
count;
cval = s2k->iter;
! count = iter_to_count(cval);
md_rlen = px_md_result_size(md);
*************** calc_s2k_iter_salted(PGP_S2K *s2k, PX_MD
*** 200,215 ****
* Too small: weak
* Too big: slow
* gpg defaults to 96 => 65536 iters
! * let it float a bit: 96 + 32 => 262144 iters
*/
static int
! decide_count(unsigned rand_byte)
{
! return 96 + (rand_byte & 0x1F);
}
int
! pgp_s2k_fill(PGP_S2K *s2k, int mode, int digest_algo)
{
int res = 0;
uint8 tmp;
--- 200,223 ----
* Too small: weak
* Too big: slow
* gpg defaults to 96 => 65536 iters
! * For our default let it float a bit: 96 + 32 => 262144 iters
! * Otherwise, find the smallest iteration which provides
! * at least the specified count.
*/
static int
! decide_count(unsigned rand_byte,int count)
{
! int iter;
! if (count == -1)
! return 96 + (rand_byte & 0x1F);
! for (iter=0; iter<=255; iter++)
! if (iter_to_count(iter) >= count)
! return iter;
! return 255;
}
int
! pgp_s2k_fill(PGP_S2K *s2k, int mode, int digest_algo, int count)
{
int res = 0;
uint8 tmp;
*************** pgp_s2k_fill(PGP_S2K *s2k, int mode, int
*** 231,237 ****
res = px_get_pseudo_random_bytes(&tmp, 1);
if (res < 0)
break;
! s2k->iter = decide_count(tmp);
break;
default:
res = PXE_PGP_BAD_S2K_MODE;
--- 239,245 ----
res = px_get_pseudo_random_bytes(&tmp, 1);
if (res < 0)
break;
! s2k->iter = decide_count(tmp,count);
break;
default:
res = PXE_PGP_BAD_S2K_MODE;
diff --git a/contrib/pgcrypto/pgp.c b/contrib/pgcrypto/pgp.c
new file mode 100644
index 03fe48f..f0e016c
*** a/contrib/pgcrypto/pgp.c
--- b/contrib/pgcrypto/pgp.c
***************
*** 40,45 ****
--- 40,46 ----
static int def_cipher_algo = PGP_SYM_AES_128;
static int def_s2k_cipher_algo = -1;
static int def_s2k_mode = PGP_S2K_ISALTED;
+ static int def_s2k_count = -1;
static int def_s2k_digest_algo = PGP_DIGEST_SHA1;
static int def_compress_algo = PGP_COMPR_NONE;
static int def_compress_level = 6;
*************** pgp_init(PGP_Context **ctx_p)
*** 206,211 ****
--- 207,213 ----
ctx->cipher_algo = def_cipher_algo;
ctx->s2k_cipher_algo = def_s2k_cipher_algo;
ctx->s2k_mode = def_s2k_mode;
+ ctx->s2k_count = def_s2k_count;
ctx->s2k_digest_algo = def_s2k_digest_algo;
ctx->compress_algo = def_compress_algo;
ctx->compress_level = def_compress_level;
*************** pgp_set_s2k_mode(PGP_Context *ctx, int m
*** 270,275 ****
--- 272,288 ----
}
int
+ pgp_set_s2k_count(PGP_Context *ctx, int count)
+ {
+ if (ctx->s2k_mode == PGP_S2K_ISALTED && count >= 1024 && count <= 65011712)
+ {
+ ctx->s2k_count = count;
+ return PXE_OK;
+ }
+ return PXE_ARGUMENT_ERROR;
+ }
+
+ int
pgp_set_compress_algo(PGP_Context *ctx, int algo)
{
switch (algo)
diff --git a/contrib/pgcrypto/pgp.h b/contrib/pgcrypto/pgp.h
new file mode 100644
index 62b8517..c3e86e3
*** a/contrib/pgcrypto/pgp.h
--- b/contrib/pgcrypto/pgp.h
***************
*** 34,39 ****
--- 34,41 ----
#include "mbuf.h"
#include "px.h"
+ #define iter_to_count(cval) (((unsigned) 16 + (cval & 15)) << ((cval >> 4) + 6))
+
enum PGP_S2K_TYPE
{
PGP_S2K_SIMPLE = 0,
*************** struct PGP_Context
*** 138,143 ****
--- 140,146 ----
*/
PGP_S2K s2k;
int s2k_mode;
+ int s2k_count;
int s2k_digest_algo;
int s2k_cipher_algo;
int cipher_algo;
*************** const char *pgp_get_cipher_name(int code
*** 243,248 ****
--- 246,252 ----
int pgp_set_cipher_algo(PGP_Context *ctx, const char *name);
int pgp_set_s2k_mode(PGP_Context *ctx, int type);
+ int pgp_set_s2k_count(PGP_Context *ctx, int count);
int pgp_set_s2k_cipher_algo(PGP_Context *ctx, const char *name);
int pgp_set_s2k_digest_algo(PGP_Context *ctx, const char *name);
int pgp_set_convert_crlf(PGP_Context *ctx, int doit);
*************** int pgp_load_cipher(int c, PX_Cipher *
*** 267,273 ****
int pgp_get_cipher_key_size(int c);
int pgp_get_cipher_block_size(int c);
! int pgp_s2k_fill(PGP_S2K *s2k, int mode, int digest_algo);
int pgp_s2k_read(PullFilter *src, PGP_S2K *s2k);
int pgp_s2k_process(PGP_S2K *s2k, int cipher, const uint8 *key, int klen);
--- 271,277 ----
int pgp_get_cipher_key_size(int c);
int pgp_get_cipher_block_size(int c);
! int pgp_s2k_fill(PGP_S2K *s2k, int mode, int digest_algo, int count);
int pgp_s2k_read(PullFilter *src, PGP_S2K *s2k);
int pgp_s2k_process(PGP_S2K *s2k, int cipher, const uint8 *key, int klen);
diff --git a/contrib/pgcrypto/sql/pgp-encrypt.sql b/contrib/pgcrypto/sql/pgp-encrypt.sql
new file mode 100644
index a9ac0b9..1dd575d
*** a/contrib/pgcrypto/sql/pgp-encrypt.sql
--- b/contrib/pgcrypto/sql/pgp-encrypt.sql
*************** select pgp_sym_decrypt(
*** 55,60 ****
--- 55,68 ----
pgp_sym_encrypt('Secret.', 'key', 's2k-mode=3'),
'key', 'expect-s2k-mode=3');
+ -- s2k count change
+ select pgp_sym_decrypt(
+ pgp_sym_encrypt('Secret.', 'key', 's2k-count=1024'),
+ 'key', 'expect-s2k-count=1024');
+ select pgp_sym_decrypt(
+ pgp_sym_encrypt('Secret.', 'key', 's2k-count=65000000'),
+ 'key', 'expect-s2k-count=65000000'); -- rounded up
+
-- s2k digest change
select pgp_sym_decrypt(
pgp_sym_encrypt('Secret.', 'key', 's2k-digest-algo=md5'),
diff --git a/doc/src/sgml/pgcrypto.sgml b/doc/src/sgml/pgcrypto.sgml
new file mode 100644
index bfcbe02..21a884b
*** a/doc/src/sgml/pgcrypto.sgml
--- b/doc/src/sgml/pgcrypto.sgml
*************** Applies to: pgp_sym_encrypt
*** 859,864 ****
--- 859,877 ----
</sect4>
<sect4>
+ <title>s2k-count</title>
+
+ <para>
+ The number of iterations of the S2K algorithm to use. It must
+ be a value between 1024 and 65011712, inclusive.
+ </para>
+ <literallayout>
+ Default: A random value bewteen 65536 and 253952
+ Applies to: pgp_sym_encrypt, only with s2k-mode=3
+ </literallayout>
+ </sect4>
+
+ <sect4>
<title>s2k-digest-algo</title>
<para>
On Wed, Feb 10, 2016 at 12:44 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
pgcrypto supports s2k-mode for key-stretching during symmetric
encryption, and even defaults to s2k-mode=3, which means configurable
iterations. But it doesn't support s2k-count to actually set those
iterations to be anything other than the default. If you are
interested in key-stretching, the default is not going to cut it.
(You could argue that pgp's s2k doesn't cut it either even at the max,
but at least we should offer the maximum that the pgp spec makes
available.)This patch implements s2k-count as an option to pgp_sym_encrypt.
Demo (note the password is intentionally wrong in the last character):
select pgp_sym_decrypt(
pgp_sym_encrypt('foobar','acf86729b6b0289f4d1909db8c1aaf0c','s2k-mode=3'),
'acf86729b6b0289f4d1909db8c1aaf0d');
ERROR: Wrong key or corrupt data
Time: 1.606 msselect pgp_sym_decrypt(
pgp_sym_encrypt('foobar','acf86729b6b0289f4d1909db8c1aaf0c','s2k-mode=3,s2k-count=65000000'),
'acf86729b6b0289f4d1909db8c1aaf0d');
ERROR: Wrong key or corrupt data
Time: 615.720 msI did not bump the extension version. I realized the migration file
would be empty, as there no change to SQL-level functionality (the new
s2k-count is parsed out of a string down in the C code). Since only
one version of contrib extensions binary object files are installed in
any given postgres installation, people using the newer binary gets
the feature even if they have not updated the extension version. So I
don't know if it makes sense to bump the version if people inherently
get the feature anyway.
There's zero reason to bump the extension version if the SQL interface
hasn't changed.
(I have no opinion on the underlying patch.)
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
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, Feb 12, 2016 at 2:46 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Feb 10, 2016 at 12:44 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
I did not bump the extension version. I realized the migration file
would be empty, as there no change to SQL-level functionality (the new
s2k-count is parsed out of a string down in the C code). Since only
one version of contrib extensions binary object files are installed in
any given postgres installation, people using the newer binary gets
the feature even if they have not updated the extension version. So I
don't know if it makes sense to bump the version if people inherently
get the feature anyway.There's zero reason to bump the extension version if the SQL interface
hasn't changed.(I have no opinion on the underlying patch.)
+1.
--
Michael--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Jeff Janes wrote:
pgcrypto supports s2k-mode for key-stretching during symmetric
encryption, and even defaults to s2k-mode=3, which means configurable
iterations. But it doesn't support s2k-count to actually set those
iterations to be anything other than the default. If you are
interested in key-stretching, the default is not going to cut it.
Talking about deep rabbit holes ...
I gave this a look. I adjusted it here and there for project style and
general cleanliness (something that could be applied to pgcrypto much
more generally) and eventually arrived at trying to figure out how is
the s2k count actually used by the underlying algorithm. I arrived at
the function calc_s2k_iter_salted which is where it is actually used to
encrypt things. But that function is completely devoid of comments and
not completely trivial. At this point I cannot for the life of me
determine whether that function should use the one-byte format specified
by the relevant RFC (4880) or the decoded, human-understandable number
of iterations.
I would love to be able to read gnupg's code to figure out what it is
that they do, but the structure of their code is even more impenetrable
than pgcrypto's. Perhaps it would be easier to measure the time it
takes to run some s2k operations ...
I CCed Marko here. Hopefully he can chime in on whether this patch is
correct.
Anyway, assuming that the iteration count was already being used
correctly, then as far as I'm concerned we're ready to go. The attached
patch is what I would commit.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
pgp_s2k_count_v2.patchtext/x-diff; charset=us-asciiDownload
diff --git a/contrib/pgcrypto/expected/pgp-encrypt.out b/contrib/pgcrypto/expected/pgp-encrypt.out
index b35de79..8fc558c 100644
--- a/contrib/pgcrypto/expected/pgp-encrypt.out
+++ b/contrib/pgcrypto/expected/pgp-encrypt.out
@@ -103,6 +103,25 @@ select pgp_sym_decrypt(
Secret.
(1 row)
+-- s2k count change
+select pgp_sym_decrypt(
+ pgp_sym_encrypt('Secret.', 'key', 's2k-count=1024'),
+ 'key', 'expect-s2k-count=1024');
+ pgp_sym_decrypt
+-----------------
+ Secret.
+(1 row)
+
+-- s2k_count rounds up
+select pgp_sym_decrypt(
+ pgp_sym_encrypt('Secret.', 'key', 's2k-count=65000000'),
+ 'key', 'expect-s2k-count=65000000');
+NOTICE: pgp_decrypt: unexpected s2k_count: expected 65000000 got 65011712
+ pgp_sym_decrypt
+-----------------
+ Secret.
+(1 row)
+
-- s2k digest change
select pgp_sym_decrypt(
pgp_sym_encrypt('Secret.', 'key', 's2k-digest-algo=md5'),
diff --git a/contrib/pgcrypto/pgp-decrypt.c b/contrib/pgcrypto/pgp-decrypt.c
index 5c69745..37f374a 100644
--- a/contrib/pgcrypto/pgp-decrypt.c
+++ b/contrib/pgcrypto/pgp-decrypt.c
@@ -643,6 +643,7 @@ parse_symenc_sesskey(PGP_Context *ctx, PullFilter *src)
if (res < 0)
return res;
ctx->s2k_mode = ctx->s2k.mode;
+ ctx->s2k_count = s2k_iter_to_count(ctx->s2k.iter);
ctx->s2k_digest_algo = ctx->s2k.digest_algo;
/*
diff --git a/contrib/pgcrypto/pgp-encrypt.c b/contrib/pgcrypto/pgp-encrypt.c
index 2320c75..c9148fd 100644
--- a/contrib/pgcrypto/pgp-encrypt.c
+++ b/contrib/pgcrypto/pgp-encrypt.c
@@ -567,7 +567,7 @@ init_s2k_key(PGP_Context *ctx)
if (ctx->s2k_cipher_algo < 0)
ctx->s2k_cipher_algo = ctx->cipher_algo;
- res = pgp_s2k_fill(&ctx->s2k, ctx->s2k_mode, ctx->s2k_digest_algo);
+ res = pgp_s2k_fill(&ctx->s2k, ctx->s2k_mode, ctx->s2k_digest_algo, ctx->s2k_count);
if (res < 0)
return res;
diff --git a/contrib/pgcrypto/pgp-pgsql.c b/contrib/pgcrypto/pgp-pgsql.c
index 1842985..1f65b66 100644
--- a/contrib/pgcrypto/pgp-pgsql.c
+++ b/contrib/pgcrypto/pgp-pgsql.c
@@ -181,6 +181,7 @@ struct debug_expect
int expect;
int cipher_algo;
int s2k_mode;
+ int s2k_count;
int s2k_cipher_algo;
int s2k_digest_algo;
int compress_algo;
@@ -196,6 +197,7 @@ fill_expect(struct debug_expect * ex, int text_mode)
ex->expect = 0;
ex->cipher_algo = -1;
ex->s2k_mode = -1;
+ ex->s2k_count = -1;
ex->s2k_cipher_algo = -1;
ex->s2k_digest_algo = -1;
ex->compress_algo = -1;
@@ -218,6 +220,7 @@ check_expect(PGP_Context *ctx, struct debug_expect * ex)
{
EX_CHECK(cipher_algo);
EX_CHECK(s2k_mode);
+ EX_CHECK(s2k_count);
EX_CHECK(s2k_digest_algo);
EX_CHECK(use_sess_key);
if (ctx->use_sess_key)
@@ -247,6 +250,8 @@ set_arg(PGP_Context *ctx, char *key, char *val,
res = pgp_set_sess_key(ctx, atoi(val));
else if (strcmp(key, "s2k-mode") == 0)
res = pgp_set_s2k_mode(ctx, atoi(val));
+ else if (strcmp(key, "s2k-count") == 0)
+ res = pgp_set_s2k_count(ctx, atoi(val));
else if (strcmp(key, "s2k-digest-algo") == 0)
res = pgp_set_s2k_digest_algo(ctx, val);
else if (strcmp(key, "s2k-cipher-algo") == 0)
@@ -286,6 +291,11 @@ set_arg(PGP_Context *ctx, char *key, char *val,
ex->expect = 1;
ex->s2k_mode = atoi(val);
}
+ else if (ex != NULL && strcmp(key, "expect-s2k-count") == 0)
+ {
+ ex->expect = 1;
+ ex->s2k_count = atoi(val);
+ }
else if (ex != NULL && strcmp(key, "expect-s2k-digest-algo") == 0)
{
ex->expect = 1;
diff --git a/contrib/pgcrypto/pgp-s2k.c b/contrib/pgcrypto/pgp-s2k.c
index 193dd95..92cf369 100644
--- a/contrib/pgcrypto/pgp-s2k.c
+++ b/contrib/pgcrypto/pgp-s2k.c
@@ -132,12 +132,10 @@ calc_s2k_iter_salted(PGP_S2K *s2k, PX_MD *md, const uint8 *key,
unsigned preload = 0;
unsigned remain,
c,
- cval,
curcnt,
count;
- cval = s2k->iter;
- count = ((unsigned) 16 + (cval & 15)) << ((cval >> 4) + 6);
+ count = s2k_iter_to_count(s2k->iter);
md_rlen = px_md_result_size(md);
@@ -195,21 +193,34 @@ calc_s2k_iter_salted(PGP_S2K *s2k, PX_MD *md, const uint8 *key,
}
/*
- * Decide S2K_ISALTED iteration count
+ * Decide PGP_S2K_ISALTED iteration count (in OpenPGP one-byte representation)
*
* Too small: weak
* Too big: slow
* gpg defaults to 96 => 65536 iters
- * let it float a bit: 96 + 32 => 262144 iters
+ *
+ * For our default (count=-1) we let it float a bit: 96 + 32 => between 65536
+ * and 262144 iterations
+ *
+ * Otherwise, find the smallest iteration number which provides at least the
+ * specified count.
*/
static int
-decide_count(unsigned rand_byte)
+decide_s2k_iter(unsigned rand_byte, int count)
{
- return 96 + (rand_byte & 0x1F);
+ int iter;
+
+ if (count == -1)
+ return 96 + (rand_byte & 0x1F);
+ /* this is a bit brute-force, but should be quick enough */
+ for (iter = 0; iter <= 255; iter++)
+ if (s2k_iter_to_count(iter) >= count)
+ return iter;
+ return 255;
}
int
-pgp_s2k_fill(PGP_S2K *s2k, int mode, int digest_algo)
+pgp_s2k_fill(PGP_S2K *s2k, int mode, int digest_algo, int count)
{
int res = 0;
uint8 tmp;
@@ -219,19 +230,19 @@ pgp_s2k_fill(PGP_S2K *s2k, int mode, int digest_algo)
switch (s2k->mode)
{
- case 0:
+ case PGP_S2K_SIMPLE:
break;
- case 1:
+ case PGP_S2K_SALTED:
res = px_get_pseudo_random_bytes(s2k->salt, PGP_S2K_SALT);
break;
- case 3:
+ case PGP_S2K_ISALTED:
res = px_get_pseudo_random_bytes(s2k->salt, PGP_S2K_SALT);
if (res < 0)
break;
res = px_get_pseudo_random_bytes(&tmp, 1);
if (res < 0)
break;
- s2k->iter = decide_count(tmp);
+ s2k->iter = decide_s2k_iter(tmp, count);
break;
default:
res = PXE_PGP_BAD_S2K_MODE;
diff --git a/contrib/pgcrypto/pgp.c b/contrib/pgcrypto/pgp.c
index 03fe48f..0800fc3 100644
--- a/contrib/pgcrypto/pgp.c
+++ b/contrib/pgcrypto/pgp.c
@@ -40,6 +40,7 @@
static int def_cipher_algo = PGP_SYM_AES_128;
static int def_s2k_cipher_algo = -1;
static int def_s2k_mode = PGP_S2K_ISALTED;
+static int def_s2k_count = -1;
static int def_s2k_digest_algo = PGP_DIGEST_SHA1;
static int def_compress_algo = PGP_COMPR_NONE;
static int def_compress_level = 6;
@@ -206,6 +207,7 @@ pgp_init(PGP_Context **ctx_p)
ctx->cipher_algo = def_cipher_algo;
ctx->s2k_cipher_algo = def_s2k_cipher_algo;
ctx->s2k_mode = def_s2k_mode;
+ ctx->s2k_count = def_s2k_count;
ctx->s2k_digest_algo = def_s2k_digest_algo;
ctx->compress_algo = def_compress_algo;
ctx->compress_level = def_compress_level;
@@ -270,6 +272,17 @@ pgp_set_s2k_mode(PGP_Context *ctx, int mode)
}
int
+pgp_set_s2k_count(PGP_Context *ctx, int count)
+{
+ if (ctx->s2k_mode == PGP_S2K_ISALTED && count >= 1024 && count <= 65011712)
+ {
+ ctx->s2k_count = count;
+ return PXE_OK;
+ }
+ return PXE_ARGUMENT_ERROR;
+}
+
+int
pgp_set_compress_algo(PGP_Context *ctx, int algo)
{
switch (algo)
diff --git a/contrib/pgcrypto/pgp.h b/contrib/pgcrypto/pgp.h
index 62b8517..c0a7c90 100644
--- a/contrib/pgcrypto/pgp.h
+++ b/contrib/pgcrypto/pgp.h
@@ -138,6 +138,7 @@ struct PGP_Context
*/
PGP_S2K s2k;
int s2k_mode;
+ int s2k_count;
int s2k_digest_algo;
int s2k_cipher_algo;
int cipher_algo;
@@ -171,6 +172,10 @@ struct PGP_Context
unsigned sess_key_len;
};
+/* from RFC 4880 3.7.1.3 */
+#define s2k_iter_to_count(cval) \
+ (((unsigned) 16 + (cval & 15)) << ((cval >> 4) + 6))
+
struct PGP_MPI
{
uint8 *data;
@@ -243,6 +248,7 @@ const char *pgp_get_cipher_name(int code);
int pgp_set_cipher_algo(PGP_Context *ctx, const char *name);
int pgp_set_s2k_mode(PGP_Context *ctx, int type);
+int pgp_set_s2k_count(PGP_Context *ctx, int count);
int pgp_set_s2k_cipher_algo(PGP_Context *ctx, const char *name);
int pgp_set_s2k_digest_algo(PGP_Context *ctx, const char *name);
int pgp_set_convert_crlf(PGP_Context *ctx, int doit);
@@ -267,7 +273,7 @@ int pgp_load_cipher(int c, PX_Cipher **res);
int pgp_get_cipher_key_size(int c);
int pgp_get_cipher_block_size(int c);
-int pgp_s2k_fill(PGP_S2K *s2k, int mode, int digest_algo);
+int pgp_s2k_fill(PGP_S2K *s2k, int mode, int digest_algo, int count);
int pgp_s2k_read(PullFilter *src, PGP_S2K *s2k);
int pgp_s2k_process(PGP_S2K *s2k, int cipher, const uint8 *key, int klen);
diff --git a/contrib/pgcrypto/sql/pgp-encrypt.sql b/contrib/pgcrypto/sql/pgp-encrypt.sql
index a9ac0b9..ed9d2c8 100644
--- a/contrib/pgcrypto/sql/pgp-encrypt.sql
+++ b/contrib/pgcrypto/sql/pgp-encrypt.sql
@@ -55,6 +55,15 @@ select pgp_sym_decrypt(
pgp_sym_encrypt('Secret.', 'key', 's2k-mode=3'),
'key', 'expect-s2k-mode=3');
+-- s2k count change
+select pgp_sym_decrypt(
+ pgp_sym_encrypt('Secret.', 'key', 's2k-count=1024'),
+ 'key', 'expect-s2k-count=1024');
+-- s2k_count rounds up
+select pgp_sym_decrypt(
+ pgp_sym_encrypt('Secret.', 'key', 's2k-count=65000000'),
+ 'key', 'expect-s2k-count=65000000');
+
-- s2k digest change
select pgp_sym_decrypt(
pgp_sym_encrypt('Secret.', 'key', 's2k-digest-algo=md5'),
diff --git a/doc/src/sgml/pgcrypto.sgml b/doc/src/sgml/pgcrypto.sgml
index bfcbe02..21a884b 100644
--- a/doc/src/sgml/pgcrypto.sgml
+++ b/doc/src/sgml/pgcrypto.sgml
@@ -859,6 +859,19 @@ Applies to: pgp_sym_encrypt
</sect4>
<sect4>
+ <title>s2k-count</title>
+
+ <para>
+ The number of iterations of the S2K algorithm to use. It must
+ be a value between 1024 and 65011712, inclusive.
+ </para>
+<literallayout>
+Default: A random value bewteen 65536 and 253952
+Applies to: pgp_sym_encrypt, only with s2k-mode=3
+</literallayout>
+ </sect4>
+
+ <sect4>
<title>s2k-digest-algo</title>
<para>
Alvaro Herrera wrote:
Anyway, assuming that the iteration count was already being used
correctly, then as far as I'm concerned we're ready to go. The attached
patch is what I would commit.
I read some more (gnupg code as well as our own) and applied some more
tweaks, and pushed.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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
On Tue, Mar 8, 2016 at 4:09 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
Jeff Janes wrote:
pgcrypto supports s2k-mode for key-stretching during symmetric
encryption, and even defaults to s2k-mode=3, which means configurable
iterations. But it doesn't support s2k-count to actually set those
iterations to be anything other than the default. If you are
interested in key-stretching, the default is not going to cut it.Talking about deep rabbit holes ...
I gave this a look. I adjusted it here and there for project style and
general cleanliness (something that could be applied to pgcrypto much
more generally) and eventually arrived at trying to figure out how is
the s2k count actually used by the underlying algorithm. I arrived at
the function calc_s2k_iter_salted which is where it is actually used to
encrypt things. But that function is completely devoid of comments and
not completely trivial. At this point I cannot for the life of me
determine whether that function should use the one-byte format specified
by the relevant RFC (4880) or the decoded, human-understandable number
of iterations.
Thanks for taking this up.
Yeah, I find that pretty impenetrable too. I just treated it as a
black box, I changed how the number passed into it gets set, but not
the meaning of that number. Initially I had the user set the one-byte
format directly because that was much simpler, but before submitting
the patch I changed it to take the human-readable value and do the
conversion to the one byte format, because the gpg command-line tool
takes the number of iterations, not the one byte format, as the input
for its own s2k-count setting.
I would love to be able to read gnupg's code to figure out what it is
that they do, but the structure of their code is even more impenetrable
than pgcrypto's. Perhaps it would be easier to measure the time it
takes to run some s2k operations ...
The timings are about the same between the patched pgcrypto and gpg
when using the same settings for s2k-count. Also, if I encrypt with
gpg with a certain setting, pgcrypto properly detects that iteration
count and uses it (if it didn't get it correct, it would be unable to
decrypt). And vice versa.
Do we know why the default for pgcrypto is to use a stochastic number
of iterations? I don't see how that can enhance security, as the
number of iterations actually done is not a secret.
And I see that you committed it now, so thanks for that too.
Cheers,
Jeff
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Jeff Janes wrote:
On Tue, Mar 8, 2016 at 4:09 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
Yeah, I find that pretty impenetrable too. I just treated it as a
black box, I changed how the number passed into it gets set, but not
the meaning of that number. Initially I had the user set the one-byte
format directly because that was much simpler, but before submitting
the patch I changed it to take the human-readable value and do the
conversion to the one byte format, because the gpg command-line tool
takes the number of iterations, not the one byte format, as the input
for its own s2k-count setting.
Funny -- I partially edited the patch to use the one-byte number instead
too, because that seemed more reasonable, but eventually (looking at
gnupg) decided not to. And deleted the email on which I explained that,
without sending.
I would love to be able to read gnupg's code to figure out what it is
that they do, but the structure of their code is even more impenetrable
than pgcrypto's. Perhaps it would be easier to measure the time it
takes to run some s2k operations ...The timings are about the same between the patched pgcrypto and gpg
when using the same settings for s2k-count. Also, if I encrypt with
gpg with a certain setting, pgcrypto properly detects that iteration
count and uses it (if it didn't get it correct, it would be unable to
decrypt). And vice versa.
OK, it seems we're good then.
Do we know why the default for pgcrypto is to use a stochastic number
of iterations? I don't see how that can enhance security, as the
number of iterations actually done is not a secret.
Nope, unless Marko has some input there. I find it baffling too.
And I see that you committed it now, so thanks for that too.
You're welcome, thanks for the patch.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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