From 9e3526f236a94423830b75e59db40d20ac69b856 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Mon, 20 Dec 2004 10:05:20 +0000 Subject: [PATCH] * seckey-cert.c (do_check): Handle case when checksum was okay but passphrase still wrong. Roman Pavlik found such a case. * mpicoder.c (mpi_read_from_buffer): Don't abort in case of an invalid MPI but print a message and return NULL. Use log_info and not log_error. --- ChangeLog | 5 +++++ THANKS | 1 + g10/ChangeLog | 5 +++++ g10/seckey-cert.c | 19 +++++++++++++++++-- mpi/ChangeLog | 6 ++++++ mpi/mpicoder.c | 21 +++++++++++++++------ 6 files changed, 49 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index cdda977ae..4213450f6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -12,6 +12,11 @@ * configure.ac: Check for arpa/nameser.h. +2004-12-16 Werner Koch + + * THANKS: Added John Clizbe for help testing the 1.4.0a W32 + binary. + 2004-12-16 Werner Koch Released 1.4.0. diff --git a/THANKS b/THANKS index 00a1ceec2..d72763a85 100644 --- a/THANKS +++ b/THANKS @@ -101,6 +101,7 @@ Jim Small cavenewt@my-deja.com Joachim Backes backes@rhrk.uni-kl.de Joe Rhett jrhett@isite.net John A. Martin jam@jamux.com +John Clizbe JPClizbe@comcast.net Johnny Teveßen j.tevessen@gmx.de Jörg Schilling schilling@fokus.gmd.de Jos Backus Jos.Backus@nl.origin-it.com diff --git a/g10/ChangeLog b/g10/ChangeLog index b2b395da1..aed7b32f3 100644 --- a/g10/ChangeLog +++ b/g10/ChangeLog @@ -1,3 +1,8 @@ +2004-12-20 Werner Koch + + * seckey-cert.c (do_check): Handle case when checksum was okay but + passphrase still wrong. Roman Pavlik found such a case. + 2004-12-20 David Shaw * keyedit.c (keyedit_menu): Invisible alias "passwd" as diff --git a/g10/seckey-cert.c b/g10/seckey-cert.c index 84533bd3e..9153b9508 100644 --- a/g10/seckey-cert.c +++ b/g10/seckey-cert.c @@ -147,12 +147,20 @@ do_check( PKT_secret_key *sk, const char *tryagain_text, int mode, } } - /* must check it here otherwise the mpi_read_xx would fail + /* Must check it here otherwise the mpi_read_xx would fail because the length may have an arbitrary value */ if( sk->csum == csum ) { for( ; i < pubkey_get_nskey(sk->pubkey_algo); i++ ) { nbytes = ndata; sk->skey[i] = mpi_read_from_buffer(p, &nbytes, 1 ); + if (!sk->skey[i]) + { + /* Checksum was okay, but not correctly + decrypted. */ + sk->csum = 0; + csum = 1; + break; + } ndata -= nbytes; p += nbytes; } @@ -179,8 +187,15 @@ do_check( PKT_secret_key *sk, const char *tryagain_text, int mode, csum += checksum (buffer, ndata); mpi_free (sk->skey[i]); sk->skey[i] = mpi_read_from_buffer (buffer, &ndata, 1); - assert (sk->skey[i]); m_free (buffer); + if (!sk->skey[i]) + { + /* Checksum was okay, but not correctly + decrypted. */ + sk->csum = 0; + csum = 1; + break; + } /* csum += checksum_mpi (sk->skey[i]); */ } } diff --git a/mpi/ChangeLog b/mpi/ChangeLog index ab9002722..20175f961 100644 --- a/mpi/ChangeLog +++ b/mpi/ChangeLog @@ -1,3 +1,9 @@ +2004-12-20 Werner Koch + + * mpicoder.c (mpi_read_from_buffer): Don't abort in case of an + invalid MPI but print a message and return NULL. Use log_info and + not log_error. + 2004-10-26 Werner Koch * config.links: Use HOST instead of TARGET. diff --git a/mpi/mpicoder.c b/mpi/mpicoder.c index 0005f21a8..4d4e45553 100644 --- a/mpi/mpicoder.c +++ b/mpi/mpicoder.c @@ -125,7 +125,7 @@ mpi_read(IOBUF inp, unsigned *ret_nread, int secure) MPI -mpi_read_from_buffer(byte *buffer, unsigned *ret_nread, int secure) +mpi_read_from_buffer(byte *buffer, unsigned int *ret_nread, int secure) { int i, j; unsigned nbits, nbytes, nlimbs, nread=0; @@ -136,7 +136,7 @@ mpi_read_from_buffer(byte *buffer, unsigned *ret_nread, int secure) goto leave; nbits = buffer[0] << 8 | buffer[1]; if( nbits > MAX_EXTERN_MPI_BITS ) { - log_error("mpi too large (%u bits)\n", nbits); + log_info ("mpi too large (%u bits)\n", nbits); goto leave; } buffer += 2; @@ -154,10 +154,19 @@ mpi_read_from_buffer(byte *buffer, unsigned *ret_nread, int secure) for( ; j > 0; j-- ) { a = 0; for(; i < BYTES_PER_MPI_LIMB; i++ ) { - if( ++nread > *ret_nread ) - log_bug("mpi larger than buffer\n"); - a <<= 8; - a |= *buffer++; + if( ++nread > *ret_nread ) { + /* This (as well as the above error condition) may + happen if we use this function to parse a decrypted + MPI which didn't turn out to be a real MPI - possible + because the supplied key was wrong but the OpenPGP + checksum didn't caught it. */ + log_info ("mpi larger than buffer\n"); + mpi_free (val); + val = MPI_NULL; + goto leave; + } + a <<= 8; + a |= *buffer++; } i = 0; val->d[j-1] = a;