NetBSD Problem Report #51240

From www@NetBSD.org  Tue Jun 14 15:12:28 2016
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 4713A7A0F3
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 14 Jun 2016 15:12:28 +0000 (UTC)
Message-Id: <20160614151226.B7A9F7AAB1@mollari.NetBSD.org>
Date: Tue, 14 Jun 2016 15:12:26 +0000 (UTC)
From: xnox@ubuntu.com
Reply-To: xnox@ubuntu.com
To: gnats-bugs@NetBSD.org
Subject: Version comment mandatory for clearsign netpgpverify
X-Send-Pr-Version: www-1.0

>Number:         51240
>Category:       security
>Synopsis:       Version comment mandatory for clearsign netpgpverify
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    security-officer
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Jun 14 15:15:00 +0000 2016
>Closed-Date:    Tue Jun 14 18:07:50 +0000 2016
>Last-Modified:  Mon Apr 17 19:55:01 +0000 2017
>Originator:     Dimitri John Ledkov
>Release:        netpgpverify-20160313
>Organization:
individual
>Environment:
4.4.0-24-generic x86_64 GNU/Linux
>Description:
# echo bar | gpg --clearsign | gpg --verify
works
# echo bar | gpg --clearsign --no-emit-version | gpg --verify
works
# echo bar | gpg --clearsign | ./netpgpverify
works

# echo bar | gpg --clearsign --no-emit-version | ./netpgpverify 
Signature did not match contents -- No signature found

Does not work....
>How-To-Repeat:
# echo bar | gpg --clearsign --no-emit-version | ./netpgpverify 
>Fix:
libverify.c is clearly failing to parse the signature block correctly when it's missing a Version comment, and/or other comments?!

>Release-Note:

>Audit-Trail:
From: "Alistair G. Crooks" <agc@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/51240 CVS commit: pkgsrc/security/netpgpverify/files
Date: Tue, 14 Jun 2016 18:00:59 +0000

 Module Name:	pkgsrc
 Committed By:	agc
 Date:		Tue Jun 14 18:00:59 UTC 2016

 Modified Files:
 	pkgsrc/security/netpgpverify/files: Makefile.bsd Makefile.in
 	    libverify.c verify.h
 Added Files:
 	pkgsrc/security/netpgpverify/files: noversion.asc

 Log Message:
 Update netpgpverify (and libnetpgpverify) to 20160614

 + handle signatures created by gpg with "--no-emit-version", don't assume
 there will always be a version string.

 + add a test for above

 Fixes security PR/51240.

 Thanks to xnox@ubuntu.com for reporting the error


 To generate a diff of this commit:
 cvs rdiff -u -r1.8 -r1.9 pkgsrc/security/netpgpverify/files/Makefile.bsd
 cvs rdiff -u -r1.4 -r1.5 pkgsrc/security/netpgpverify/files/Makefile.in
 cvs rdiff -u -r1.13 -r1.14 pkgsrc/security/netpgpverify/files/libverify.c
 cvs rdiff -u -r0 -r1.1 pkgsrc/security/netpgpverify/files/noversion.asc
 cvs rdiff -u -r1.20 -r1.21 pkgsrc/security/netpgpverify/files/verify.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: agc@NetBSD.org
State-Changed-When: Tue, 14 Jun 2016 18:07:50 +0000
State-Changed-Why:
Thanks for the bug report - should be fixed now


From: Dimitri John Ledkov <xnox@ubuntu.com>
To: agc@netbsd.org
Cc: security-officer@netbsd.org, gnats-admin@netbsd.org, 
	security-alert@netbsd.org, gnats-bugs@netbsd.org
