Control: tags 739424 + patch

On Wed 2014-10-01 14:01:45 -0400, Marc Lehmann wrote:
> On Wed, Oct 01, 2014 at 07:01:27PM +0200, Werner Koch <w...@gnupg.org> wrote:
>> and 3072 DSA (as per standard).  If you hack the system or use a bug to
>> create way larger keys you are on your own.
>
> Werner, please don't accuse people of "hacking systems" or exploiting bugs
> - people who used documented features in gnupg in the past, or features in
> other implementations run into this issue just as well.

Can we please dial back the vitriol on this discussion?  it's clear to
me that everyone involved wants to support people using good crypto,
let's not beat up our allies over the details, even where we disagree
with them.

> It doesn't do you well to reduce these people to those having done
> something weird. Some might have legitimate reasons, too - the most
> obvious reason is that long RSA keys are expected to last longer against
> quantum computer attacks (whether this will turn out to be true, or not,
> or unneeded is not something we can know today).
> 
> And regarding elliptic curves - you are surely aware that it is mostly
> gnupg that kept elliptic curves back in the openpgp arena.

Actually, afaict GnuPG is one of the few places where work on elliptic
curves in OpenPGP has actually happened.  It does need to be deployed
more widely, though, and getting more of the 2.1.0 betas out to a
broader audience would help (see recent discussion on gnupg-devel and
pkg-gnupg-maint about progress on that front).

>> In this case I do not want to help the race to more and more stupid key
>> properties.  If there is a problem with an 8k RSA key I am willing to
>> help, but somewhere we have to stop.

I agree with this sentiment, but we don't all agree on what the right
place to stop is.  And we've broken interoperability with old keys for
multiple users right now.

I think everyone probably agrees that trying to support > 16Kib RSA keys
is too much (16Kib RSA keys are slow to use even on modern hardware),
and there are existing interoperability concerns for people with keys
below (and close to) that cutoff, which happens to also be close to the
current best-estimate of an adversary's ~256-bit work-factor.

I don't think that anyone in the discussion actually believes they have
an adversary capable of 2^256 compute power either now or in the near
future, so presumably the disagreement is about whether the current
best-estimate might be wrong for some classes of attacker
(e.g. privately-held mathematical or hardware advances).

fwiw, recent versions of gpg also work fine with the public part of the
pre-existing larger RSA keys, although it is of course slower to do
public key operations with this sort of material.  So the concern is
just for people who hold large RSA secret keys.

If i understand it correctly, i think what changed in 1.4.16 was RSA
blinding and resistance against the acoustic attacks.  Presumably this
change ended up using more secure memory per key than had been
previously used (though i don't think i understand the exact details).
This is a critical fix, so it's clearly not something we want to revert.

I don't think i've said anything controversial so far :)

-----

GnuPG has maintained backward-compatibility (even bug-compatibility)
across much less serious concerns, so i think we should try to support
pre-existing keys as well, even if they were generated by what we see as
an earlier bug.  If GnuPG doesn't want to support this bug-compatibility
upstream, i think debian can carry a patch, though i would definitely prefer
to avoid diverging from upstream where possible.

So i'm attaching two different patches that we could use to address this
issue.  If either of them is acceptable upstream, i'll use that one.


The first variant adds a compile-time flag: ./configure
--enable-large-rsa-keys, which defaults to off.  If it is present, then
we allocate double the secmem (64KiB instead of 32KiB) and permit up to
8Kib RSA keys when doing --gen-key --batch (the upper limit on
interactive key generation remains at 4Kib).


The second variant adds a runtime flag: gpg --enable-large-rsa , which
defaults to off.  If the flag is present at runtime, then the secmem is
doubled and the max RSA size for --gen-key --batch goes from 4Kib to
8Kib.  Because the config file is parsed before secure memory is
allocated, this flag has to exist directly on the command line for
people using larger keys.


Werner, do you have a preference for which one to apply?

I think i prefer the first variant, because (a) there are already too
many command line options, and (b) a gpg option that only works on the
command line and not in the config file breaks the expected calling
conventions for gpg, and i think will be harder for most users to
deploy.  If there was a way to get the enable-large-rsa to work in
gpg.conf without parsing the configfile while under elevated privileges
(in a suid deployment -- are those still common?), i might change my
mind.

If i don't hear any objections or counterproposals, i'll probably apply
the first variant (build-time options) and set debian/rules to
./configure --enable-large-rsa-keys

Regards,

        --dkg

From 17c59a94bea138eaf7224bbfe7119bc80cf45bc0 Mon Sep 17 00:00:00 2001
From: Daniel Kahn Gillmor <d...@fifthhorseman.net>
Date: Thu, 2 Oct 2014 01:06:40 -0400
Subject: [PATCH] add build-time option for larger RSA key support

* configure.ac: added --enable-large-rsa-keys option
* g10/gpg.c: use SECMEM_BUFFER_SIZE instead of static
* g10/keygen.c: limit batch mode keys to RSA_KEY_MAX_SIZE

