From patchwork Tue Jul 20 16:05:00 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nick Child X-Patchwork-Id: 1507670 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=pikSZOnI; 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 4GTl370zS0z9sWd 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 4GTl366Y5Mz3bVT for ; Wed, 21 Jul 2021 02:46:10 +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=pikSZOnI; 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::f31; helo=mail-qv1-xf31.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=pikSZOnI; dkim-atps=neutral Received: from mail-qv1-xf31.google.com (mail-qv1-xf31.google.com [IPv6:2607:f8b0:4864:20::f31]) (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 4GTk7x0bKDz2xKM for ; Wed, 21 Jul 2021 02:05:16 +1000 (AEST) Received: by mail-qv1-xf31.google.com with SMTP id f3so10335662qvm.2 for ; Tue, 20 Jul 2021 09:05:16 -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=9gdTKQuD3xL5mNJxxvDFe/SjzwmrdbkZUFyPP7tAbZI=; b=pikSZOnIRpz9RRt5ph4yEyc/j8yIlLF942Hl426/1pHCKaRJ5FXWVVuyH3VahCOBmY NA60f1dWlb+ayM4VkCajb4J5TQyikvkwcEB15Bj/qdImeLUi0A+SGFEC3E5asj1gy8WY hY6ZzX/B5HH+N6SzpGYBoM/++dgGGs3cBTeFWf3HRnr2fvGQZFuyAhHz2Kj71MYG5aKt yG/bEMF600BWhTqCNBZBEXGS6Vx0vKdwv281WSVOHZZOiq9DsnAwIZLKgbP+QUWI/+eo Kg/Uhj9opeg1StPNPdBvEZF11cpZ1gAk/TcnuZcIt3q9DnhuF3Ry8E8m1Gj1Lg5ILm2p otCQ== 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=9gdTKQuD3xL5mNJxxvDFe/SjzwmrdbkZUFyPP7tAbZI=; b=mJp0oSM5UztYi53ow+Bt7C+XwwdZMZnBW/TDxULHmsELFnw1ZbHNa2oUpVAIt4qsRt +l4LNys2n3A2G8kPWpXB84NeRHS4UPE4iSMldJ9tCrmGhVdMkT7HRFEWb78I/kCyijHd LY5mxWMbiQUHRcJV70sRUPGqn3h0kAgohBVfyniQp24Aa3IwwEnSvvQyCKWzrB/O3SyZ ZsGvFmbFSiUuhUnQ6ePTEPft3zIImHPtyifjriQDvPkVYhjtUnaft5FKJt9JWbPFmdxr tInel++mz015b1o4kk6pWLdpSqYSf2swiETm0dqgjJHCj9P7gQy1SIhWTLelJUhGc8LW C0zg== X-Gm-Message-State: AOAM531MZ3B2PxElNqbTfDsCTTVS8d0qqXfHPOGGqpsgHOYwav/J9MAy quiURRSKo4pfAuX7mBo7Trs5A81C2CC/ng== X-Google-Smtp-Source: ABdhPJxu+xXtDrLAqMbGCk6GPduaHYJBOo7h0WRVwqbyX2C0ZV38RU4VCVU9E+q6GTc/G4gc+SkAKw== X-Received: by 2002:a0c:d644:: with SMTP id e4mr30698101qvj.45.1626797113840; Tue, 20 Jul 2021 09:05:13 -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.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 20 Jul 2021 09:05:13 -0700 (PDT) From: Nick Child X-Google-Original-From: Nick Child To: skiboot-stable@lists.ozlabs.org Date: Tue, 20 Jul 2021 12:05:00 -0400 Message-Id: <20210720160501.9850-3-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 3/4] secvar: return error if validate_esl has extra data 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: 355176a9405c83320748f804e8655e6a8ee2324f from upstream. Currently, in `validate_esl_list`, the return code is initialized to zero (our success value). While looping though the ESL's in the submitted ESL chain, the loop will break if there is not enough data to meet minimum ESL requirements. This condition was not setting a return code, meaning that the successful return code can pass to the end of the function if there is extra data at the end of the ESL. As a consequence, any properly signed update can successfully commit any data (as long as it is less than the min size of an ESL) to the secvars. This commit will return an error if the described condition is met. This means all data in the appended ESL of an auth file must be accounted for. No extra bytes can be added to the end since, on success, this data will become the updated secvar. Additionally, a test case has been added to ensure that this commit addresses the issue 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 | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/libstb/secvar/backend/edk2-compat-process.c b/libstb/secvar/backend/edk2-compat-process.c index 81ddb9a6..ecba3c2b 100644 --- a/libstb/secvar/backend/edk2-compat-process.c +++ b/libstb/secvar/backend/edk2-compat-process.c @@ -266,8 +266,11 @@ int validate_esl_list(const char *key, const char *esl, const size_t size) while (eslvarsize > 0) { prlog(PR_DEBUG, "esl var size is %d offset is %lu\n", eslvarsize, size - eslvarsize); - if (eslvarsize < sizeof(EFI_SIGNATURE_LIST)) + if (eslvarsize < sizeof(EFI_SIGNATURE_LIST)) { + prlog(PR_ERR, "ESL with size %d is too small\n", eslvarsize); + rc = OPAL_PARAMETER; break; + } /* Check Supported ESL Type */ list = get_esl_signature_list(esl, eslvarsize); diff --git a/libstb/secvar/test/secvar-test-edk2-compat.c b/libstb/secvar/test/secvar-test-edk2-compat.c index 3cdc6ef2..1edce112 100644 --- a/libstb/secvar/test/secvar-test-edk2-compat.c +++ b/libstb/secvar/test/secvar-test-edk2-compat.c @@ -165,6 +165,21 @@ int run_test() ASSERT(5 == list_length(&variable_bank)); ASSERT(setup_mode); + /* Add PK with bad ESL. should fail since data is not big enough to be ESL*/ + printf("Add PK with invalid appended ESL"); + /* 1014 is length of appended ESL Header and its data */ + tmp = new_secvar("PK", 3, PK_auth, PK_auth_len - 1014 + sizeof(EFI_SIGNATURE_LIST) - 1, 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(5 == list_length(&variable_bank)); + ASSERT(0 == list_length(&update_bank)); + rc = edk2_compat_post_process(&variable_bank, &update_bank); + ASSERT(5 == list_length(&variable_bank)); + ASSERT(setup_mode); + + /* Add PK to update and .process(). */ printf("Add PK"); tmp = new_secvar("PK", 3, PK_auth, PK_auth_len, 0);