Subject: Re: PR/51240 CVS commit: pkgsrc/security/netpgpverify/files
Date: Wed, 15 Jun 2016 00:36:57 +0300

 --001a114b14b4fbe5a3053543cd75
 Content-Type: text/plain; charset=UTF-8

 Hello,

 >  Log Message:
 >  Update netpgpverify (and libnetpgpverify) to 20160614
 >
 >  + handle signatures created by gpg with "--no-emit-version", don't assume
 >  there will always be a version string.
 >
 >  + add a test for above
 >
 >  Fixes security PR/51240.

 The fix is good, and is almost the same that I was working on offline.

 But the signature block can have other headers to e.g.

 -----BEGIN PGP SIGNATURE-----
 Version: GnuPG v1
 Comment: Foo bar

 or
 -----BEGIN PGP SIGNATURE-----
 Comment: Foo bar

 When gpg invoked with --comment "Foo bar" --no-version.

 Thus I'd recommend to tweak the libverify.c as attached:

  - Check if p[0] points at new line, then there are no optional
 comments/version lines and one can do p += 1 to continue.

  - Otherwise look for double \n\n for end of comment/version lines.

 Or some such.

 I'm reading RFC 4880 and i'm guessing that there are a few possible
 headers that maybe used:

 https://tools.ietf.org/html/rfc4880#page-56

 Both "Version" and "Comment" are well known.

 -- 
 Regards,

 Dimitri.

 --001a114b14b4fbe5a3053543cd75
 Content-Type: text/x-patch; charset=US-ASCII; name="libverify.patch"
 Content-Disposition: attachment; filename="libverify.patch"
 Content-Transfer-Encoding: base64
 X-Attachment-Id: f_ipfyfode0

 LS0tIGxpYnZlcmlmeS5jLm9sZAkyMDE2LTA2LTE1IDAwOjA5OjMzLjIwMjI0MDEzNSArMDMwMAor
 KysgbGlidmVyaWZ5LmMJMjAxNi0wNi0xNSAwMDoxNjozNS42MTQ2ODYxMjIgKzAzMDAKQEAgLTIw
 MjIsMTYgKzIwMjIsMTcgQEAKIAl9CiAJbGl0ZGF0YS51LmxpdGRhdGEubGVuID0gbGl0ZGF0YS5z
 LnNpemUgPSAoc2l6ZV90KShwIC0gZGF0YXN0YXJ0KTsKIAlwICs9IHN0cmxlbihTSUdTVEFSVCk7
 Ci0JLyogV29yayBvdXQgd2h0aGVyIHRoZXJlJ3MgYSB2ZXJzaW9uIGxpbmUgKi8KLQlpZiAobWVt
 Y21wKHAsICJWZXJzaW9uOiIsIDgpID09IDApIHsKKwkvKiBXb3JrIG91dCB3aHRoZXIgdGhlcmUg
 YXJlIHZlcnNpb24vY29tbWVudCBsaW5lcyAqLworCWlmIChwWzBdID09ICdcbicpIHsKKwkJcCAr
 PSAxOworCX0gZWxzZSB7CisJCS8qIFNraXAgb3B0aW9uYWwgdmVyc2lvbi9jb21tZW50IGxpbmVz
 ICovCiAJCWlmICgocCA9IGZpbmRfYmluX3N0cmluZyhwLCBtZW0tPnNpemUsICJcblxuIiwgIDIp
 KSA9PSBOVUxMKSB7CiAJCQlzbnByaW50ZihjdXJzb3ItPndoeSwgc2l6ZW9mKGN1cnNvci0+d2h5
 KSwKIAkJCQkibWFsZm9ybWVkIGFybWVkIHNpZ25hdHVyZSBhdCAlenUiLCAoc2l6ZV90KShwIC0g
 bWVtLT5tZW0pKTsKIAkJCXJldHVybiAwOwogCQl9CiAJCXAgKz0gMjsKLQl9IGVsc2UgewotCQlw
 ICs9IDE7CiAJfQogCXNpZ2VuZCA9IGZpbmRfYmluX3N0cmluZyhwLCBtZW0tPnNpemUsIFNJR0VO
 RCwgc3RybGVuKFNJR0VORCkpOwogCWJpbnNpZ3NpemUgPSBiNjRkZWNvZGUoKGNoYXIgKilwLCAo
 c2l6ZV90KShzaWdlbmQgLSBwKSwgYmluc2lnLCBzaXplb2YoYmluc2lnKSk7Cg==
 --001a114b14b4fbe5a3053543cd75--

From: Alistair Crooks <agc@pkgsrc.org>
To: Dimitri John Ledkov <xnox@ubuntu.com>
Cc: Alistair Crooks <agc@netbsd.org>, security-officer <security-officer@netbsd.org>, gnats-admin@netbsd.org, 
	security-alert@netbsd.org, gnats-bugs@netbsd.org