Some older implementations built and used RSA keys up to 16Kib, but
the larger keys now fail when used by more recent GnuPG, due to secure
memory limitations.

Building with ./configure --enable-large-rsa-keys will make gpg
capable of working with those secret keys, as well as being able to
generate keys up to 8Kib when used in --batch mode.

Debian-bug-id: 739424
---
 configure.ac | 19 +++++++++++++++++++
 g10/gpg.c    |  2 +-
 g10/keygen.c |  4 ++--
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index ae63a4a..d4c5265 100644
--- a/configure.ac
+++ b/configure.ac
@@ -158,6 +158,7 @@ use_exec=yes
 card_support=yes
 agent_support=yes
 disable_keyserver_path=no
+large_rsa_support=no
 
 AC_ARG_ENABLE(minimal,
    AC_HELP_STRING([--enable-minimal],[build the smallest gpg binary possible]),
@@ -177,6 +178,24 @@ AC_ARG_ENABLE(minimal,
    agent_support=no)
 
 
+AC_MSG_CHECKING([whether larger RSA keys should be supported])
+AC_ARG_ENABLE(large-rsa-keys,
+              AC_HELP_STRING([--enable-large-rsa-keys],
+                             [enable large RSA keys]),
+              large_rsa_support=$enableval, large_rsa_support=no)
+AC_MSG_RESULT($large_rsa_support)
+
+if test "$large_rsa_support" = yes ; then
+  AC_DEFINE(SECMEM_BUFFER_SIZE,65536,[Size of secure memory buffer])
+  AC_DEFINE(RSA_KEY_MAX_SIZE,8192,
+            [Maximum size of batch-generated RSA keys])
+else
+  AC_DEFINE(SECMEM_BUFFER_SIZE,32768,[Size of secure memory buffer])
+  AC_DEFINE(RSA_KEY_MAX_SIZE,4096,
+            [Maximum size of batch-generated RSA keys])
+fi
+
+
 AC_MSG_CHECKING([whether OpenPGP card support is requested])
 AC_ARG_ENABLE(card-support,
               AC_HELP_STRING([--disable-card-support],
diff --git a/g10/gpg.c b/g10/gpg.c
index 1b0a364..acd49db 100644
--- a/g10/gpg.c
+++ b/g10/gpg.c
@@ -1995,7 +1995,7 @@ main (int argc, char **argv )
     }
 #endif
     /* initialize the secure memory. */
-    got_secmem=secmem_init( 32768 );
+    got_secmem=secmem_init( SECMEM_BUFFER_SIZE );
     maybe_setuid = 0;
     /* Okay, we are now working under our real uid */
 
diff --git a/g10/keygen.c b/g10/keygen.c
index 84f852f..21c6c0b 100644
--- a/g10/keygen.c
+++ b/g10/keygen.c
@@ -1260,8 +1260,8 @@ gen_rsa(int algo, unsigned nbits, KBNODE pub_root, KBNODE sec_root, DEK *dek,
 	nbits = 2048;
 	log_info(_("keysize invalid; using %u bits\n"), nbits );
     }
-    else if (nbits > 4096) {
-        nbits = 4096;
+    else if (nbits > RSA_KEY_MAX_SIZE) {
+        nbits = RSA_KEY_MAX_SIZE;
         log_info(_("keysize invalid; using %u bits\n"), nbits );
     }
 
-- 
2.1.0

From 278f9f5c35196cdc1b7e07e7ff68ac7d636826ee Mon Sep 17 00:00:00 2001
From: Daniel Kahn Gillmor <d...@fifthhorseman.net>
Date: Thu, 2 Oct 2014 10:58:54 -0400
Subject: [PATCH] add runtime support for larger RSA keys with
 --enable-large-rsa

* g10/options.h: add opt.flags.large_rsa
* g10/keygen.c: adjust max RSA size based on opt.flags.large_rsa
* g10/gpg.c: add --enable-large-rsa, bound to opt.flags.large_rsa, and
  adjust secmem size
* doc/gpg.texi: document --enable-large-rsa

--

Some older implementations built and used RSA keys up to 16Kib, but
the larger secret keys now fail when used by more recent GnuPG, due to
secure memory limitations.

The new gpg option --enable-large-rsa will let gpg work with those
secret keys, as well as being able to generate keys up to 8Kib when
used in --batch mode.

Debian-bug-id: 739424
---
 doc/gpg.texi  | 13 +++++++++++++
 g10/gpg.c     | 27 ++++++++++++++++++++++++++-
 g10/keygen.c  |  5 +++--
 g10/options.h |  1 +
 4 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/doc/gpg.texi b/doc/gpg.texi
index ded69ce..fbc952d 100644
--- a/doc/gpg.texi
+++ b/doc/gpg.texi
@@ -1104,6 +1104,19 @@ the opposite meaning. The options are:
   validation. This option is only meaningful if pka-lookups is set.
 @end table
 
+@item --enable-large-rsa
+@itemx --disable-large-rsa
+@opindex enable-large-rsa
+@opindex disable-large-rsa
+Enable the creation and use of larger RSA secret keys than is
+generally recommended.  This requires allocation of larger amounts of
+secure memory, so may not be advisable on tightly-constrained systems.
+With this flag and --batch, --gen-key will also allow creation of RSA
+keys up to 8192 bits.
+
+Because secure memory is allocated before the configuration file is
+parsed, this option needs to be supplied on the command line.
+
 @item --enable-dsa2
 @itemx --disable-dsa2
 @opindex enable-dsa2
diff --git a/g10/gpg.c b/g10/gpg.c
index 1b0a364..ea956de 100644
--- a/g10/gpg.c
+++ b/g10/gpg.c
@@ -372,6 +372,8 @@ enum cmd_and_opt_values
     oAutoKeyLocate,
     oNoAutoKeyLocate,
     oAllowMultisigVerification,
+    oEnableLargeRSA,
+    oDisableLargeRSA,
     oEnableDSA2,
     oDisableDSA2,
     oAllowMultipleMessages,
@@ -719,6 +721,8 @@ static ARGPARSE_OPTS opts[] = {
     { oDebugCCIDDriver, "debug-ccid-driver", 0, "@"},
 #endif
     { oAllowMultisigVerification, "allow-multisig-verification", 0, "@"},
+    { oEnableLargeRSA, "enable-large-rsa", 0, "@"},
+    { oDisableLargeRSA, "disable-large-rsa", 0, "@"},
     { oEnableDSA2, "enable-dsa2", 0, "@"},
     { oDisableDSA2, "disable-dsa2", 0, "@"},
     { oAllowMultipleMessages, "allow-multiple-messages", 0, "@"},
@@ -1948,6 +1952,10 @@ main (int argc, char **argv )
 	    set_homedir ( pargs.r.ret_str );
 	else if( pargs.r_opt == oNoPermissionWarn )
 	    opt.no_perm_warn=1;
+	else if( pargs.r_opt == oEnableLargeRSA )
+	    opt.flags.large_rsa=1;
+	else if( pargs.r_opt == oDisableLargeRSA )
+	    opt.flags.large_rsa=0;
 	else if (pargs.r_opt == oStrict )
 	  {
 	    opt.strict=1;
@@ -1995,7 +2003,7 @@ main (int argc, char **argv )
     }
 #endif
     /* initialize the secure memory. */
-    got_secmem=secmem_init( 32768 );
+    got_secmem=secmem_init( opt.flags.large_rsa ? 65536 : 32768 );
     maybe_setuid = 0;
     /* Okay, we are now working under our real uid */
 
@@ -2851,6 +2859,23 @@ main (int argc, char **argv )
 	    release_akl();
 	    break;
 
+	  case oEnableLargeRSA:
+            if (configname)
+              log_error(_("%s:%d: --enable-large-rsa must be "
+                          "supplied on the command line\n"),
+                        configname,configlineno);
+            else
+                opt.flags.large_rsa=1;
+            break;
+	  case oDisableLargeRSA:
+            if (configname)
+              log_error(_("%s:%d: --disable-large-rsa must be "
+                          "supplied on the command line\n"),
+                        configname,configlineno);
+            else
+                opt.flags.large_rsa=0;
+            break;
+
 	  case oEnableDSA2: opt.flags.dsa2=1; break;
 	  case oDisableDSA2: opt.flags.dsa2=0; break;
 
diff --git a/g10/keygen.c b/g10/keygen.c
index 84f852f..9020908 100644
--- a/g10/keygen.c
+++ b/g10/keygen.c
@@ -1253,6 +1253,7 @@ gen_rsa(int algo, unsigned nbits, KBNODE pub_root, KBNODE sec_root, DEK *dek,
     PKT_public_key *pk;
     MPI skey[6];
     MPI *factors;
+    const unsigned maxsize = (opt.flags.large_rsa ? 8192 : 4096);
 
     assert( is_RSA(algo) );
 
@@ -1260,8 +1261,8 @@ gen_rsa(int algo, unsigned nbits, KBNODE pub_root, KBNODE sec_root, DEK *dek,
 	nbits = 2048;
 	log_info(_("keysize invalid; using %u bits\n"), nbits );
     }
-    else if (nbits > 4096) {
-        nbits = 4096;
+    else if (nbits > maxsize) {
+        nbits = maxsize;
         log_info(_("keysize invalid; using %u bits\n"), nbits );
     }
 
diff --git a/g10/options.h b/g10/options.h
index d6326d8..670cf64 100644
--- a/g10/options.h
+++ b/g10/options.h
@@ -231,6 +231,7 @@ struct
     unsigned int utf8_filename:1;
     unsigned int dsa2:1;
     unsigned int allow_multiple_messages:1;
+    unsigned int large_rsa:1;
   } flags;
 
   /* Linked list of ways to find a key if the key isn't on the local
-- 
2.1.0

Attachment: pgp2pPGBP91dV.pgp
Description: PGP signature

Reply via email to