From patchwork Fri Nov 14 10:33:46 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laszlo Ersek X-Patchwork-Id: 40808 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-wi0-f199.google.com (mail-wi0-f199.google.com [209.85.212.199]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id C1ABC240ED for ; Fri, 14 Nov 2014 10:34:48 +0000 (UTC) Received: by mail-wi0-f199.google.com with SMTP id r20sf813429wiv.10 for ; Fri, 14 Nov 2014 02:34:48 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:from:to:date:message-id:subject :precedence:reply-to:list-id:list-unsubscribe:list-archive:list-post :list-help:list-subscribe:mime-version:errors-to:x-original-sender :x-original-authentication-results:mailing-list:content-type :content-transfer-encoding; bh=fzovbdtPkGeQKrZVcW8fvZiB5VNC09gMw3rMUWM9prw=; b=MifjCMDqk2wtINuhclnhbt79T350PPZiNMs+C1fDz1wDtMIzTwWT63B9e5M1QDkjku Qnv7AnfTMp+Y8HZytjFIaAiXjriOMjtDx6l1+gsPILiioNohO18zM3zEU1jvZ7cRVhpP Dd0DiNI2WCR/gANcuubUoRogAH7h1IVArJSBb1gXj33iGxjtMw9GCEhvnqMkCZRrSyvE 7ad48ElYUn+UheXdfGhTeY5PFmK9YnER12DzaFwkzvcRjBT0L2Nurosdw8kzfEhe4N2M NSzILQ42dj2IYh3ett3Xcdoo7mS5XX7jJkrXCMkEpsWCiMv+O/vrd68XBmCTUTkeGuuC LC/A== X-Gm-Message-State: ALoCoQm//kpiDbdFlJJD9WRXHcjgdK1XA6VDQjy6dB9b1audn/0+EotfUipEoMqXGdlOhW2V9acP X-Received: by 10.112.154.194 with SMTP id vq2mr4354091lbb.10.1415961288052; Fri, 14 Nov 2014 02:34:48 -0800 (PST) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.120.98 with SMTP id lb2ls434943lab.81.gmail; Fri, 14 Nov 2014 02:34:47 -0800 (PST) X-Received: by 10.112.169.106 with SMTP id ad10mr7460945lbc.13.1415961287250; Fri, 14 Nov 2014 02:34:47 -0800 (PST) Received: from mail-lb0-f174.google.com (mail-lb0-f174.google.com. [209.85.217.174]) by mx.google.com with ESMTPS id ob1si41092462lbb.113.2014.11.14.02.34.47 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 14 Nov 2014 02:34:47 -0800 (PST) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.174 as permitted sender) client-ip=209.85.217.174; Received: by mail-lb0-f174.google.com with SMTP id w7so3842614lbi.5 for ; Fri, 14 Nov 2014 02:34:47 -0800 (PST) X-Received: by 10.153.7.170 with SMTP id dd10mr7459087lad.44.1415961287110; Fri, 14 Nov 2014 02:34:47 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.184.201 with SMTP id ew9csp742268lbc; Fri, 14 Nov 2014 02:34:46 -0800 (PST) X-Received: by 10.51.17.104 with SMTP id gd8mr4987105igd.21.1415961285777; Fri, 14 Nov 2014 02:34:45 -0800 (PST) Received: from lists.sourceforge.net (lists.sourceforge.net. [216.34.181.88]) by mx.google.com with ESMTPS id i126si3455045ioe.94.2014.11.14.02.34.45 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Fri, 14 Nov 2014 02:34:45 -0800 (PST) Received-SPF: pass (google.com: domain of edk2-devel-bounces@lists.sourceforge.net designates 216.34.181.88 as permitted sender) client-ip=216.34.181.88; Received: from localhost ([127.0.0.1] helo=sfs-ml-1.v29.ch3.sourceforge.com) by sfs-ml-1.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1XpED2-0005iB-1t; Fri, 14 Nov 2014 10:34:32 +0000 Received: from sog-mx-2.v43.ch3.sourceforge.com ([172.29.43.192] helo=mx.sourceforge.net) by sfs-ml-1.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1XpECY-0005M9-N9 for edk2-devel@lists.sourceforge.net; Fri, 14 Nov 2014 10:34:02 +0000 Received-SPF: pass (sog-mx-2.v43.ch3.sourceforge.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=lersek@redhat.com; helo=mx1.redhat.com; Received: from mx1.redhat.com ([209.132.183.28]) by sog-mx-2.v43.ch3.sourceforge.com with esmtps (TLSv1:AES256-SHA:256) (Exim 4.76) id 1XpECW-0004yO-RK for edk2-devel@lists.sourceforge.net; Fri, 14 Nov 2014 10:34:02 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sAEAXrcT015401 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Fri, 14 Nov 2014 05:33:53 -0500 Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-125.ams2.redhat.com [10.36.116.125]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sAEAXo6f007053; Fri, 14 Nov 2014 05:33:51 -0500 From: Laszlo Ersek To: "Qin Long" , edk2-devel@lists.sourceforge.net Date: Fri, 14 Nov 2014 11:33:46 +0100 Message-Id: <1415961226-12899-1-git-send-email-lersek@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Spam-Score: -1.5 (-) X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -1.5 SPF_CHECK_PASS SPF reports sender host as permitted sender for sender-domain -0.0 SPF_HELO_PASS SPF: HELO matches SPF record -0.0 T_RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -0.0 SPF_PASS SPF: sender matches SPF record X-Headers-End: 1XpECW-0004yO-RK Subject: [edk2] [PATCH] SecurityPkg: VariableServiceSetVariable(): fix dbt <-> GUID association X-BeenThere: edk2-devel@lists.sourceforge.net X-Mailman-Version: 2.1.9 Precedence: list Reply-To: edk2-devel@lists.sourceforge.net List-Id: List-Unsubscribe: , List-Archive: List-Post: , List-Help: , List-Subscribe: , MIME-Version: 1.0 Errors-To: edk2-devel-bounces@lists.sourceforge.net X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: lersek@redhat.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.174 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 SVN r16380 ("UEFI 2.4 X509 Certificate Hash and RFC3161 Timestamp Verification support for Secure Boot") broke the "dbt" variable's association with its expected namespace GUID. According to "MdePkg/Include/Guid/ImageAuthentication.h", *all* of the "db", "dbx", and "dbt" (== EFI_IMAGE_SECURITY_DATABASE2) variables have their special meanings in the EFI_IMAGE_SECURITY_DATABASE_GUID namespace. However, the above commit introduced the following expression in VariableServiceSetVariable(): - } else if (CompareGuid (VendorGuid, &gEfiImageSecurityDatabaseGuid) && - ((StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE) == 0) || (StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE1) == 0))) { + } else if (CompareGuid (VendorGuid, &gEfiImageSecurityDatabaseGuid) && + ((StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE) == 0) || (StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE1) == 0)) + || (StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE2)) == 0) { Simply replacing the individual expressions with the predicates "GuidMatch", "DbMatch", "DbxMatch", and "DbtMatch", the above transformation becomes: - } else if (GuidMatch && - ((DbMatch) || (DbxMatch))) { + } else if (GuidMatch && + ((DbMatch) || (DbxMatch)) + || DbtMatch) { In shorter form, we change GuidMatch && (DbMatch || DbxMatch) into GuidMatch && (DbMatch || DbxMatch) || DbtMatch which is incorrect, because this way "dbt" will match outside of the intended namespace / GUID. The error was caught by gcc: > SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c: In function > 'VariableServiceSetVariable': > > SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c:3188:71: error: > suggest parentheses around '&&' within '||' [-Werror=parentheses] > > } else if (CompareGuid (VendorGuid, &gEfiImageSecurityDatabaseGuid) && > ^ > cc1: all warnings being treated as errors Fix the parentheses. This change may have security implications. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek --- SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c index 432531f..ac043d9 100644 --- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c +++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c @@ -3186,8 +3186,11 @@ VariableServiceSetVariable ( } else if (CompareGuid (VendorGuid, &gEfiGlobalVariableGuid) && (StrCmp (VariableName, EFI_KEY_EXCHANGE_KEY_NAME) == 0)) { Status = ProcessVarWithPk (VariableName, VendorGuid, Data, DataSize, &Variable, Attributes, FALSE); } else if (CompareGuid (VendorGuid, &gEfiImageSecurityDatabaseGuid) && - ((StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE) == 0) || (StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE1) == 0)) - || (StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE2)) == 0) { + ((StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE) == 0) || + (StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE1) == 0) || + (StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE2) == 0) + ) + ) { Status = ProcessVarWithPk (VariableName, VendorGuid, Data, DataSize, &Variable, Attributes, FALSE); if (EFI_ERROR (Status)) { Status = ProcessVarWithKek (VariableName, VendorGuid, Data, DataSize, &Variable, Attributes);