From patchwork Tue Jul 20 16:05:01 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nick Child X-Patchwork-Id: 1507671 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-stable-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20161025 header.b=mfMCDQgn; 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 4GTl375YkNz9sWX for ; Wed, 21 Jul 2021 02:46:11 +1000 (AEST) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4GTl3747cSz3bYC for ; Wed, 21 Jul 2021 02:46:11 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20161025 header.b=mfMCDQgn; dkim-atps=neutral X-Original-To: skiboot-stable@lists.ozlabs.org Delivered-To: skiboot-stable@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gmail.com (client-ip=2607:f8b0:4864:20::82d; helo=mail-qt1-x82d.google.com; envelope-from=nnac123@gmail.com; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20161025 header.b=mfMCDQgn; dkim-atps=neutral Received: from mail-qt1-x82d.google.com (mail-qt1-x82d.google.com [IPv6:2607:f8b0:4864:20::82d]) (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 4GTk7y36q3z2xKM for ; Wed, 21 Jul 2021 02:05:18 +1000 (AEST) Received: by mail-qt1-x82d.google.com with SMTP id j7so2765715qtj.6 for ; Tue, 20 Jul 2021 09:05:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=ENAu/nvs0P1crgqrnEDCvj2R8w3U268xkKxqyXsC0To=; b=mfMCDQgnbnCUNSfmpjd1znzY5BRzmT/HbRusuC1XPGxEGXwjdBRObG3fBCHjqbdJDq Zt7he7btYUDBxAZC2VaAuupuMdcEKp6QgTgLMcK9kzT+Mdm8SmTgNy+wbgZ04sbdwCce Nk6x3UXza3Y1g1xx4uyF1bowKLPqKzoTzS78jDpJ8Ktmx88vmBz3olYQQaBW7cSqLq24 mDuPgM5JcNq0eK9W7vf8+cWTHCW5PfsBg/84bzANQBirVoYiYwy//Je4YlN9UM20fugj h22r0vOVnvlF+O5mV0vkZoPhY1JVXA8hILc9b+iUuE70UcjYTEAkEMjug1ybWbMff7gL 7lDA== 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=ENAu/nvs0P1crgqrnEDCvj2R8w3U268xkKxqyXsC0To=; b=KjV6in8sqC1vugPPnYj+2Y5jFRTXop+hTW2RNeEVJltNfPzygsTMgo74Zb0flQRmZG 4iJWgam7q78THsedPndO13tR1QvrdjXRj4x042O5yWgLjksP4fzpNVQh0mjwvEGDQF53 nnmffN3+aj9H7QoFZitzmvovA0e992S13IhFJ8Xg6fj4Yh3/HWXLwhCWUVIAGs5Gw4SE H1dXDBjKsZzAUJTErohBi3FaBOWHnQ4pwIpgvWw2Z7Yp5HMyZPMI+Z8e9MuJaooeo394 90u2/Zoo8TOKCBqSF6QnFcHqy/lTKTU4tbwXH8mo5UrxriHC7lTmlh9CNmUJJ3vgJojW INIg== X-Gm-Message-State: AOAM532SWaRy7ZbnBOG4rrZo1fYlStMo1nktAG8OM0uzde1dLtXXO0CF NT5iTB9m1Fvmxyx5OkAiTuDgqEVjcqbq2g== X-Google-Smtp-Source: ABdhPJxP+xkKaeM1JdPS5k4m32hNQiKDq1plrgSTZT91iy0U5clCT4FVXTBi+Sib9L/zEB4sTXYPAQ== X-Received: by 2002:a05:622a:134d:: with SMTP id w13mr26559312qtk.275.1626797114999; Tue, 20 Jul 2021 09:05:14 -0700 (PDT) Received: from starship-12.hsd1.fl.comcast.net ([2601:589:4a00:1ed0:4391:e426:6e2:788b]) by smtp.gmail.com with ESMTPSA id 67sm7964727qtf.83.2021.07.20.09.05.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 20 Jul 2021 09:05:14 -0700 (PDT) From: Nick Child X-Google-Original-From: Nick Child To: skiboot-stable@lists.ozlabs.org Date: Tue, 20 Jul 2021 12:05:01 -0400 Message-Id: <20210720160501.9850-4-nick.child@ibm.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210720160501.9850-1-nick.child@ibm.com> References: <20210720160501.9850-1-nick.child@ibm.com> MIME-Version: 1.0 X-Mailman-Approved-At: Wed, 21 Jul 2021 02:46:07 +1000 Subject: [Skiboot-stable] [PATCH v1 4/4] secvar: return error if verify_signature runs out of ESLs X-BeenThere: skiboot-stable@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Patches, review, and discussion for stable releases of skiboot" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: nick.child@ibm.com, Nick Child , nayna@linux.ibm.com, dja@axtens.net Errors-To: skiboot-stable-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Skiboot-stable" From: Nick Child commit ID: 56658ad4a0249cdf516e6bc21781cce901965998 from upstream. Currently, in `verify_signature`, the return code `rc` is initialized as 0 (our success value). While looping through the ESL's in the given secvar, the function will break if the remaining data in the secvar is not enough to contain another ESL. This break from the loop was not setting a return code, this means that the successful return code can pass to the end of the function if the first iteration meets this condition. In other words, if a current secvar has a size that is less than minimum size for an ESL, than it will approve any update. In response to this bug, this commit will return an error code if the described condition is met. Additionally, a test case has been added to ensure that this unlikely event is handled correctly. Fixes: 87562bc5c1a6 ("secvar/backend: add edk2 derived key updates processing") Signed-off-by: Nick Child Reviewed-by: Nayna Jain Tested-by: Nayna Jain Signed-off-by: Vasant Hegde --- libstb/secvar/backend/edk2-compat-process.c | 5 +++- libstb/secvar/test/secvar-test-edk2-compat.c | 25 ++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/libstb/secvar/backend/edk2-compat-process.c b/libstb/secvar/backend/edk2-compat-process.c index ecba3c2b..c0006a5e 100644 --- a/libstb/secvar/backend/edk2-compat-process.c +++ b/libstb/secvar/backend/edk2-compat-process.c @@ -491,8 +491,11 @@ static int verify_signature(const struct efi_variable_authentication_2 *auth, /* Variable is not empty */ while (eslvarsize > 0) { prlog(PR_DEBUG, "esl var size is %d offset is %d\n", eslvarsize, offset); - if (eslvarsize < sizeof(EFI_SIGNATURE_LIST)) + if (eslvarsize < sizeof(EFI_SIGNATURE_LIST)) { + rc = OPAL_INTERNAL_ERROR; + prlog(PR_ERR, "ESL data is corrupted\n"); break; + } /* Calculate the size of the ESL */ eslsize = get_esl_signature_list_size(avar->data + offset, diff --git a/libstb/secvar/test/secvar-test-edk2-compat.c b/libstb/secvar/test/secvar-test-edk2-compat.c index 1edce112..100fda7d 100644 --- a/libstb/secvar/test/secvar-test-edk2-compat.c +++ b/libstb/secvar/test/secvar-test-edk2-compat.c @@ -89,6 +89,7 @@ int run_test() { int rc = -1; struct secvar *tmp; + size_t tmp_size; char empty[64] = {0}; /* The sequence of test cases here is important to ensure that @@ -213,6 +214,30 @@ int run_test() tmp = find_secvar("db", 3, &variable_bank); ASSERT(NULL != tmp); + /* Add db, should fail with no KEK and invalid PK size */ + printf("Add db, corrupt PK"); + /* Somehow PK gets assigned wrong size */ + tmp = find_secvar("PK", 3, &variable_bank); + ASSERT(NULL != tmp); + tmp_size = tmp->data_size; + tmp->data_size = sizeof(EFI_SIGNATURE_LIST) - 1; + tmp = new_secvar("db", 3, DB_auth, DB_auth_len, 0); + ASSERT(0 == edk2_compat_validate(tmp)); + list_add_tail(&update_bank, &tmp->link); + ASSERT(1 == list_length(&update_bank)); + + rc = edk2_compat_process(&variable_bank, &update_bank); + ASSERT(OPAL_INTERNAL_ERROR == rc); + ASSERT(5 == list_length(&variable_bank)); + ASSERT(0 == list_length(&update_bank)); + tmp = find_secvar("db", 3, &variable_bank); + ASSERT(NULL != tmp); + ASSERT(0 == tmp->data_size); + /* Restore PK data size */ + tmp = find_secvar("PK", 3, &variable_bank); + ASSERT(NULL != tmp); + tmp->data_size = tmp_size; + /* Add trimmed KEK, .process(), should fail. */ printf("Add trimmed KEK\n"); tmp = new_secvar("KEK", 4, trimmedKEK_auth, trimmedKEK_auth_len, 0);