From c5445cc3239e7654fbaf145c6e697de54093892f Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 11 Sep 2002 07:27:54 +0000 Subject: [PATCH] * g10.c, options.h: Removed option --emulate-checksum-bug. * misc.c (checksum_u16_nobug): Removed. (checksum_u16): Removed the bug emulation. (checksum_mpi): Ditto. (checksum_mpi_counted_nbits): Removed and replaced all calls with checksum_mpi. * parse-packet.c (read_protected_v3_mpi): New. (parse_key): Use it here to store it as an opaque MPI. * seckey-cert.c (do_check): Changed the v3 unprotection to the new why to store these keys. (protect_secret_key): Likewise. * build-packet.c (do_secret_key): And changed the writing. --- g10/ChangeLog | 22 ++++++++++++- g10/build-packet.c | 16 ++++++++-- g10/g10.c | 1 - g10/keygen.c | 12 +++---- g10/main.h | 1 - g10/misc.c | 50 ++--------------------------- g10/options.h | 1 - g10/parse-packet.c | 73 ++++++++++++++++++++++++++++++++++++------- g10/seckey-cert.c | 78 ++++++++++++++++++++-------------------------- g10/tdbio.c | 24 ++++++++------ 10 files changed, 152 insertions(+), 126 deletions(-) diff --git a/g10/ChangeLog b/g10/ChangeLog index 757df923c..a180dfc55 100644 --- a/g10/ChangeLog +++ b/g10/ChangeLog @@ -1,10 +1,30 @@ 2002-09-10 David Shaw * exec.c (expand_args): Remove loop left over from earlier - implementation. (exec_write): Missed one tick. + implementation. + (exec_write): Missed one tick. 2002-09-10 Werner Koch + * g10.c, options.h: Removed option --emulate-checksum-bug. + * misc.c (checksum_u16_nobug): Removed. + (checksum_u16): Removed the bug emulation. + (checksum_mpi): Ditto. + (checksum_mpi_counted_nbits): Removed and replaced all calls + with checksum_mpi. + + * parse-packet.c (read_protected_v3_mpi): New. + (parse_key): Use it here to store it as an opaque MPI. + * seckey-cert.c (do_check): Changed the v3 unprotection to the new + why to store these keys. + (protect_secret_key): Likewise. + * build-packet.c (do_secret_key): And changed the writing. + + * tdbio.c (tdbio_set_dbname, open_db): Use new macro MY_O_BINARY + to avoid silly ifdefs. + (open_db): Fallback to RDONLY so that gpg may be used from a + RO-medium. + * encode.c (encode_simple): Make sure we don't use an ESK packet when we don't have a salt in the S2K. diff --git a/g10/build-packet.c b/g10/build-packet.c index c4508116c..da1cbbe39 100644 --- a/g10/build-packet.c +++ b/g10/build-packet.c @@ -390,7 +390,7 @@ do_secret_key( IOBUF out, int ctb, PKT_secret_key *sk ) iobuf_put(a, sk->protect.algo ); if( sk->protect.s2k.mode >= 1000 ) { /* These modes are not possible in OpenPGP, we use them - to implement our extesnsions, 101 can ve views as a + to implement our extensions, 101 can be seen as a private/experimental extension (this is not specified in rfc2440 but the same scheme is used for all other algorithm identifiers) */ @@ -426,8 +426,20 @@ do_secret_key( IOBUF out, int ctb, PKT_secret_key *sk ) p = mpi_get_opaque( sk->skey[npkey], &i ); iobuf_write(a, p, i ); } + else if( sk->is_protected ) { + /* The secret key is protected te old v4 way. */ + for( ; i < nskey; i++ ) { + byte *p; + int ndata; + + assert (mpi_is_opaque (sk->skey[i])); + p = mpi_get_opaque (sk->skey[i], &ndata); + iobuf_write (a, p, ndata); + } + write_16(a, sk->csum ); + } else { - /* v3 way - same code for protected and non- protected key */ + /* non-protected key */ for( ; i < nskey; i++ ) mpi_write(a, sk->skey[i] ); write_16(a, sk->csum ); diff --git a/g10/g10.c b/g10/g10.c index e0d2e9450..8206d641e 100644 --- a/g10/g10.c +++ b/g10/g10.c @@ -1432,7 +1432,6 @@ main( int argc, char **argv ) case oNoPGP6: opt.pgp6 = 0; break; case oPGP7: opt.pgp7 = 1; break; case oNoPGP7: opt.pgp7 = 0; break; - case oEmuChecksumBug: opt.emulate_bugs |= EMUBUG_GPGCHKSUM; break; case oEmuMDEncodeBug: opt.emulate_bugs |= EMUBUG_MDENCODE; break; case oCompressSigs: opt.compress_sigs = 1; break; case oRunAsShmCP: diff --git a/g10/keygen.c b/g10/keygen.c index b8398b88a..b45618e90 100644 --- a/g10/keygen.c +++ b/g10/keygen.c @@ -690,7 +690,7 @@ gen_elg(int algo, unsigned nbits, KBNODE pub_root, KBNODE sec_root, DEK *dek, sk->is_protected = 0; sk->protect.algo = 0; - sk->csum = checksum_mpi_counted_nbits( sk->skey[3] ); + sk->csum = checksum_mpi( sk->skey[3] ); if( ret_sk ) /* not a subkey: return an unprotected version of the sk */ *ret_sk = copy_secret_key( NULL, sk ); @@ -776,7 +776,7 @@ gen_dsa(unsigned int nbits, KBNODE pub_root, KBNODE sec_root, DEK *dek, sk->is_protected = 0; sk->protect.algo = 0; - sk->csum = checksum_mpi_counted_nbits( sk->skey[4] ); + sk->csum = checksum_mpi ( sk->skey[4] ); if( ret_sk ) /* not a subkey: return an unprotected version of the sk */ *ret_sk = copy_secret_key( NULL, sk ); @@ -866,10 +866,10 @@ gen_rsa(int algo, unsigned nbits, KBNODE pub_root, KBNODE sec_root, DEK *dek, sk->is_protected = 0; sk->protect.algo = 0; - sk->csum = checksum_mpi_counted_nbits( sk->skey[2] ); - sk->csum += checksum_mpi_counted_nbits( sk->skey[3] ); - sk->csum += checksum_mpi_counted_nbits( sk->skey[4] ); - sk->csum += checksum_mpi_counted_nbits( sk->skey[5] ); + sk->csum = checksum_mpi (sk->skey[2] ); + sk->csum += checksum_mpi (sk->skey[3] ); + sk->csum += checksum_mpi (sk->skey[4] ); + sk->csum += checksum_mpi (sk->skey[5] ); if( ret_sk ) /* not a subkey: return an unprotected version of the sk */ *ret_sk = copy_secret_key( NULL, sk ); diff --git a/g10/main.h b/g10/main.h index efb528aed..0d04911d7 100644 --- a/g10/main.h +++ b/g10/main.h @@ -64,7 +64,6 @@ int disable_core_dumps(void); u16 checksum_u16( unsigned n ); u16 checksum( byte *p, unsigned n ); u16 checksum_mpi( MPI a ); -u16 checksum_mpi_counted_nbits( MPI a ); u32 buffer_to_u32( const byte *buffer ); const byte *get_session_marker( size_t *rlen ); int openpgp_cipher_test_algo( int algo ); diff --git a/g10/misc.c b/g10/misc.c index f4728eb5d..ae553eb47 100644 --- a/g10/misc.c +++ b/g10/misc.c @@ -108,26 +108,12 @@ checksum_u16( unsigned n ) { u16 a; - a = (n >> 8) & 0xff; - if( opt.emulate_bugs & EMUBUG_GPGCHKSUM ) { - a |= n & 0xff; - log_debug("csum_u16 emulated for n=%u\n", n); - } - else - a += n & 0xff; - return a; -} - -static u16 -checksum_u16_nobug( unsigned n ) -{ - u16 a; - a = (n >> 8) & 0xff; a += n & 0xff; return a; } + u16 checksum( byte *p, unsigned n ) { @@ -147,45 +133,13 @@ checksum_mpi( MPI a ) unsigned nbits; buffer = mpi_get_buffer( a, &nbytes, NULL ); - /* some versions of gpg encode wrong values for the length of an mpi - * so that mpi_get_nbits() which counts the mpi yields another (shorter) - * value than the one store with the mpi. mpi_get_nbit_info() returns - * this stored value if it is still available. - */ - - if( opt.emulate_bugs & EMUBUG_GPGCHKSUM ) - nbits = 0; - else - nbits = mpi_get_nbit_info(a); - if( !nbits ) - nbits = mpi_get_nbits(a); + nbits = mpi_get_nbits(a); csum = checksum_u16( nbits ); csum += checksum( buffer, nbytes ); m_free( buffer ); return csum; } -/**************** - * This is the correct function - */ -u16 -checksum_mpi_counted_nbits( MPI a ) -{ - u16 csum; - byte *buffer; - unsigned nbytes; - unsigned nbits; - - buffer = mpi_get_buffer( a, &nbytes, NULL ); - nbits = mpi_get_nbits(a); - mpi_set_nbit_info(a,nbits); - csum = checksum_u16_nobug( nbits ); - csum += checksum( buffer, nbytes ); - m_free( buffer ); - return csum; -} - - u32 buffer_to_u32( const byte *buffer ) { diff --git a/g10/options.h b/g10/options.h index 9ab1042f6..f9301fdd3 100644 --- a/g10/options.h +++ b/g10/options.h @@ -180,7 +180,6 @@ struct { } opt; -#define EMUBUG_GPGCHKSUM 1 #define EMUBUG_MDENCODE 4 #define DBG_PACKET_VALUE 1 /* debug packet reading/writing */ diff --git a/g10/parse-packet.c b/g10/parse-packet.c index 18f860e7a..5ed75e490 100644 --- a/g10/parse-packet.c +++ b/g10/parse-packet.c @@ -1386,6 +1386,51 @@ parse_onepass_sig( IOBUF inp, int pkttype, unsigned long pktlen, } +static MPI +read_protected_v3_mpi (IOBUF inp, unsigned long *length) +{ + int c; + unsigned int nbits, nbytes; + unsigned char *buf, *p; + MPI val; + + if (*length < 2) + { + log_error ("mpi too small\n"); + return NULL; + } + + if ((c=iobuf_get (inp)) == -1) + return NULL; + --*length; + nbits = c << 8; + if ((c=iobuf_get(inp)) == -1) + return NULL; + --*length; + nbits |= c; + + if (nbits > 16384) + { + log_error ("mpi too large (%u bits)\n", nbits); + return NULL; + } + nbytes = (nbits+7) / 8; + buf = p = m_alloc (2 + nbytes); + *p++ = nbits >> 8; + *p++ = nbits; + for (; nbytes && length; nbytes--, --*length) + *p++ = iobuf_get (inp); + if (nbytes) + { + log_error ("packet shorter tham mpi\n"); + m_free (buf); + return NULL; + } + + /* convert buffer into an opaque MPI */ + val = mpi_set_opaque (NULL, buf, p-buf); + return val; +} static int @@ -1666,18 +1711,22 @@ parse_key( IOBUF inp, int pkttype, unsigned long pktlen, } else { /* v3 method: the mpi length is not encrypted */ for(i=npkey; i < nskey; i++ ) { - n = pktlen; sk->skey[i] = mpi_read(inp, &n, 0 ); pktlen -=n; - if( sk->is_protected && sk->skey[i] ) - mpi_set_protect_flag(sk->skey[i]); - if( list_mode ) { - printf( "\tskey[%d]: ", i); - if( sk->is_protected ) - printf( "[encrypted]\n"); - else { - mpi_print(stdout, sk->skey[i], mpi_print_mode ); - putchar('\n'); - } - } + if ( sk->is_protected ) { + sk->skey[i] = read_protected_v3_mpi (inp, &pktlen); + if( list_mode ) + printf( "\tskey[%d]: [encrypted]\n", i); + } + else { + n = pktlen; + sk->skey[i] = mpi_read(inp, &n, 0 ); + pktlen -=n; + if( list_mode ) { + printf( "\tskey[%d]: ", i); + mpi_print(stdout, sk->skey[i], mpi_print_mode ); + putchar('\n'); + } + } + if (!sk->skey[i]) rc = G10ERR_INVALID_PACKET; } diff --git a/g10/seckey-cert.c b/g10/seckey-cert.c index 2eadc9dd3..d2d39dacf 100644 --- a/g10/seckey-cert.c +++ b/g10/seckey-cert.c @@ -149,43 +149,26 @@ do_check( PKT_secret_key *sk, const char *tryagain_text ) else { for(i=pubkey_get_npkey(sk->pubkey_algo); i < pubkey_get_nskey(sk->pubkey_algo); i++ ) { - buffer = mpi_get_secure_buffer( sk->skey[i], &nbytes, NULL ); - cipher_sync( cipher_hd ); - assert( mpi_is_protected(sk->skey[i]) ); - cipher_decrypt( cipher_hd, buffer, buffer, nbytes ); - mpi_set_buffer( sk->skey[i], buffer, nbytes, 0 ); - mpi_clear_protect_flag( sk->skey[i] ); - csum += checksum_mpi( sk->skey[i] ); - m_free( buffer ); - } - if( csum != sk->csum ) { - /* Due to a fix of a bug in mpi_get_secure_buffer we - might encounter seceret keys which are not correctly - encrypted. We fix this by a second try, this time - with a reversed bug fix (the memmove below). */ byte *p; - - copy_secret_key( sk, save_sk ); - cipher_setiv( cipher_hd, sk->protect.iv, sk->protect.ivlen ); - csum = 0; - for(i=pubkey_get_npkey (sk->pubkey_algo); - i < pubkey_get_nskey (sk->pubkey_algo); i++ ) { - buffer = mpi_get_secure_buffer (sk->skey[i], &nbytes,NULL); - for (p=buffer; !*p && nbytes; p++, --nbytes ) - ; - if (p != buffer) - memmove (buffer, p, nbytes); - cipher_sync (cipher_hd); - assert (mpi_is_protected(sk->skey[i])); - cipher_decrypt (cipher_hd, buffer, buffer, nbytes); - mpi_set_buffer (sk->skey[i], buffer, nbytes, 0); - mpi_clear_protect_flag (sk->skey[i]); - csum += checksum_mpi (sk->skey[i]); - m_free (buffer); - } - } - if( opt.emulate_bugs & EMUBUG_GPGCHKSUM ) { - csum = sk->csum; + int ndata; + unsigned int dummy; + + assert (mpi_is_opaque (sk->skey[i])); + p = mpi_get_opaque (sk->skey[i], &ndata); + assert (ndata >= 2); + assert (ndata == ((p[0] << 8 | p[1]) + 7)/8 + 2); + buffer = m_alloc_secure (ndata); + cipher_sync (cipher_hd); + buffer[0] = p[0]; + buffer[1] = p[1]; + cipher_decrypt (cipher_hd, buffer+2, p+2, ndata-2); + csum += checksum (buffer, ndata); + mpi_free (sk->skey[i]); + dummy = ndata; + sk->skey[i] = mpi_read_from_buffer (buffer, &dummy, 1); + assert (sk->skey[i]); + m_free (buffer); +/* csum += checksum_mpi (sk->skey[i]); */ } } cipher_close( cipher_hd ); @@ -366,19 +349,26 @@ protect_secret_key( PKT_secret_key *sk, DEK *dek ) sk->skey[i] = mpi_set_opaque(NULL, data, ndata ); } else { - /* NOTE: we always recalculate the checksum because there - * are some test releases which calculated it wrong */ csum = 0; for(i=pubkey_get_npkey(sk->pubkey_algo); i < pubkey_get_nskey(sk->pubkey_algo); i++ ) { - csum += checksum_mpi_counted_nbits( sk->skey[i] ); + byte *data; + unsigned int nbits; + + csum += checksum_mpi (sk->skey[i]); buffer = mpi_get_buffer( sk->skey[i], &nbytes, NULL ); - cipher_sync( cipher_hd ); - assert( !mpi_is_protected(sk->skey[i]) ); - cipher_encrypt( cipher_hd, buffer, buffer, nbytes ); - mpi_set_buffer( sk->skey[i], buffer, nbytes, 0 ); - mpi_set_protect_flag( sk->skey[i] ); + cipher_sync (cipher_hd); + assert ( !mpi_is_opaque (sk->skey[i]) ); + data = m_alloc (nbytes+2); + nbits = mpi_get_nbits (sk->skey[i]); + assert (nbytes == (nbits + 7)/8); + data[0] = nbits >> 8; + data[1] = nbits; + cipher_encrypt (cipher_hd, data+2, buffer, nbytes); m_free( buffer ); + + mpi_free (sk->skey[i]); + sk->skey[i] = mpi_set_opaque (NULL, data, nbytes+2); } sk->csum = csum; } diff --git a/g10/tdbio.c b/g10/tdbio.c index cdeba46cf..62ff26774 100644 --- a/g10/tdbio.c +++ b/g10/tdbio.c @@ -43,6 +43,13 @@ #define ftruncate chsize #endif +#ifdef HAVE_DOSISH_SYSTEM +#define MY_O_BINARY O_BINARY +#else +#define MY_O_BINARY 0 +#endif + + /**************** * Yes, this is a very simple implementation. We should really * use a page aligned buffer and read complete pages. @@ -484,11 +491,7 @@ tdbio_set_dbname( const char *new_dbname, int create ) if( !fp ) log_fatal( _("%s: can't create: %s\n"), fname, strerror(errno) ); fclose(fp); - #ifdef HAVE_DOSISH_SYSTEM - db_fd = open( db_name, O_RDWR | O_BINARY ); - #else - db_fd = open( db_name, O_RDWR ); - #endif + db_fd = open( db_name, O_RDWR | MY_O_BINARY ); if( db_fd == -1 ) log_fatal( _("%s: can't open: %s\n"), db_name, strerror(errno) ); @@ -544,11 +547,12 @@ open_db() if (make_dotlock( lockhandle, -1 ) ) log_fatal( _("%s: can't make lock\n"), db_name ); #endif /* __riscos__ */ -#ifdef HAVE_DOSISH_SYSTEM - db_fd = open (db_name, O_RDWR | O_BINARY ); -#else - db_fd = open (db_name, O_RDWR ); -#endif + db_fd = open (db_name, O_RDWR | MY_O_BINARY ); + if (db_fd == -1 && errno == EACCES) { + db_fd = open (db_name, O_RDONLY | MY_O_BINARY ); + if (db_fd != -1) + log_info (_("NOTE: trustdb not writable\n")); + } if ( db_fd == -1 ) log_fatal( _("%s: can't open: %s\n"), db_name, strerror(errno) );