From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Subject: bug#33733: Irrelevant narinfo signatures are honored Date: Thu, 13 Dec 2018 23:43:39 +0100 Message-ID: <875zvx9hes.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/signed; boundary="==-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:43452) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gXZj6-0000w9-3y for bug-guix@gnu.org; Thu, 13 Dec 2018 17:45:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gXZj4-00024V-8x for bug-guix@gnu.org; Thu, 13 Dec 2018 17:45:04 -0500 Received: from debbugs.gnu.org ([208.118.235.43]:42759) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gXZj4-00023o-2n for bug-guix@gnu.org; Thu, 13 Dec 2018 17:45:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1gXZj3-0008LY-P9 for bug-guix@gnu.org; Thu, 13 Dec 2018 17:45:01 -0500 Sender: "Debbugs-submit" Resent-Message-ID: Received: from eggs.gnu.org ([2001:4830:134:3::10]:43038) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gXZhv-0000LQ-OZ for bug-guix@gnu.org; Thu, 13 Dec 2018 17:43:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gXZhs-00086r-23 for bug-guix@gnu.org; Thu, 13 Dec 2018 17:43:50 -0500 Received: from fencepost.gnu.org ([2001:4830:134:3::e]:45616) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gXZhq-00085A-OJ for bug-guix@gnu.org; Thu, 13 Dec 2018 17:43:47 -0500 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=60836 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1gXZhq-0002za-AH for bug-guix@gnu.org; Thu, 13 Dec 2018 17:43:46 -0500 List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org Sender: "bug-Guix" To: 33733@debbugs.gnu.org --==-=-= Content-Type: multipart/mixed; boundary="=-=-=" --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hello Guix, =E2=80=98guix substitute=E2=80=99 checks the signature over everything that= precedes the =E2=80=9CSignature:=E2=80=9D field of a narinfo: (define (narinfo-sha256 narinfo) "Return the sha256 hash of NARINFO as a bytevector, or #f if NARINFO la= cks a 'Signature' field." (let ((contents (narinfo-contents narinfo))) (match (string-contains contents "Signature:") (#f #f) (index (let ((above-signature (string-take contents index))) ;<-- here! (sha256 (string->utf8 above-signature))))))) (define* (valid-narinfo? narinfo #:optional (acl (current-acl)) #:key verbose?) "Return #t if NARINFO's signature is not valid." (or %allow-unauthenticated-substitutes? (let ((hash (narinfo-sha256 narinfo)) ;<-- here! (signature (narinfo-signature narinfo)) (uri (uri->string (narinfo-uri narinfo)))) (and hash signature (signature-case (signature hash acl) =E2=80=A6))))) Narinfos produced by =E2=80=98guix publish=E2=80=99 look like this: =2D-8<---------------cut here---------------start------------->8--- StorePath: /gnu/store/nrkm1683p1cqnkcmhlmhiig9q9qd7xqh-sed-4.5 URL: nar/gzip/nrkm1683p1cqnkcmhlmhiig9q9qd7xqh-sed-4.5 Compression: gzip NarHash: sha256:17ymma68kh0l5hrsc6w1ma0ixb52lbwwm43wdhl0mm1q19j69s9n NarSize: 704040 References: 4sqps8d=E2=80=A6 FileSize: 240878 Signature: 1;berlin.guixsd.org;KHNp=E2=80=A6 =2D-8<---------------cut here---------------end--------------->8--- So =E2=80=98guix substitutes=E2=80=99 expects the signature to be computed = over everything. However, a server could well send this: =2D-8<---------------cut here---------------start------------->8--- Signature: 1;EVIL.example.org;ABCd=E2=80=A6 StorePath: /gnu/store/nrkm1683p1cqnkcmhlmhiig9q9qd7xqh-sed-4.5 URL: nar/gzip/nrkm1683p1cqnkcmhlmhiig9q9qd7xqh-sed-4.5 Compression: gzip NarHash: sha256:17ymma68kh0l5hrsc6w1ma0ixb52lbwwm43wdhl0mm1q19j69s9n NarSize: 704040 References: 4sqps8d=E2=80=A6 FileSize: 240878 =2D-8<---------------cut here---------------end--------------->8--- =E2=80=A6 in which case the signature is expected to be computed over the e= mpty string (thus it=E2=80=99s the same for all the narinfos it serves.) The problem is that =E2=80=98guix substitute=E2=80=99 will accept such nari= nfos (when they are signed by an authorized key), even though the signature doesn=E2= =80=99t cover the important parts (namely: StorePath, NarHash, and References; the rest is mostly informative.) A fix is attached with tests that illustrate the problem. I think the main consequence is repudiation: if you receive a narinfo where the signature comes first, that doesn=E2=80=99t prove anything; the s= erver operator could pretend it never sent it since in essence its contents are unsigned. It=E2=80=99s not clear to me whether/how this could be explo= ited. Also keep in mind that this is limited to servers with a key present in the user=E2=80=99s /etc/guix/acl (=E2=80=9Ctrusted=E2=80=9D servers.) In t= his context, servers are in a position to do more harm to the user anyway since they serve substitutes. TIA, Ludo=E2=80=99. PS: Thanks to Leo and Ricardo for their quick feedback on the guix-security mailing list! --=-=-= Content-Type: text/x-patch; charset=utf-8 Content-Disposition: inline; filename=0001-DRAFT-substitute-Ignore-irrelevant-narinfo-signature.patch Content-Transfer-Encoding: quoted-printable Content-Description: the fix From=20eb6f7aa5e57185acbe100eb21abb300f0cfb264b Mon Sep 17 00:00:00 2001 From: =3D?UTF-8?q?Ludovic=3D20Court=3DC3=3DA8s?=3D Date: Thu, 13 Dec 2018 19:45:47 +0100 Subject: [PATCH] DRAFT substitute: Ignore irrelevant narinfo signatures. Fixes XXX. Fixes a bug whereby 'guix substitute' would accept narinfos whose signature did not cover the StorePath/NarHash/References tuple. * guix/scripts/substitute.scm (narinfo-sha256)[%mandatory-fields]: New variable. Compute SIGNED-FIELDS; return #f unless each of the %MANDATORY-FIELDS is among SIGNED-FIELDS. * tests/substitute.scm ("query narinfo with signature over nothing") ("query narinfo with signature over irrelevant bits"): New tests. =2D-- guix/scripts/substitute.scm | 13 ++++++++++-- tests/substitute.scm | 42 ++++++++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/guix/scripts/substitute.scm b/guix/scripts/substitute.scm index d6dc9b6448..53b1777241 100755 =2D-- a/guix/scripts/substitute.scm +++ b/guix/scripts/substitute.scm @@ -392,12 +392,21 @@ No authentication and authorization checks are perfor= med here!" (define (narinfo-sha256 narinfo) "Return the sha256 hash of NARINFO as a bytevector, or #f if NARINFO lac= ks a 'Signature' field." + (define %mandatory-fields + ;; List of fields that must be signed. If they are not signed, the + ;; narinfo is considered unsigned. + '("StorePath" "NarHash" "References")) + (let ((contents (narinfo-contents narinfo))) (match (string-contains contents "Signature:") (#f #f) (index =2D (let ((above-signature (string-take contents index))) =2D (sha256 (string->utf8 above-signature))))))) + (let* ((above-signature (string-take contents index)) + (signed-fields (match (call-with-input-string above-signature + fields->alist) + (((fields . values) ...) fields)))) + (and (every (cut member <> signed-fields) %mandatory-fields) + (sha256 (string->utf8 above-signature)))))))) =20 (define* (valid-narinfo? narinfo #:optional (acl (current-acl)) #:key verbose?) diff --git a/tests/substitute.scm b/tests/substitute.scm index 964a57f30b..f4f2e9512d 100644 =2D-- a/tests/substitute.scm +++ b/tests/substitute.scm @@ -1,6 +1,6 @@ ;;; GNU Guix --- Functional package management for GNU ;;; Copyright =C2=A9 2014 Nikita Karetnikov =2D;;; Copyright =C2=A9 2014, 2015, 2017 Ludovic Court=C3=A8s +;;; Copyright =C2=A9 2014, 2015, 2017, 2018 Ludovic Court=C3=A8s ;;; ;;; This file is part of GNU Guix. ;;; @@ -211,6 +211,46 @@ a file for NARINFO." (lambda () (guix-substitute "--query")))))))) =20 +(test-equal "query narinfo with signature over nothing" + ;; The signature is computed over the empty string, not over the importa= nt + ;; parts, so the narinfo must be ignored. + "" + + (with-narinfo (string-append "Signature: " (signature-field "") "\n" + %narinfo "\n") + (string-trim-both + (with-output-to-string + (lambda () + (with-input-from-string (string-append "have " (%store-prefix) + "/aaaaaaaaaaaaaaaaaaaaaaaa= aaaaaaaa-foo") + (lambda () + (guix-substitute "--query")))))))) + +(test-equal "query narinfo with signature over irrelevant bits" + ;; The signature is valid but it does not cover the + ;; StorePath/NarHash/References tuple and is thus irrelevant; the narinfo + ;; must be ignored. + "" + + (let ((prefix (string-append "StorePath: " (%store-prefix) + "/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-foo +URL: example.nar +Compression: none\n"))) + (with-narinfo (string-append prefix + "Signature: " (signature-field prefix) " +NarHash: sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +NarSize: 42 +References: bar baz +Deriver: " (%store-prefix) "/foo.drv +System: mips64el-linux\n") + (string-trim-both + (with-output-to-string + (lambda () + (with-input-from-string (string-append "have " (%store-prefix) + "/aaaaaaaaaaaaaaaaaaaaaa= aaaaaaaaaa-foo") + (lambda () + (guix-substitute "--query"))))))))) + (test-equal "query narinfo signed with authorized key" (string-append (%store-prefix) "/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-foo") =20 =2D-=20 2.19.2 --=-=-=-- --==-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEPORkVYqE/cadtAz7CQsRmT2a67UFAlwS4JsACgkQCQsRmT2a 67XloQ//RhSzXGff8qBY1IaR2bdvLvcqZvs+OUwETft7PgdlSOJioKHcxwP/p1nZ YTdr0PJ6w6bLHzDfAXn8dGV60OvEgtoTzKLVY0n25Grzj7o3zynHZGAFkJOmFZLJ NDyy1DMesr7p5pEkjr7LiDX1uLIYNS1Nqat0yTF48K01Acx8Jux7n/gko/YDJnpx SgeMmX6MJo8y1RCD2tD1tRdNt2OH6efRkH6SXqGE+5uUl0mNAtaE52OfTVKSYEL4 /ieP/r8ycpHmlC9ylNeMOytrgSftflH9EwCbb8uAzPBzlOXtmOpTeMMS8dFnPB40 s9lSYhb49OQaZuo56TVAvskblHEg9Trs9bDxNoWubVt0eTQcgZLqkSUV6PHy0CgQ txAkSgptKZlpa7chXKqXK3ERNEmu0clUMQvxTF7FlhGvNFGWr7rInFVMwzNhGofb VK/iN5UHlWhS++arygo6W40deNzD8HN9PvsYRiEFUvpFOXSo5t9C8vNsbXDpE7oN aibroYirLzZrDbmkArgvBnn48gtFH4o84s1nK+AoeZNcLXmiLE4loZt3cYoyR7BK E+YIKcbP9Kl9EcK0AR7OxKMkiQK5BqbiPykfW9DC/LPXXac6omWykiZU9t7wiDRc 9AvVw4DjNKzTSn/h6bcvzRsHuQ+0gx12WkJjVmcLbWPkfl5j18o= =ZcH/ -----END PGP SIGNATURE----- --==-=-=--