diff options
| -rw-r--r-- | NEWS | 4 | ||||
| -rw-r--r-- | g10/ChangeLog | 11 | ||||
| -rw-r--r-- | g10/armor.c | 60 | ||||
| -rw-r--r-- | g10/main.h | 1 | ||||
| -rw-r--r-- | g10/mainproc.c | 61 | ||||
| -rw-r--r-- | g10/misc.c | 27 | ||||
| -rw-r--r-- | g10/packet.h | 7 | ||||
| -rw-r--r-- | g10/parse-packet.c | 68 |
8 files changed, 192 insertions, 47 deletions
@@ -3,7 +3,9 @@ be used to verify signatures against a list of trusted keys. * Rijndael (AES) is now supported and listed as first preference. - + + * Fixed a serious bug which could lead to false signature verification + results when more than one clearsigned signatures are in one file. Noteworthy changes in version 1.0.3 (2000-09-18) ------------------------------------------------ diff --git a/g10/ChangeLog b/g10/ChangeLog index d3cba7a74..b3887cdae 100644 --- a/g10/ChangeLog +++ b/g10/ChangeLog @@ -1,5 +1,16 @@ 2000-10-13 Werner Koch <[email protected]> + * mainproc.c (add_gpg_control): New. + (do_proc_packets): use it. + (proc_plaintext): Changed logic to detect clearsigns. + (proc_tree): Check the cleartext sig with some new code. + + * packet.h: New packet PKT_GPG_CONTROL. + * parse-packet.c (parse_gpg_control): New. + * misc.c (get_session_marker): New. + * armor.c (armor_filter): Replaced the faked 1-pass packet by the + new control packet. + * keyedit.c (keyedit_menu): Allow batchmode with a command_fd. * status.c (my_read): New. (do_get_from_fd): use it. diff --git a/g10/armor.c b/g10/armor.c index 7622dd039..cc7d97214 100644 --- a/g10/armor.c +++ b/g10/armor.c @@ -502,7 +502,7 @@ fake_packet( armor_filter_context_t *afx, IOBUF a, /* the buffer is always allocated with enough space to append * the removed [CR], LF and a Nul * The reason for this complicated procedure is to keep at least - * the original tupe of lineending - handling of the removed + * the original type of lineending - handling of the removed * trailing spaces seems to be impossible in our method * of faking a packet; either we have to use a temporary file * or calculate the hash here in this module and somehow find @@ -815,7 +815,9 @@ armor_filter( void *opaque, int control, *ret_len = n; } else if( control == IOBUFCTRL_UNDERFLOW ) { - if( size < 15+(4*15) ) /* need space for up to 4 onepass_sigs */ + /* We need some space for the faked packet. The minmum required + * size is ~18 + length of the session marker */ + if( size < 50 ) BUG(); /* supplied buffer too short */ if( afx->faked ) @@ -831,7 +833,14 @@ armor_filter( void *opaque, int control, rc = -1; } else if( afx->faked ) { - unsigned hashes = afx->hashes; + unsigned int hashes = afx->hashes; + const byte *sesmark; + size_t sesmarklen; + + sesmark = get_session_marker( &sesmarklen ); + if ( sesmarklen > 20 ) + BUG(); + /* the buffer is at least 15+n*15 bytes long, so it * is easy to construct the packets */ @@ -842,36 +851,21 @@ armor_filter( void *opaque, int control, afx->pgp2mode = 1; } n=0; - do { - /* first some onepass signature packets */ - buf[n++] = 0x90; /* old format, type 4, 1 length byte */ - buf[n++] = 13; /* length */ - buf[n++] = 3; /* version */ - buf[n++] = afx->not_dash_escaped? 0:1; /* sigclass */ - if( hashes & 1 ) { - hashes &= ~1; - buf[n++] = DIGEST_ALGO_RMD160; - } - else if( hashes & 2 ) { - hashes &= ~2; - buf[n++] = DIGEST_ALGO_SHA1; - } - else if( hashes & 4 ) { - hashes &= ~4; - buf[n++] = DIGEST_ALGO_MD5; - } - else if( hashes & 8 ) { - hashes &= ~8; - buf[n++] = DIGEST_ALGO_TIGER; - } - else - buf[n++] = 0; /* (don't know) */ - - buf[n++] = 0; /* public key algo (don't know) */ - memset(buf+n, 0, 8); /* don't know the keyid */ - n += 8; - buf[n++] = !hashes; /* last one */ - } while( hashes ); + /* first a gpg control packet */ + buf[n++] = 0xff; /* new format, type 63, 1 length byte */ + n++; /* see below */ + memcpy(buf+n, sesmark, sesmarklen ); n+= sesmarklen; + buf[n++] = 1; /* control type */ + buf[n++] = afx->not_dash_escaped? 0:1; /* sigclass */ + if( hashes & 1 ) + buf[n++] = DIGEST_ALGO_RMD160; + if( hashes & 2 ) + buf[n++] = DIGEST_ALGO_SHA1; + if( hashes & 4 ) + buf[n++] = DIGEST_ALGO_MD5; + if( hashes & 8 ) + buf[n++] = DIGEST_ALGO_TIGER; + buf[1] = n - 2; /* followed by a plaintext packet */ buf[n++] = 0xaf; /* old packet format, type 11, var length */ diff --git a/g10/main.h b/g10/main.h index 13178c735..cd52c827b 100644 --- a/g10/main.h +++ b/g10/main.h @@ -60,6 +60,7 @@ 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 ); /*-- helptext.c --*/ void display_online_help( const char *keyword ); diff --git a/g10/mainproc.c b/g10/mainproc.c index 6449d3823..ac9aecc66 100644 --- a/g10/mainproc.c +++ b/g10/mainproc.c @@ -119,6 +119,24 @@ add_onepass_sig( CTX c, PACKET *pkt ) } +static int +add_gpg_control( CTX c, PACKET *pkt ) +{ + if ( pkt->pkt.gpg_control->control == 1 ) { + /* New clear text signature. + * Process the last one and reset everything */ + release_list(c); + } + + if( c->list ) /* add another packet */ + add_kbnode( c->list, new_kbnode( pkt )); + else /* insert the first one */ + c->list = new_kbnode( pkt ); + + return 1; +} + + static int add_user_id( CTX c, PACKET *pkt ) @@ -412,20 +430,23 @@ proc_plaintext( CTX c, PACKET *pkt ) } if( n->pkt->pkt.onepass_sig->sig_class != 0x01 ) only_md5 = 0; - - /* Check whether this is a cleartext signature. We assume that - * we have one if the sig_class is 1 and the keyid is 0, that - * are the faked packets produced by armor.c. There is a - * possibility that this fails, but there is no other easy way - * to do it. (We could use a special packet type to indicate - * this, but this may also be faked - it simply can't be verified - * and is _no_ security issue) - */ - if( n->pkt->pkt.onepass_sig->sig_class == 0x01 - && !n->pkt->pkt.onepass_sig->keyid[0] - && !n->pkt->pkt.onepass_sig->keyid[1] ) - clearsig = 1; } + else if( n->pkt->pkttype == PKT_GPG_CONTROL + && n->pkt->pkt.gpg_control->control == 1 ) { + size_t datalen = n->pkt->pkt.gpg_control->datalen; + const byte *data = n->pkt->pkt.gpg_control->data; + + /* check that we have at least the sigclass and one hash */ + if ( datalen < 2 ) + log_fatal("invalid control packet of type 1\n"); + /* Note that we don't set the clearsig flag for not-dash-escaped + * documents */ + clearsig = (*data == 0x01); + for( data++, datalen--; datalen; datalen--, data++ ) + md_enable( c->mfx.md, *data ); + any = 1; + break; /* no pass signature pakets are expected */ + } } if( !any && !opt.skip_verify ) { @@ -993,6 +1014,7 @@ do_proc_packets( CTX c, IOBUF a ) case PKT_PLAINTEXT: proc_plaintext( c, pkt ); break; case PKT_COMPRESSED: proc_compressed( c, pkt ); break; case PKT_ONEPASS_SIG: newpkt = add_onepass_sig( c, pkt ); break; + case PKT_GPG_CONTROL: newpkt = add_gpg_control(c, pkt); break; default: newpkt = 0; break; } } @@ -1011,6 +1033,7 @@ do_proc_packets( CTX c, IOBUF a ) case PKT_PLAINTEXT: proc_plaintext( c, pkt ); break; case PKT_COMPRESSED: proc_compressed( c, pkt ); break; case PKT_ONEPASS_SIG: newpkt = add_onepass_sig( c, pkt ); break; + case PKT_GPG_CONTROL: newpkt = add_gpg_control(c, pkt); break; default: newpkt = 0; break; } } @@ -1035,6 +1058,7 @@ do_proc_packets( CTX c, IOBUF a ) case PKT_PLAINTEXT: proc_plaintext( c, pkt ); break; case PKT_COMPRESSED: proc_compressed( c, pkt ); break; case PKT_ONEPASS_SIG: newpkt = add_onepass_sig( c, pkt ); break; + case PKT_GPG_CONTROL: newpkt = add_gpg_control(c, pkt); break; case PKT_RING_TRUST: newpkt = add_ring_trust( c, pkt ); break; default: newpkt = 0; break; } @@ -1228,6 +1252,17 @@ proc_tree( CTX c, KBNODE node ) for( n1 = node; (n1 = find_next_kbnode(n1, PKT_SIGNATURE )); ) check_sig_and_print( c, n1 ); } + else if( node->pkt->pkttype == PKT_GPG_CONTROL + && node->pkt->pkt.gpg_control->control == 1 ) { + /* clear text signed message */ + if( !c->have_data ) { + log_error("cleartext signature without data\n" ); + return; + } + + for( n1 = node; (n1 = find_next_kbnode(n1, PKT_SIGNATURE )); ) + check_sig_and_print( c, n1 ); + } else if( node->pkt->pkttype == PKT_SIGNATURE ) { PKT_signature *sig = node->pkt->pkt.signature; diff --git a/g10/misc.c b/g10/misc.c index 9669af574..317bdbdce 100644 --- a/g10/misc.c +++ b/g10/misc.c @@ -22,6 +22,7 @@ #include <stdio.h> #include <stdlib.h> #include <string.h> +#include <unistd.h> #include <errno.h> #if defined(__linux__) && defined(__alpha__) && __GLIBC__ < 2 #include <asm/sysinfo.h> @@ -244,6 +245,32 @@ print_digest_algo_note( int algo ) } +/* Return a string which is used as a kind of process ID */ +const byte * +get_session_marker( size_t *rlen ) +{ + static byte marker[SIZEOF_UNSIGNED_LONG*2]; + static int initialized; + + if ( !initialized ) { + volatile ulong aa, bb; /* we really want the unitialized value */ + ulong a, b; + + initialized = 1; + /* also this marker is guessable it is not easy to use this + * for a faked control packet because an attacker does not + * have enough control about the time the verification does + * take place. Of course, we can add just more random but + * than we need the random generator even for verification + * tasks - which does not make sense. */ + a = aa ^ (ulong)getpid(); + b = bb ^ (ulong)time(NULL); + memcpy( marker, &a, SIZEOF_UNSIGNED_LONG ); + memcpy( marker+SIZEOF_UNSIGNED_LONG, &b, SIZEOF_UNSIGNED_LONG ); + } + *rlen = sizeof(marker); + return marker; +} diff --git a/g10/packet.h b/g10/packet.h index 851efab24..a554b7535 100644 --- a/g10/packet.h +++ b/g10/packet.h @@ -50,6 +50,7 @@ typedef enum { PKT_ENCRYPTED_MDC =18, /* integrity protected encrypted data */ PKT_MDC =19, /* manipulaion detection code packet */ PKT_COMMENT =61, /* new comment packet (private) */ + PKT_GPG_CONTROL =63 /* internal control packet */ } pkttype_t; typedef struct packet_struct PACKET; @@ -194,6 +195,11 @@ typedef struct { char name[1]; } PKT_plaintext; +typedef struct { + int control; + size_t datalen; + char data[1]; +} PKT_gpg_control; /* combine all packets into a union */ struct packet_struct { @@ -213,6 +219,7 @@ struct packet_struct { PKT_mdc *mdc; /* PKT_MDC */ PKT_ring_trust *ring_trust; /* PKT_RING_TRUST */ PKT_plaintext *plaintext; /* PKT_PLAINTEXT */ + PKT_gpg_control *gpg_control; /* PKT_GPG_CONTROL */ } pkt; }; diff --git a/g10/parse-packet.c b/g10/parse-packet.c index 3521951b9..ee2ff56eb 100644 --- a/g10/parse-packet.c +++ b/g10/parse-packet.c @@ -75,6 +75,8 @@ static int parse_encrypted( IOBUF inp, int pkttype, unsigned long pktlen, PACKET *packet, int new_ctb); static int parse_mdc( IOBUF inp, int pkttype, unsigned long pktlen, PACKET *packet, int new_ctb); +static int parse_gpg_control( IOBUF inp, int pkttype, unsigned long pktlen, + PACKET *packet ); static unsigned short read_16(IOBUF inp) @@ -446,6 +448,9 @@ parse( IOBUF inp, PACKET *pkt, int reqtype, ulong *retpos, case PKT_MDC: rc = parse_mdc(inp, pkttype, pktlen, pkt, new_ctb ); break; + case PKT_GPG_CONTROL: + rc = parse_gpg_control(inp, pkttype, pktlen, pkt ); + break; default: skip_packet(inp, pkttype, pktlen); break; @@ -1783,3 +1788,66 @@ parse_mdc( IOBUF inp, int pkttype, unsigned long pktlen, return 0; } + +/* + * This packet is internally generated by PGG (by armor.c) to + * transfer some information to the lower layer. To make sure that + * this packet is really a GPG faked one and not one comming from outside, + * we first check that tehre is a unique tag in it. + * The format of such a control packet is: + * n byte session marker + * 1 byte control type: 1 = Clearsign hash info + * m byte control data + */ + +static int +parse_gpg_control( IOBUF inp, + int pkttype, unsigned long pktlen, PACKET *packet ) +{ + byte *p; + const byte *sesmark; + size_t sesmarklen; + int i; + + if ( list_mode ) + printf(":packet 63: length %lu ", pktlen); + + sesmark = get_session_marker ( &sesmarklen ); + if ( pktlen < sesmarklen+1 ) /* 1 is for the control bytes */ + goto skipit; + for( i=0; i < sesmarklen; i++, pktlen-- ) { + if ( sesmark[i] != iobuf_get_noeof(inp) ) + goto skipit; + } + if ( list_mode ) + puts ("- gpg control packet"); + + packet->pkt.gpg_control = m_alloc(sizeof *packet->pkt.gpg_control + + pktlen - 1); + packet->pkt.gpg_control->control = iobuf_get_noeof(inp); pktlen--; + packet->pkt.gpg_control->datalen = pktlen; + p = packet->pkt.gpg_control->data; + for( ; pktlen; pktlen--, p++ ) + *p = iobuf_get_noeof(inp); + + return 0; + + skipit: + if ( list_mode ) { + int c, i=0 ; + printf("- private (rest length %lu)\n", pktlen); + if( iobuf_in_block_mode(inp) ) { + while( (c=iobuf_get(inp)) != -1 ) + dump_hex_line(c, &i); + } + else { + for( ; pktlen; pktlen-- ) + dump_hex_line(iobuf_get(inp), &i); + } + putchar('\n'); + } + skip_rest(inp,pktlen); + return G10ERR_INVALID_PACKET; +} + + |