From patchwork Fri Aug 14 05:39:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: AKASHI Takahiro X-Patchwork-Id: 247693 Delivered-To: patch@linaro.org Received: by 2002:a92:cc90:0:0:0:0:0 with SMTP id x16csp105080ilo; Thu, 13 Aug 2020 22:40:09 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyq+Sa4NLh9mrmsAEsTGVxKU3FdyJNYml8M08JqYGQETLRLnDiZImmGgQ9/l0x3RGPU0iVN X-Received: by 2002:a50:fd8d:: with SMTP id o13mr735639edt.313.1597383608912; Thu, 13 Aug 2020 22:40:08 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1597383608; cv=none; d=google.com; s=arc-20160816; b=LscqfgdJcq36Y6esE4cB39zQodRVqXxx+Bs3Z03n9ifDP5GgII2otghHD2AwKnugiW 5Mwg6EqTvaCoeT457gn/rEc47MkSnWv+O+9Q666d+MbnYaYwUPvh3xp8IJILYFY+3T97 G3Z4Sc06RcERyJ7krUh998RqpJdTxHSvn5acSqL3R3JNvptHqjASqizsXIQabsd4+0Gw hiXwA8Vd8LbSpwhE4HKDQraL02qmSMU74HLxd5AyHCYBKVUSrPgvcjz60sM0qFLhuJOI Ssq/jyd4IZNCQOqaSCffhq7nFat1gKYDaqUqokxUaaOlbNaf5nfDrwewsd4UBBHHAoeO gkNA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-transfer-encoding :mime-version:message-id:date:subject:cc:to:from:dkim-signature; bh=xT6lIU13QMS2Z3DpBv9RzqWm8ylC0i5UjzmPU4lE1Bg=; b=oUrIuhp85DIJGQllfqsUYIk29RTb/kpIsJfHjJYqe2ZVL5NvXN9LPFzzIQf4eDLdXu vpLtk3+2fYufqGpgjyW7NJJxi9psTGfASVM7k2w8pKIxDtaQpc4C1228WJzO9sGMMiaZ +3Uct3SbICZKvQloFxBNp/HJI2BU34bdz73QY01ovrr0RHHe1ln3/s00dl/yY+7UqGWR olwy+Ocu01vMfRBEXDLVNeb7qWYL0aLtjBp7p3bDA2vj8u/4op87g7dt/s1hILa9ny3I eFixVBUM36wBly80Almqd9NHSYDDdCQVKadklfPcjZyJwWR3FhlQbkcKK9evus4eyglm 4f7g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=w4hn3wa1; spf=pass (google.com: domain of u-boot-bounces@lists.denx.de designates 2a01:238:438b:c500:173d:9f52:ddab:ee01 as permitted sender) smtp.mailfrom=u-boot-bounces@lists.denx.de; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from phobos.denx.de (phobos.denx.de. [2a01:238:438b:c500:173d:9f52:ddab:ee01]) by mx.google.com with ESMTPS id q25si4824308edr.402.2020.08.13.22.40.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Aug 2020 22:40:08 -0700 (PDT) Received-SPF: pass (google.com: domain of u-boot-bounces@lists.denx.de designates 2a01:238:438b:c500:173d:9f52:ddab:ee01 as permitted sender) client-ip=2a01:238:438b:c500:173d:9f52:ddab:ee01; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=w4hn3wa1; spf=pass (google.com: domain of u-boot-bounces@lists.denx.de designates 2a01:238:438b:c500:173d:9f52:ddab:ee01 as permitted sender) smtp.mailfrom=u-boot-bounces@lists.denx.de; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id B4D1B812FE; Fri, 14 Aug 2020 07:40:04 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="w4hn3wa1"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 91D9A81A2A; Fri, 14 Aug 2020 07:40:00 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on phobos.denx.de X-Spam-Level: X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,SPF_HELO_NONE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from mail-pj1-x1043.google.com (mail-pj1-x1043.google.com [IPv6:2607:f8b0:4864:20::1043]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 58BAD808F1 for ; Fri, 14 Aug 2020 07:39:57 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=takahiro.akashi@linaro.org Received: by mail-pj1-x1043.google.com with SMTP id mw10so3866840pjb.2 for ; Thu, 13 Aug 2020 22:39:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=xT6lIU13QMS2Z3DpBv9RzqWm8ylC0i5UjzmPU4lE1Bg=; b=w4hn3wa1rfaLNVUHTGmFqSw7omlxf7q/XPbmcXC6AZYVsEny2SH+DFffBIaGXK5Ds8 ivhDHcCxz6CvYIhSKrPQLBmJ1aeRvQSJceR21OrpB5KOCM8ELwpjjjyJC46yuphV8tLb sDdcNqZeo9X6HKqyWD8/DJLwZYyNLLL5buAZO2dkXzbXySfWEPtFrb7YMSwvcpab03kS 57dm7na8h50JqScsCEUCV+ARgDW++wfWbX4UqrfmcuTAUM89MGZ1Qg7dyTUsLgM3BwOS YdBd1aPSLYKbnxle1WWKgdUYMx0pGpGc1EQWbbzUn94xHTNwM8U0svXH0pFpwFiOkaXs PVMg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=xT6lIU13QMS2Z3DpBv9RzqWm8ylC0i5UjzmPU4lE1Bg=; b=oDTHUIYWkgLQr90nFWJJHbZwJ4sfP4dpzXnJeWI9tmuKXD9s+TkgT4Ro7uvxEdhxf0 c8JVafznBwftFHm/gMCVd8fUkmrjZPVI97C5BUjY8uFqzODw+KL8IUSwsZg1BxYbMAAu Dh5IX5sgb6qVnHhkyHVCpzufyvJs1QphIq5FWV0IFoM3pjvySMUCNTosNUTUTZ2/3AuD NKNTzdcqVAqI+ZETiPrMrUhNZP62by6WKVZuGpid/UL9BQaiV0MiVVoYqu9IGRHmsbz9 R3FOmvSQSbEU1D1j0eNoE4KFMX3Kdp9hA2wSkcSdwZk+/uXAiRFxbPSTOss2y9sTUlPY OWEQ== X-Gm-Message-State: AOAM532YJ6fFaco4l60ZblcZdb65fQYVj12+uarL+0IrJ+QepkLLyg5Q pxgcV1+ZZyf0jIyKPfwLZIlEXQ== X-Received: by 2002:a17:902:c203:: with SMTP id 3mr929164pll.121.1597383595327; Thu, 13 Aug 2020 22:39:55 -0700 (PDT) Received: from localhost.localdomain (p784a66b9.tkyea130.ap.so-net.ne.jp. [120.74.102.185]) by smtp.gmail.com with ESMTPSA id f89sm7402691pjg.5.2020.08.13.22.39.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Aug 2020 22:39:54 -0700 (PDT) From: AKASHI Takahiro To: xypron.glpk@gmx.de, agraf@csgraf.de Cc: u-boot@lists.denx.de, AKASHI Takahiro Subject: [PATCH 1/2] efi_loader: signature: correct a behavior against multiple signatures Date: Fri, 14 Aug 2020 14:39:23 +0900 Message-Id: <20200814053924.342094-1-takahiro.akashi@linaro.org> X-Mailer: git-send-email 2.28.0 MIME-Version: 1.0 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.102.3 at phobos.denx.de X-Virus-Status: Clean Under the current implementation, all the signatures, if any, in a signed image must be verified before loading it. Meanwhile, UEFI specification v2.8b section 32.5.3.3 says, Multiple signatures are allowed to exist in the binary’s certificate table (as per PE/COFF Section “Attribute Certificate Table”). Only one hash or signature is required to be present in db in order to pass validation, so long as neither the SHA-256 hash of the binary nor any present signature is reflected in dbx. This patch makes the semantics of signature verification compliant with the specification mentioned above. Signed-off-by: AKASHI Takahiro Reported-by: Heinrich Schuchardt --- include/efi_loader.h | 9 ++-- lib/efi_loader/efi_image_loader.c | 33 +++++++------- lib/efi_loader/efi_signature.c | 76 +++---------------------------- 3 files changed, 30 insertions(+), 88 deletions(-) -- 2.28.0 diff --git a/include/efi_loader.h b/include/efi_loader.h index df8dc377257c..ae01e909b56c 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -770,13 +770,16 @@ struct pkcs7_message; bool efi_signature_lookup_digest(struct efi_image_regions *regs, struct efi_signature_store *db); -bool efi_signature_verify_one(struct efi_image_regions *regs, - struct pkcs7_message *msg, - struct efi_signature_store *db); bool efi_signature_verify(struct efi_image_regions *regs, struct pkcs7_message *msg, struct efi_signature_store *db, struct efi_signature_store *dbx); +static inline bool efi_signature_verify_one(struct efi_image_regions *regs, + struct pkcs7_message *msg, + struct efi_signature_store *db) +{ + return efi_signature_verify(regs, msg, db, NULL); +} bool efi_signature_check_signers(struct pkcs7_message *msg, struct efi_signature_store *dbx); diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index 832bce939403..eea42cc20436 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -546,6 +546,11 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) goto err; } + if (efi_signature_lookup_digest(regs, dbx)) { + EFI_PRINT("Image's digest was found in \"dbx\"\n"); + goto err; + } + /* * go through WIN_CERTIFICATE list * NOTE: @@ -553,10 +558,9 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) * in PE header, or as pkcs7 SignerInfo's in SignedData. * So the verification policy here is: * - Success if, at least, one of signatures is verified - * - unless - * any of signatures is rejected explicitly, or - * none of digest algorithms are supported + * - unless signature is rejected explicitly with its digest. */ + for (wincert = wincerts, wincerts_end = (u8 *)wincerts + wincerts_len; (u8 *)wincert < wincerts_end; wincert = (WIN_CERTIFICATE *) @@ -627,32 +631,29 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) /* try black-list first */ if (efi_signature_verify_one(regs, msg, dbx)) { EFI_PRINT("Signature was rejected by \"dbx\"\n"); - goto err; + continue; } if (!efi_signature_check_signers(msg, dbx)) { EFI_PRINT("Signer(s) in \"dbx\"\n"); - goto err; - } - - if (efi_signature_lookup_digest(regs, dbx)) { - EFI_PRINT("Image's digest was found in \"dbx\"\n"); - goto err; + continue; } /* try white-list */ - if (efi_signature_verify(regs, msg, db, dbx)) - continue; + if (efi_signature_verify(regs, msg, db, dbx)) { + ret = true; + break; + } debug("Signature was not verified by \"db\"\n"); - if (efi_signature_lookup_digest(regs, db)) - continue; + if (efi_signature_lookup_digest(regs, db)) { + ret = true; + break; + } debug("Image's digest was not found in \"db\" or \"dbx\"\n"); - goto err; } - ret = true; err: efi_sigstore_free(db); diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c index 7b33a4010fe8..a8da2496360d 100644 --- a/lib/efi_loader/efi_signature.c +++ b/lib/efi_loader/efi_signature.c @@ -175,6 +175,8 @@ static bool efi_lookup_certificate(struct x509_certificate *cert, if (IS_ERR_OR_NULL(cert_tmp)) continue; + EFI_PRINT("%s: against %s\n", __func__, + cert_tmp->subject); reg[0].data = cert_tmp->tbs; reg[0].size = cert_tmp->tbs_size; if (!efi_hash_regions(reg, 1, &hash_tmp, NULL)) @@ -267,7 +269,7 @@ out: * protocol at this time and any image will be unconditionally revoked * when this match occurs. * - * Return: true if check passed, false otherwise. + * Return: true if check passed (not found), false otherwise. */ static bool efi_signature_check_revocation(struct pkcs7_signed_info *sinfo, struct x509_certificate *cert, @@ -337,70 +339,6 @@ out: return !revoked; } -/** - * efi_signature_verify_one - verify signatures with database - * @regs: List of regions to be authenticated - * @msg: Signature - * @db: Signature database - * - * All the signature pointed to by @msg against image pointed to by @regs - * will be verified by signature database pointed to by @db. - * - * Return: true if verification for one of signatures passed, false - * otherwise - */ -bool efi_signature_verify_one(struct efi_image_regions *regs, - struct pkcs7_message *msg, - struct efi_signature_store *db) -{ - struct pkcs7_signed_info *sinfo; - struct x509_certificate *signer; - bool verified = false; - int ret; - - EFI_PRINT("%s: Enter, %p, %p, %p\n", __func__, regs, msg, db); - - if (!db) - goto out; - - if (!db->sig_data_list) - goto out; - - EFI_PRINT("%s: Verify signed image with db\n", __func__); - for (sinfo = msg->signed_infos; sinfo; sinfo = sinfo->next) { - EFI_PRINT("Signed Info: digest algo: %s, pkey algo: %s\n", - sinfo->sig->hash_algo, sinfo->sig->pkey_algo); - - EFI_PRINT("Verifying certificate chain\n"); - signer = NULL; - ret = pkcs7_verify_one(msg, sinfo, &signer); - if (ret == -ENOPKG) - continue; - - if (ret < 0 || !signer) - goto out; - - if (sinfo->blacklisted) - continue; - - EFI_PRINT("Verifying last certificate in chain\n"); - if (signer->self_signed) { - if (efi_lookup_certificate(signer, db)) { - verified = true; - goto out; - } - } else if (efi_verify_certificate(signer, db, NULL)) { - verified = true; - goto out; - } - EFI_PRINT("Valid certificate not in db\n"); - } - -out: - EFI_PRINT("%s: Exit, verified: %d\n", __func__, verified); - return verified; -} - /* * efi_signature_verify - verify signatures with db and dbx * @regs: List of regions to be authenticated @@ -463,7 +401,7 @@ bool efi_signature_verify(struct efi_image_regions *regs, if (efi_lookup_certificate(signer, db)) if (efi_signature_check_revocation(sinfo, signer, dbx)) - continue; + break; } else if (efi_verify_certificate(signer, db, &root)) { bool check; @@ -471,13 +409,13 @@ bool efi_signature_verify(struct efi_image_regions *regs, dbx); x509_free_certificate(root); if (check) - continue; + break; } EFI_PRINT("Certificate chain didn't reach trusted CA\n"); - goto out; } - verified = true; + if (sinfo) + verified = true; out: EFI_PRINT("%s: Exit, verified: %d\n", __func__, verified); return verified;