Subject: Re: PR/51240 CVS commit: pkgsrc/security/netpgpverify/files
Date: Tue, 14 Jun 2016 14:45:22 -0700

 Thanks for the patch.

 I think your version is good, but it would be much more efficient to
 backup one character when the signature start is found, and just
 search for the "\n\n" sequence. I'll look into doing that later on
 today or tomorrow.

 Regards,
 Alistair

 PS. Not sure why there's such a wide distribution list to this one.
 Please advise.

 On 14 June 2016 at 14:36, Dimitri John Ledkov <xnox@ubuntu.com> wrote:
 > Hello,
 >
 >>  Log Message:
 >>  Update netpgpverify (and libnetpgpverify) to 20160614
 >>
 >>  + handle signatures created by gpg with "--no-emit-version", don't assume
 >>  there will always be a version string.
 >>
 >>  + add a test for above
 >>
 >>  Fixes security PR/51240.
 >
 > The fix is good, and is almost the same that I was working on offline.
 >
 > But the signature block can have other headers to e.g.
 >
 > -----BEGIN PGP SIGNATURE-----
 > Version: GnuPG v1
 > Comment: Foo bar
 >
 > or
 > -----BEGIN PGP SIGNATURE-----
 > Comment: Foo bar
 >
 > When gpg invoked with --comment "Foo bar" --no-version.
 >
 > Thus I'd recommend to tweak the libverify.c as attached:
 >
 >  - Check if p[0] points at new line, then there are no optional
 > comments/version lines and one can do p += 1 to continue.
 >
 >  - Otherwise look for double \n\n for end of comment/version lines.
 >
 > Or some such.
 >
 > I'm reading RFC 4880 and i'm guessing that there are a few possible
 > headers that maybe used:
 >
 > https://tools.ietf.org/html/rfc4880#page-56
 >
 > Both "Version" and "Comment" are well known.
 >
 > --
 > Regards,
 >
 > Dimitri.

