From patchwork Mon Jun 21 08:26:37 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Axtens X-Patchwork-Id: 1494940 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ozlabs.org (client-ip=2404:9400:2:0:216:3eff:fee1:b9f1; helo=lists.ozlabs.org; envelope-from=skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=axtens.net header.i=@axtens.net header.a=rsa-sha256 header.s=google header.b=GriMrQp0; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2404:9400:2:0:216:3eff:fee1:b9f1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4G7jLW2H84z9sWQ for ; Mon, 21 Jun 2021 18:26:59 +1000 (AEST) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4G7jLW6bdtz3083 for ; Mon, 21 Jun 2021 18:26:59 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=axtens.net header.i=@axtens.net header.a=rsa-sha256 header.s=google header.b=GriMrQp0; dkim-atps=neutral X-Original-To: skiboot@lists.ozlabs.org Delivered-To: skiboot@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=axtens.net (client-ip=2607:f8b0:4864:20::52e; helo=mail-pg1-x52e.google.com; envelope-from=dja@axtens.net; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=axtens.net header.i=@axtens.net header.a=rsa-sha256 header.s=google header.b=GriMrQp0; dkim-atps=neutral Received: from mail-pg1-x52e.google.com (mail-pg1-x52e.google.com [IPv6:2607:f8b0:4864:20::52e]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4G7jLP5WJDz2xxq for ; Mon, 21 Jun 2021 18:26:53 +1000 (AEST) Received: by mail-pg1-x52e.google.com with SMTP id t9so13547066pgn.4 for ; Mon, 21 Jun 2021 01:26:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=axtens.net; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=xILOCbPTOFeSZ7M1DRFX32FQPXJRxv42ByTjDlLGyZg=; b=GriMrQp0MLy35pRA6Z0Enyje4rWcI/TPtVx9wAneTcM8NX/TJY0rOVzyLOTfrYsqRi FwTHXBKrCET2XsmIxEjGNoFJyGcaw+rODGl494iGnFFz2u5SjsEALIn+KIDppcIsVpsd lZ3z9r8BVkVAaT8VHNa+pXNZaBb+lZiRMkGRs= 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=xILOCbPTOFeSZ7M1DRFX32FQPXJRxv42ByTjDlLGyZg=; b=jrRwHsSdqWHNosfHo+xR8qLXBzKtOjC9X5NT5dVhu8TS8JsnNwvbgTny/FLKHaXGvv JVd+Xx+otWDIup0yMV+nxdTt6ym93iE8BKnzjkCEr+FOD11fiZUwHCkc28QZ16sQ+L3N OHL6kb16uIPpQ9IzYG/DHsjTGHRf/tuUCO3Uagts2iX2FfZy1f2U1WFl489SUNGtAI5U lpwCj+lynINRVWPaxMRyJjnagffbbyrBJtbaonoLKq3QKzsM1SCZ7zKjSo+27wJuqkHj hvJUrzFf+SJxWv79cXiL/1QuxEDIAd/RYnHYblsmnUcXW8syiQOdFUW9MBp6oPjOLfi1 /y9A== X-Gm-Message-State: AOAM531GGqZZwLHwtuDPvL5Ym+d7dzRhCVpQ29nn9zLbbnu9YJusDqpY fjI8eWKhcixjoYKn2lBWFAen/CB+ynNOHQ== X-Google-Smtp-Source: ABdhPJz92L4Q97OXmjv1cbkp6jyc6F+4tlOUJvHYC5VRPt2nbU/EMfgFGYc8n8AZ8DJ8s60KCfP74g== X-Received: by 2002:aa7:829a:0:b029:2e9:e53:198d with SMTP id s26-20020aa7829a0000b02902e90e53198dmr18580035pfm.72.1624264009199; Mon, 21 Jun 2021 01:26:49 -0700 (PDT) Received: from localhost ([203.206.29.204]) by smtp.gmail.com with ESMTPSA id r128sm2773563pfc.138.2021.06.21.01.26.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Jun 2021 01:26:48 -0700 (PDT) From: Daniel Axtens To: skiboot@lists.ozlabs.org Date: Mon, 21 Jun 2021 18:26:37 +1000 Message-Id: <20210621082641.26476-2-dja@axtens.net> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210621082641.26476-1-dja@axtens.net> References: <20210621082641.26476-1-dja@axtens.net> MIME-Version: 1.0 Subject: [Skiboot] [PATCH v2 1/5] secvar/backend: rename verify_signature parameters X-BeenThere: skiboot@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list for skiboot development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: nick.child@ibm.com, nayna@linux.ibm.com Errors-To: skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Skiboot" verify_signature() currently takes newcert and new_data_len. However, these variables are used only as parameters to mbedtls_pkcs7_signed_hash_verify() where they represent a hash value and the length of the hash value. verify_signature() is static, and the only caller of the function is process_update(). process_update() passes in tbhbuffer and tbhbuffersize. Those are unfortunate names too - because the data that process_update() passes in is not a to-be-hashed buffer, but a hash. We'll fix that later. Call the parameters hash and hash_len. Signed-off-by: Daniel Axtens Reviewed-by: Nick Child Tested-by: Nick Child --- libstb/secvar/backend/edk2-compat-process.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libstb/secvar/backend/edk2-compat-process.c b/libstb/secvar/backend/edk2-compat-process.c index 244f23403fe0..8324dc068b8e 100644 --- a/libstb/secvar/backend/edk2-compat-process.c +++ b/libstb/secvar/backend/edk2-compat-process.c @@ -455,7 +455,7 @@ out: /* Verify the PKCS7 signature on the signed data. */ static int verify_signature(const struct efi_variable_authentication_2 *auth, - const char *newcert, const size_t new_data_size, + const char *hash, const size_t hash_len, const struct secvar *avar) { mbedtls_pkcs7 *pkcs7 = NULL; @@ -534,7 +534,7 @@ static int verify_signature(const struct efi_variable_authentication_2 *auth, free(x509_buf); x509_buf = NULL; - rc = mbedtls_pkcs7_signed_hash_verify(pkcs7, &x509, newcert, new_data_size); + rc = mbedtls_pkcs7_signed_hash_verify(pkcs7, &x509, hash, hash_len); /* If you find a signing certificate, you are done */ if (rc == 0) { From patchwork Mon Jun 21 08:26:38 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Axtens X-Patchwork-Id: 1494941 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ozlabs.org (client-ip=112.213.38.117; helo=lists.ozlabs.org; envelope-from=skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=axtens.net header.i=@axtens.net header.a=rsa-sha256 header.s=google header.b=qbXcRVra; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4G7jLc0r9Mz9sWM for ; Mon, 21 Jun 2021 18:27:04 +1000 (AEST) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4G7jLc5XNHz307j for ; Mon, 21 Jun 2021 18:27:04 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=axtens.net header.i=@axtens.net header.a=rsa-sha256 header.s=google header.b=qbXcRVra; dkim-atps=neutral X-Original-To: skiboot@lists.ozlabs.org Delivered-To: skiboot@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=axtens.net (client-ip=2607:f8b0:4864:20::533; helo=mail-pg1-x533.google.com; envelope-from=dja@axtens.net; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=axtens.net header.i=@axtens.net header.a=rsa-sha256 header.s=google header.b=qbXcRVra; dkim-atps=neutral Received: from mail-pg1-x533.google.com (mail-pg1-x533.google.com [IPv6:2607:f8b0:4864:20::533]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4G7jLS4Gldz306l for ; Mon, 21 Jun 2021 18:26:56 +1000 (AEST) Received: by mail-pg1-x533.google.com with SMTP id h4so2028081pgp.5 for ; Mon, 21 Jun 2021 01:26:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=axtens.net; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=fBcrnEZOuyQJBq5fto7gzZXK3LLMRpzh8rCu+iS+PLw=; b=qbXcRVrajInXko4tF28oiLdHIiX2QmZbbWYZL8HScGKa/d42jQk6Lh2pITf/eVHuq1 GeGBIjRu52EpOzgB+mymUQi1PwFSTiF/KkhS3r2tKYqPFXVyFd5ECmRXShAe+AQnMt/X iRTzKpzELJNCtZCsTFiT37/ZD30WFplM0ZadI= 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=fBcrnEZOuyQJBq5fto7gzZXK3LLMRpzh8rCu+iS+PLw=; b=cMqXazNukdbki3d6HUNowVliGwbduyq3UwXtvF5Ntro+AlpNWmeV1pfsP1uKEr9Scl e1Kyo0H/c7UQD4cvTt1tf+G3QdGOUaDwA2PGDe0GERu6SYCl3QEWwhmbEPwn838vr0mF fFgIlsyob1oOoY7kLcBGP9VPUnz3U3u1TxLk+wTDMd6HLMJdlznUQ3K3a6qbqJUn1nhi famtNJvj3HQkuEEqq1APOdVnJKkMOwH2bG6kynjtZP5NBwDkucSEOT7ZBu9Ku4TR/AYU opeOwpanu0ZILgnIFtng4ikBn6nHQG5SE4IageT2MUmh2/5PHquuoN+YEG9voC1+/3/p SCXg== X-Gm-Message-State: AOAM5329r/iIqsj6uD7bHKK5l7l+nK8FYTHjcN+9eIT0zJs1IpU9OqHr WDxhPAlA0ohNSecULprOnmxmZIFbh5Auhg== X-Google-Smtp-Source: ABdhPJxgdFF/pOLLMIBILekVMYyoO093F4iqogEAi/I3vIA1nQmRKg1L4zasQbGVfLz5bb7YBV+8jg== X-Received: by 2002:aa7:8881:0:b029:2f8:5004:9523 with SMTP id z1-20020aa788810000b02902f850049523mr18461026pfe.31.1624264013178; Mon, 21 Jun 2021 01:26:53 -0700 (PDT) Received: from localhost ([203.206.29.204]) by smtp.gmail.com with ESMTPSA id r10sm16622501pga.48.2021.06.21.01.26.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Jun 2021 01:26:52 -0700 (PDT) From: Daniel Axtens To: skiboot@lists.ozlabs.org Date: Mon, 21 Jun 2021 18:26:38 +1000 Message-Id: <20210621082641.26476-3-dja@axtens.net> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210621082641.26476-1-dja@axtens.net> References: <20210621082641.26476-1-dja@axtens.net> MIME-Version: 1.0 Subject: [Skiboot] [PATCH v2 2/5] secvar/backend: clarify variables in process_update X-BeenThere: skiboot@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list for skiboot development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: nick.child@ibm.com, nayna@linux.ibm.com Errors-To: skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Skiboot" process_update() has tbhbuffer and tbhbuffersize. However, tbhbuffer doesn't contain to-be-hashed data, but a hash: /* Prepare the data to be verified */ tbhbuffer = get_hash_to_verify(update->key, *newesl, *new_data_size, timestamp); And tbhbuffersize is initalised to zero and then never filled with the actual length, so 0 is passed through to verify_signature. verify_signature will in turn pass that to mbedtls, which will interpret it as "figure out the length yourself based on the hash type". So this has always worked, but by accident. Rename tbhbuffer to hash, as that's what it is. Drop tbhbuffersize, and pass 32 directly to verify_signature. We only support SHA-256, and SHA-256 is 32 bytes long. Signed-off-by: Daniel Axtens Reviewed-by: Nick Child Tested-by: Nick Child --- libstb/secvar/backend/edk2-compat-process.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/libstb/secvar/backend/edk2-compat-process.c b/libstb/secvar/backend/edk2-compat-process.c index 8324dc068b8e..b39f35e0c3dc 100644 --- a/libstb/secvar/backend/edk2-compat-process.c +++ b/libstb/secvar/backend/edk2-compat-process.c @@ -662,8 +662,7 @@ int process_update(const struct secvar *update, char **newesl, void *auth_buffer = NULL; int auth_buffer_size = 0; const char *key_authority[3]; - char *tbhbuffer = NULL; - size_t tbhbuffersize = 0; + char *hash = NULL; struct secvar *avar = NULL; int rc = 0; int i; @@ -723,9 +722,9 @@ int process_update(const struct secvar *update, char **newesl, } /* Prepare the data to be verified */ - tbhbuffer = get_hash_to_verify(update->key, *newesl, *new_data_size, + hash = get_hash_to_verify(update->key, *newesl, *new_data_size, timestamp); - if (!tbhbuffer) { + if (!hash) { rc = OPAL_INTERNAL_ERROR; goto out; } @@ -746,9 +745,8 @@ int process_update(const struct secvar *update, char **newesl, if (!avar || !avar->data_size) continue; - /* Verify the signature */ - rc = verify_signature(auth, tbhbuffer, tbhbuffersize, - avar); + /* Verify the signature. sha256 is 32 bytes long. */ + rc = verify_signature(auth, hash, 32, avar); /* Break if signature verification is successful */ if (rc == OPAL_SUCCESS) { @@ -759,7 +757,7 @@ int process_update(const struct secvar *update, char **newesl, out: free(auth_buffer); - free(tbhbuffer); + free(hash); return rc; } From patchwork Mon Jun 21 08:26:39 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Axtens X-Patchwork-Id: 1494942 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ozlabs.org (client-ip=112.213.38.117; helo=lists.ozlabs.org; envelope-from=skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=axtens.net header.i=@axtens.net header.a=rsa-sha256 header.s=google header.b=kmJWQYSo; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4G7jLj5vKRz9sWQ for ; Mon, 21 Jun 2021 18:27:09 +1000 (AEST) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4G7jLk3QSmz306c for ; Mon, 21 Jun 2021 18:27:10 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=axtens.net header.i=@axtens.net header.a=rsa-sha256 header.s=google header.b=kmJWQYSo; dkim-atps=neutral X-Original-To: skiboot@lists.ozlabs.org Delivered-To: skiboot@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=axtens.net (client-ip=2607:f8b0:4864:20::1035; helo=mail-pj1-x1035.google.com; envelope-from=dja@axtens.net; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=axtens.net header.i=@axtens.net header.a=rsa-sha256 header.s=google header.b=kmJWQYSo; dkim-atps=neutral Received: from mail-pj1-x1035.google.com (mail-pj1-x1035.google.com [IPv6:2607:f8b0:4864:20::1035]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4G7jLX37tfz3089 for ; Mon, 21 Jun 2021 18:27:00 +1000 (AEST) Received: by mail-pj1-x1035.google.com with SMTP id k5so9506942pjj.1 for ; Mon, 21 Jun 2021 01:26:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=axtens.net; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=/aEQ9xzBWtBAxoxnmVlXOq3xzLR1sUKZjUnxGiDhrAU=; b=kmJWQYSoR5bd8enSDtUYl94+ax6I84ZmS4CE2/hYRmwfibr0iDFSLMl25m9AxRGeVt MfyGjxDUwQhAisSD5VkPcfvrLGKtWbnqmkBFtRHrPc2GIYDuDOZ0aVzC5UXnJdBiKmx1 jT9iWx5TsVEtkiK4Ai5qIXpSy/cnNGUDCNkZc= 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=/aEQ9xzBWtBAxoxnmVlXOq3xzLR1sUKZjUnxGiDhrAU=; b=HomWgUIyD5Mb+NZhXTrLO9kmaFXKXZRxM0uP2xl0bb/jaIeAADARUE/Sqjwee6gp3H yRuv8wnzlbJ/zr1bZ+ZJlfYqJDM+3RfWlqTCGI4N4aMBzgJJZcolOKa46FPoqwY6EheL c5WWxFAtXJb5VotkgzIuPkRS8R/fKrj7U8diIrjc/dglRxxbTdirsCnkS8w2uF6PA/kS ArFtrfA/msTOmXicVgYTyLczDWBeiu5KxuTm/Vt23EVIpzchKYfwIlgo8fXDbJtPShsB CitvkRcKJ954aoIHksr5fuvwqOlJBdL9i+a/SfMqTeEWaSIWcz7MgbhOZPec36937Ppl aBxg== X-Gm-Message-State: AOAM530VaM1aDvipIdcmo1ejiC403+Dph3ciYU+CKPMcEaI9y3Det2fz EbKQsAqUqbF/te6VC/LAphkh5IzalcJOLw== X-Google-Smtp-Source: ABdhPJyKEKkpQ74pSSwz8ZsxLclOlvT9OOiWeftmswoHQzEDRz9akeQkD74/WC6EQo/KOT+3VWhynQ== X-Received: by 2002:a17:902:6b82:b029:120:3404:ce99 with SMTP id p2-20020a1709026b82b02901203404ce99mr16899963plk.49.1624264017168; Mon, 21 Jun 2021 01:26:57 -0700 (PDT) Received: from localhost ([203.206.29.204]) by smtp.gmail.com with ESMTPSA id d189sm7684826pfa.28.2021.06.21.01.26.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Jun 2021 01:26:56 -0700 (PDT) From: Daniel Axtens To: skiboot@lists.ozlabs.org Date: Mon, 21 Jun 2021 18:26:39 +1000 Message-Id: <20210621082641.26476-4-dja@axtens.net> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210621082641.26476-1-dja@axtens.net> References: <20210621082641.26476-1-dja@axtens.net> MIME-Version: 1.0 Subject: [Skiboot] [PATCH v2 3/5] secvar/backend: fix comment of get_hash_to_verify X-BeenThere: skiboot@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list for skiboot development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: nick.child@ibm.com, nayna@linux.ibm.com Errors-To: skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Skiboot" get_hash_to_verify claims to return a negative error code in the case of error. It actually returns NULL. Fix the comment. Signed-off-by: Daniel Axtens Reviewed-by: Nick Child --- libstb/secvar/backend/edk2-compat-process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libstb/secvar/backend/edk2-compat-process.c b/libstb/secvar/backend/edk2-compat-process.c index b39f35e0c3dc..3ef3cf73c8f7 100644 --- a/libstb/secvar/backend/edk2-compat-process.c +++ b/libstb/secvar/backend/edk2-compat-process.c @@ -572,7 +572,7 @@ static int verify_signature(const struct efi_variable_authentication_2 *auth, * Create the hash of the buffer * name || vendor guid || attributes || timestamp || newcontent * which is submitted as signed by the user. - * Returns the sha256 hash, else negative error code. + * Returns the sha256 hash, else NULL. */ static char *get_hash_to_verify(const char *key, const char *new_data, const size_t new_data_size, From patchwork Mon Jun 21 08:26:40 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Axtens X-Patchwork-Id: 1494943 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ozlabs.org (client-ip=112.213.38.117; helo=lists.ozlabs.org; envelope-from=skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=axtens.net header.i=@axtens.net header.a=rsa-sha256 header.s=google header.b=fJY16qmS; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4G7jLp13xTz9sWM for ; Mon, 21 Jun 2021 18:27:14 +1000 (AEST) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4G7jLp5lSbz306r for ; Mon, 21 Jun 2021 18:27:14 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=axtens.net header.i=@axtens.net header.a=rsa-sha256 header.s=google header.b=fJY16qmS; dkim-atps=neutral X-Original-To: skiboot@lists.ozlabs.org Delivered-To: skiboot@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=axtens.net (client-ip=2607:f8b0:4864:20::636; helo=mail-pl1-x636.google.com; envelope-from=dja@axtens.net; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=axtens.net header.i=@axtens.net header.a=rsa-sha256 header.s=google header.b=fJY16qmS; dkim-atps=neutral Received: from mail-pl1-x636.google.com (mail-pl1-x636.google.com [IPv6:2607:f8b0:4864:20::636]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4G7jLc69d7z2ysk for ; Mon, 21 Jun 2021 18:27:04 +1000 (AEST) Received: by mail-pl1-x636.google.com with SMTP id c15so8054000pls.13 for ; Mon, 21 Jun 2021 01:27:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=axtens.net; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=nhpT/gyILvUvoPwtNO9BRP2jw4PHEL4hWKtsG5rMn1I=; b=fJY16qmSkz5JlQIjp0q/oKROXUNxBCmean7WHqA5rk+YjiLO/BZg9wBzUEFVGk/lUM O2lIsBK7A8rBHTIZyXJTIaWHVDEBFXR0npelTuqjr/dgKwGadRG2BFUc99hkauFBYjEk nRbFIYSjKYyLtiOG3AifCuQC+b9rh3XurQW0E= 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=nhpT/gyILvUvoPwtNO9BRP2jw4PHEL4hWKtsG5rMn1I=; b=KkMF8zccozGP+ZCF4cNbaMT8ZHtmn74yg8OvzzbShAiX0Mjy3kBxnz+eJmO/2ohfOu iuZrK8Qu8Aqf2g0V+1BxNgiDZ4Cmk0Hnm6zRi5ZMZeGKy0og9MI4HM8FNj9UdDIrrECU 8rvx0zFYLVrvdDjlLb1cefvdRzm+Mdsx+b+KmNwsUWKXuSQoLygOAes3v0VS+CKIdTPd X/Vc7V+jXe9lRLZJVaJwFuiD+iKM3A5hYrAbHIjgpJpS2LtLUOf97w8q/Y126Xg8JSgv Lvp6Ts0ohIwQcjKGyIMBbHnhpqG9vN+nbWNtZCBIff+hJPYsRXO1r5IFNJjOEzCUobHS ItTQ== X-Gm-Message-State: AOAM531FRxpPMPMQWCnAixnCq15kgBJ74sveOxqS97On4W94eFDIyf3B BvBCjQiCYfZNfNbJMN3Yx4JfhYafwgCVXA== X-Google-Smtp-Source: ABdhPJwjJWdBedYcOjKEUAmftVnitGvIl43nmR93+VoLo+GJXuYN3dAlv7aG0Y5hGjlXhDKc/t6l2A== X-Received: by 2002:a17:902:d909:b029:11d:65e5:ac34 with SMTP id c9-20020a170902d909b029011d65e5ac34mr17053432plz.83.1624264021292; Mon, 21 Jun 2021 01:27:01 -0700 (PDT) Received: from localhost ([203.206.29.204]) by smtp.gmail.com with ESMTPSA id ne11sm1315873pjb.40.2021.06.21.01.27.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Jun 2021 01:27:01 -0700 (PDT) From: Daniel Axtens To: skiboot@lists.ozlabs.org Date: Mon, 21 Jun 2021 18:26:40 +1000 Message-Id: <20210621082641.26476-5-dja@axtens.net> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210621082641.26476-1-dja@axtens.net> References: <20210621082641.26476-1-dja@axtens.net> MIME-Version: 1.0 Subject: [Skiboot] [PATCH v2 4/5] secvar/backend: redo hash algorithm handling for auth structures X-BeenThere: skiboot@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list for skiboot development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: nick.child@ibm.com, nayna@linux.ibm.com Errors-To: skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Skiboot" We only handle sha256 hashes in auth structures. In the process of verifying an auth structure, we extract the pkcs7 message and we calculate the hopefully-matching hash, which is sha256(name || vendor guid || attributes || timestamp || newcontent) We then verify that the PKCS#7 signature matches that calculated hash. However, at no point do we check that the PKCS#7 hash algorithm is sha256. So if the PKCS#7 message says that it is a signature on a sha512, mbedtls will compare 64 bytes of hash from the signature with 64 bytes from our hash, resulting - if I've got the mbedtls flow correct - in a 32 byte overread. - Verify that the hash algorithm in the PKCS#7 message is sha256. [there is an additional complexity in the spec which our code doesn't handle. the digest algorithms are specified in 2 places: DigestAlgorithmIdentifiers, and SignerInfos..digestAlgorithm. Having them not match would be a spec violation, but nonetheless fairly trivial to construct. However, mbedtls_pkcs7_signed_hash_verify considers only the first element of DigestAlgorithmIdentifiers, so this is safe for now at least.] - Stop passing hash lengths around, they're constant. - Pass 32 into mbedtls as the hash length. I contemplated using 0, which is code for "just use what the hash algorithm says the length is", but I'm worried about what might happen if we move to an mbedtls that supports pkcs7 messages with multiple signers: what if a message contains both a sha256 and then a sha512 signature? Our new check won't catch it. At least with passing 32 there is the possibility that mbedtls can do the right thing and not over-read. If we pass in 0, then mbedtls has no way of knowing that we have only 32 bytes allocated. Hopefully the upshot of this is that the code is safer and if we do end up using another hash algorithm, we're forced to at least consider the possibility that the hash length is different. [If we ever use a version of mbedtls that supports multiple digests then we should of course review this code again. Arguably multiple digests are also going to need an mbedtls API change, it's difficult to figure out how a hash length value of 0 could be safe.] Signed-off-by: Daniel Axtens --- As always, compile tested only because I don't have access to a box set up to test this. --- libstb/secvar/backend/edk2-compat-process.c | 23 ++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/libstb/secvar/backend/edk2-compat-process.c b/libstb/secvar/backend/edk2-compat-process.c index 3ef3cf73c8f7..089a4def4cf8 100644 --- a/libstb/secvar/backend/edk2-compat-process.c +++ b/libstb/secvar/backend/edk2-compat-process.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include "libstb/crypto/pkcs7/pkcs7.h" @@ -455,11 +456,11 @@ out: /* Verify the PKCS7 signature on the signed data. */ static int verify_signature(const struct efi_variable_authentication_2 *auth, - const char *hash, const size_t hash_len, - const struct secvar *avar) + const char *hash, const struct secvar *avar) { mbedtls_pkcs7 *pkcs7 = NULL; mbedtls_x509_crt x509; + mbedtls_md_type_t md_alg; char *signing_cert = NULL; char *x509_buf = NULL; int signing_cert_size; @@ -478,6 +479,18 @@ static int verify_signature(const struct efi_variable_authentication_2 *auth, if (!pkcs7) return OPAL_PARAMETER; + /* + * We only support sha256, which has a hash length of 32. + * If the alg is not sha256, then we should bail now. + */ + rc = mbedtls_oid_get_md_alg(&pkcs7->signed_data.digest_alg_identifiers, + &md_alg); + if (rc != 0) + return OPAL_PARAMETER; + + if (md_alg != MBEDTLS_MD_SHA256) + return OPAL_PARAMETER; + prlog(PR_INFO, "Load the signing certificate from the keystore"); eslvarsize = avar->data_size; @@ -534,7 +547,7 @@ static int verify_signature(const struct efi_variable_authentication_2 *auth, free(x509_buf); x509_buf = NULL; - rc = mbedtls_pkcs7_signed_hash_verify(pkcs7, &x509, hash, hash_len); + rc = mbedtls_pkcs7_signed_hash_verify(pkcs7, &x509, hash, 32); /* If you find a signing certificate, you are done */ if (rc == 0) { @@ -745,8 +758,8 @@ int process_update(const struct secvar *update, char **newesl, if (!avar || !avar->data_size) continue; - /* Verify the signature. sha256 is 32 bytes long. */ - rc = verify_signature(auth, hash, 32, avar); + /* Verify the signature. */ + rc = verify_signature(auth, hash, avar); /* Break if signature verification is successful */ if (rc == OPAL_SUCCESS) { From patchwork Mon Jun 21 08:26:41 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Axtens X-Patchwork-Id: 1494944 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ozlabs.org (client-ip=2404:9400:2:0:216:3eff:fee1:b9f1; helo=lists.ozlabs.org; envelope-from=skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=axtens.net header.i=@axtens.net header.a=rsa-sha256 header.s=google header.b=bHFI1bYL; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2404:9400:2:0:216:3eff:fee1:b9f1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4G7jLt4RNlz9sWM for ; Mon, 21 Jun 2021 18:27:18 +1000 (AEST) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4G7jLv21qWz305v for ; Mon, 21 Jun 2021 18:27:19 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=axtens.net header.i=@axtens.net header.a=rsa-sha256 header.s=google header.b=bHFI1bYL; dkim-atps=neutral X-Original-To: skiboot@lists.ozlabs.org Delivered-To: skiboot@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=axtens.net (client-ip=2607:f8b0:4864:20::1030; helo=mail-pj1-x1030.google.com; envelope-from=dja@axtens.net; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=axtens.net header.i=@axtens.net header.a=rsa-sha256 header.s=google header.b=bHFI1bYL; dkim-atps=neutral Received: from mail-pj1-x1030.google.com (mail-pj1-x1030.google.com [IPv6:2607:f8b0:4864:20::1030]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4G7jLh6TN7z307S for ; Mon, 21 Jun 2021 18:27:08 +1000 (AEST) Received: by mail-pj1-x1030.google.com with SMTP id g24so9484429pji.4 for ; Mon, 21 Jun 2021 01:27:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=axtens.net; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=s87r5z9jrGmNmp5bQD8dUTmw6l2UeQv2XFIVSmA2TSw=; b=bHFI1bYLvkQlv4G9/0T4ryGbdrhN70kb+OtkTbCu0y0fLLDcGqa5dQXHbmHRroIhgb JBECKRjVLBtpM1huqspL81N5BB0OPLdC6682c/3oBUT5hGEqMpZTwG8Z5f1DdjDltu5h 0CYe2c0cCRXH1qHW2Rq0kc1xuz4XgKlF4jCLA= 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=s87r5z9jrGmNmp5bQD8dUTmw6l2UeQv2XFIVSmA2TSw=; b=oAYbhHzXhay3Q/hD8kXUnYIWHgUqkFIeSO0pOBQX3r9BZNAlj8f8W1qz8MwTsozpnv Zs+TkEyt7V1jodnJeSxSW1f+kVyOMsi+RuQfgUt+crCrydnLAR3agLieeBXHcklkguja fWk/9nHRls2rHbu3xQ1mKYHGEI5Y99YAhlmiYHah4Lf3TamhVR64meJyziz1yOistgtW QjuKfIDnJWrp1m2DMysATUO4v/rfmM2BcFI1GOhCbJbnrcrBVVxBtAecXIOWM2FBwVy3 0i0ZpuSFY0dNrV52olnQ6ziTFMyOccYiX+BLEOkPwf8eV1+i9ngPyI3cXqM5fV3kgpK0 99Nw== X-Gm-Message-State: AOAM531Hwnl9hAQsEhNisdGaFdR+egBQn/gCyU/NYEmVpeHviIlkiwih P0R2HR99KFcVO/9R218g6QdMWv2RuvRITQ== X-Google-Smtp-Source: ABdhPJzT3TQ0zue0ZsE/WnaFHWzczluXc+CrDEA2T+WyueEGsQE9Tn/DHFYmDfjBJVAThWy5fx31ew== X-Received: by 2002:a17:90b:218c:: with SMTP id ku12mr9170495pjb.235.1624264025355; Mon, 21 Jun 2021 01:27:05 -0700 (PDT) Received: from localhost ([203.206.29.204]) by smtp.gmail.com with ESMTPSA id 1sm8851583pfo.92.2021.06.21.01.27.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Jun 2021 01:27:05 -0700 (PDT) From: Daniel Axtens To: skiboot@lists.ozlabs.org Date: Mon, 21 Jun 2021 18:26:41 +1000 Message-Id: <20210621082641.26476-6-dja@axtens.net> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210621082641.26476-1-dja@axtens.net> References: <20210621082641.26476-1-dja@axtens.net> MIME-Version: 1.0 Subject: [Skiboot] [PATCH v2 5/5] secvar/pkcs7: fix a wrong sizeof() X-BeenThere: skiboot@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list for skiboot development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: nick.child@ibm.com, nayna@linux.ibm.com Errors-To: skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Skiboot" This code isn't directly used by skiboot, but it is wrong and potentially insecure so I'm fixing it in case it's used in the future. We pass sizeof(hash) into mbedtls_pk_verify(). However, hash is a pointer, not an array, so rather than passing the length of the hash to verify we'll pass in 8, and only compare the first 8 bytes of the hash rather than all 32. Pass in 0 instead. That tells mbedtls to work out the length based on the hash type. We allocated enough memory for whatever hash type the PKCS#7 message declared so this will be safe. Signed-off-by: Daniel Axtens --- libstb/crypto/pkcs7/pkcs7.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libstb/crypto/pkcs7/pkcs7.c b/libstb/crypto/pkcs7/pkcs7.c index 4407e201a4cc..3f41ba7acb2e 100644 --- a/libstb/crypto/pkcs7/pkcs7.c +++ b/libstb/crypto/pkcs7/pkcs7.c @@ -538,7 +538,7 @@ int mbedtls_pkcs7_signed_data_verify( mbedtls_pkcs7 *pkcs7, mbedtls_md( md_info, data, datalen, hash ); - ret = mbedtls_pk_verify( &pk_cxt, md_alg, hash, sizeof(hash), + ret = mbedtls_pk_verify( &pk_cxt, md_alg, hash, 0, pkcs7->signed_data.signers.sig.p, pkcs7->signed_data.signers.sig.len );