From: "Alistair G. Crooks" <agc@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/51240 CVS commit: src/crypto/external/bsd/netpgp
Date: Mon, 17 Apr 2017 19:50:28 +0000

 Module Name:	src
 Committed By:	agc
 Date:		Mon Apr 17 19:50:28 UTC 2017

 Modified Files:
 	src/crypto/external/bsd/netpgp/bin/netpgpverify: Makefile
 	src/crypto/external/bsd/netpgp/dist/src/netpgpverify: Makefile.bsd
 	    Makefile.in Makefile.lib.in Makefile.libtool.in array.h bignum.c
 	    digest.c digest.h libnetpgpverify.3 libverify.c main.c
 	    netpgpverify.1 pgpsum.c verify.h
 	src/crypto/external/bsd/netpgp/lib/verify: Makefile
 Removed Files:
 	src/crypto/external/bsd/netpgp/dist/src/netpgpverify: tiger.c tiger.h

 Log Message:
 Update netpgpverify sources in base from 20160617 to 20170201 (i.e. bring
 over changes from master sources in pkgsrc/security/netpgpverify, version 20170201):

 Changes:

 Update netpgpverify (and libnetpgpverify) to 20160614
 	+ handle signatures created by gpg with "--no-emit-version", don't assume
 	there will always be a version string.
 	+ add a test for above
 	Fixes security PR  51240.
 	Thanks to xnox@ubuntu.com for reporting the error

 Update netpgpverify and libnetpgpverify to 20160615:
 	Simplify the method of finding the end of the versioning information
 	in the signature - back up to the "\n" character at the end of the
 	signature start:

 		"-----BEGIN PGP SIGNATURE-----\n"

 	and then find the "\n\n" character sequence to denote the start of the
 	signature itself. The previous version worked, but this is more efficient.

 Update netpgpverify and libnetpgpverify to 20160616
 	+ bring over joerg's printflike change from the netpgpverify
 	version in src/crypto
 	+ add a test for cleartext signatures with version information
 	to complement the one with no version information

 Update netpgpverify and libnetpgpverify to 20160622 during freeze to fix PR  51262
 	+ take a bit of a step backwards, and don't use stdbool.h, just to appease
 	Solaris 10 compiler

 Update netpgpverify and libnetpgpverify to 20160623
 	+ remove use of asprintf and vasprintf from libverify. Inspired
 	by work from Dimitri John Ledkov. Should allow building on Linux
 	without superfluous definitions.
 	+ also free the BIGNUM struct in PGPV_BN_clear() - from Dimitri
 	John Ledkov

 Update netpgpverify and libnetpgpverify to 20160626
 	+ make the pgpv_t and pgpv_cursor_t structures opaque
 	+ add new accessor functions for fields in the pgpv_cursor_t struct
 	+ add new creation functions for the pgpv_t and pgpv_cursor_t structs

 Update netpgpverify and libnetpgpverify to 20160704
 	+ get rid of redundant PGPV_ARRAY definition in libverify.c, brought in when
 	the definitions moved from verify.h
 	+ fix obuf_add_mem() to use a const void *, as any struct can be
 	dumped using it
 	+ remove redundant NO_SUBKEYS definition - unused
 	+ add an (unused as yet) ARRAY_FREE() macro

 Update netpgpverify and libnetpgpverify to 20160705
 	External API changes
 	====================
 	+ add a pgpv_cursor_close() function to free resources associated with
 	a cursor
 	Better memory management
 	========================
 	+ restructure the way dynamic arrays are used, to avoid memory
 	corruption issues and memory leaks - keep all dynamic arrays in the global
 	data structure, and use indices in the other data structures to index them.
 	Means lack of data localisation, but avoids stale pointers, and leaks.
 	+ make signer field of signature a uint8_t array, rather than a pointer
 	+ use our own version of strdup(3) - don't depend on it being
 	available in standard library
 	+ keep track of whether litdata filenames and userid were allocated or not,
 	and free memory in pgpv_close() if it was allocated
 	+ free up allocated resources which were allocated in pgpv_close()

 Update netpgpverify and libnetpgpverify to 20160706
 	+ 20160705 introduced a bug whereby a key subid would match and verify
 	fine, but, if formatted, would not display the correct subkey
 	information.  Fix to show the correct information in this case.

 Update netpgpverify and libnetpgpverify to 20160707 to fix some
 	unusual build errors shown by old gcc versions (works fine for
 	gcc-5.2.1 on ubuntu and gcc-5.3.0 on NetBSD 7.99.32)
 	+ use ULL suffix on unsigned 64bit constants, not UL
 	+ don't typedef the public structs twice - second time just define it
 	without the typedef
 	Fixes PR   51327

 Update netpgpverify and libnetpgpverify to 20160708
 	+ clear and free bignums properly - helps immensely with plugging
 	memory leaks

 Update netpgpverify and libnetpgpverify to 20160828
 	+ bring over change from christos in src/crypto to check for
 	the end of an ASCII-armored signature
 	+ no need for namespace protection in array.h any more, now
 	that netpgp/verify.h now contains opaque structures
 	+ minor typo clean-up in a definition (benign, ignored by compiler)

 update netpgpverify and libnetpgpverify to 20170201
 	+ make sure howmany() macro is defined
 	pointed out by cube - thanks!


 To generate a diff of this commit:
 cvs rdiff -u -r1.14 -r1.15 \
     src/crypto/external/bsd/netpgp/bin/netpgpverify/Makefile
 cvs rdiff -u -r1.6 -r1.7 \
     src/crypto/external/bsd/netpgp/dist/src/netpgpverify/Makefile.bsd
 cvs rdiff -u -r1.10 -r1.11 \
     src/crypto/external/bsd/netpgp/dist/src/netpgpverify/Makefile.in \
     src/crypto/external/bsd/netpgp/dist/src/netpgpverify/netpgpverify.1
 cvs rdiff -u -r1.1 -r1.2 \
     src/crypto/external/bsd/netpgp/dist/src/netpgpverify/Makefile.lib.in \
     src/crypto/external/bsd/netpgp/dist/src/netpgpverify/Makefile.libtool.in \
     src/crypto/external/bsd/netpgp/dist/src/netpgpverify/array.h
 cvs rdiff -u -r1.3 -r1.4 \
     src/crypto/external/bsd/netpgp/dist/src/netpgpverify/bignum.c \
     src/crypto/external/bsd/netpgp/dist/src/netpgpverify/libnetpgpverify.3
 cvs rdiff -u -r1.2 -r1.3 \
     src/crypto/external/bsd/netpgp/dist/src/netpgpverify/digest.c \
     src/crypto/external/bsd/netpgp/dist/src/netpgpverify/digest.h \
     src/crypto/external/bsd/netpgp/dist/src/netpgpverify/pgpsum.c
 cvs rdiff -u -r1.12 -r1.13 \
     src/crypto/external/bsd/netpgp/dist/src/netpgpverify/libverify.c
 cvs rdiff -u -r1.7 -r1.8 \
     src/crypto/external/bsd/netpgp/dist/src/netpgpverify/main.c
 cvs rdiff -u -r1.3 -r0 \
     src/crypto/external/bsd/netpgp/dist/src/netpgpverify/tiger.c
 cvs rdiff -u -r1.2 -r0 \
     src/crypto/external/bsd/netpgp/dist/src/netpgpverify/tiger.h
 cvs rdiff -u -r1.9 -r1.10 \
     src/crypto/external/bsd/netpgp/dist/src/netpgpverify/verify.h
 cvs rdiff -u -r1.8 -r1.9 src/crypto/external/bsd/netpgp/lib/verify/Makefile

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

>Unformatted:

Home	PR Database Search