Hi Laszlo,
On 2018/10/17 21:10, Laszlo Ersek wrote:
Hi Star,
On 10/16/18 04:41, Star Zeng wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=415
When SetVariable() to a time based auth variable with APPEND_WRITE
attribute, and if the EFI_VARIABLE_AUTHENTICATION_2.TimeStamp in
the input Data is earlier than current value, it will cause timestamp
zeroing.
This issue may bring time based auth variable downgrade problem.
For example:
A vendor released three certs at 2014, 2015, and 2016, and system
integrated the 2016 cert. User can SetVariable() with 2015 cert and
APPEND_WRITE attribute to cause timestamp zeroing first, then
SetVariable() with 2014 cert to downgrade the cert.
This patch fixes this issue.
Cc: Jiewen Yao <jiewen....@intel.com>
Cc: Chao Zhang <chao.b.zh...@intel.com>
Cc: Jian J Wang <jian.j.w...@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.z...@intel.com>
---
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index a2d61c8cd618..8e8db71bd201 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -2462,6 +2462,8 @@ UpdateVariable (
if (Variable->CurrPtr != NULL) {
if (VariableCompareTimeStampInternal (&(((AUTHENTICATED_VARIABLE_HEADER
*) CacheVariable->CurrPtr)->TimeStamp), TimeStamp)) {
CopyMem (&AuthVariable->TimeStamp, TimeStamp, sizeof (EFI_TIME));
+ } else {
+ CopyMem (&AuthVariable->TimeStamp, &(((AUTHENTICATED_VARIABLE_HEADER *)
CacheVariable->CurrPtr)->TimeStamp), sizeof (EFI_TIME));
}
}
}
thank you for the BZ reference in the commit message.
The commit message is very good, and from it, I suspected this was a
security bug -- it makes "dbx" rollbacks possible, correct? --, and I
was wondering if it should have received a CVE.
Yes, your are right. You have known there is a CVE for it.
Indeed, checking the TianoCore BZ, I can see that this patch mitigates
CVE-2018-3613.
I have requested earlier [1], and now I'm doing so again, that CVE fixes
please all mention the CVE number in the *subject line*. When people
look at the commit log, or even just patch traffic on this list, CVE
numbers should *jump* at them.
Good request. How about we document it as requirement at somewhere
(Contributions.txt?)? Then people can easily find the requirement and
follow it.
e62f7104-e341-6c7f-1af5-2130f161f111@redhat.com">http://mid.mail-archive.com/e62f7104-e341-6c7f-1af5-2130f161f111@redhat.com
Sorry, I could not access it.
Because you pushed this patch in ~25 hours after posting it to the
public list, and because TianoCore BZ#415 used to be a security bug
(restricted from mirroring to the bugzilla list, and opened up likely
most recently only), I couldn't comment on the subject line (I was on
PTO yesterday), and now we have another patch in the git history that is
a CVE fix, but states that fact nowhere at all.
I was unlucky and I am sad that I could not receive your
feedback/comment before I pushed the patch. :(
From TianoCore BZ#415, we can see the original embargoed data was
"10/26/17". For some reason, it was extended to "July 10, 2018". I
supposed some coordinator(s) should have coordinated with organizations
for this CVE before its disclosure.
I was just aware that Security Advisory including this CVE at
https://edk2-docs.gitbooks.io/security-advisory/ was released at "Oct
12, 2018" and TianoCore BZ#415 link was just made public before I posted
the patch. I thought I should make the patch into the code as quick as
possible (after following the community code review process) after this
CVE's disclosure.
To be clear, my complaint is not that the patch was pushed too quickly
(one day should be fine for CVEs after coordinated disclosure); my point
is that the patch was pushed quickly *and* it never mentioned it was a
CVE fix (in the subject line specifically).
Got it. I should have done like this if I was aware the request.:(
In addition, while the bugzilla states:
The issue is there since the auth variable driver was created in
SecurityPkg, and it is inherited to current variable driver in
MdeModulePkg after the auth variable driver in SecurityPkg was merged
to variable driver in MdeModulePkg.
some specific commit references in the fix's commit message would have
helped, so that everyone could evaluate whether they were affected.
We can see the attachment for reference in TianoCore BZ#415 link only
updates the variable driver in MdeModulePkg and we just synced the patch
to UDK2018/UDK2017/UDK2015 which all have SecurityPkg variable driver
merged into MdeModulePkg variable driver. SecurityPkg variable driver is
just present in very old UDK branches. And people is not hard to know
the history of MdeModulePkg and SecurityPkg variable driver by GIT log.
So I left the statement in TianoCore BZ#415 link.
Yes, I admit it would be better also including some statement/reference
in commit log. :)
Really thanks very much for the comments.
Star
--*--
Process-wise, I'm sad that Red Hat -- and likely many other
organizations shipping edk2-based firmware -- have not been involved in
a coordinated disclosure around this issue. The timeline in
https://bugzilla.tianocore.org/show_bug.cgi?id=415
suggests that there would have been a lot of time for this (and
apperently there was *intent* too). But here we are, caught with our
pants around our ankles.
Prasad, to my understanding, you are Red Hat's representative on the
TianoCore Bugzilla security group. I've now searched the RH Bugzilla for
"CVE-2018-3613", and there are no hits. Can you please confirm whether
this BZ was made available to us (and we missed it, and/or failed to act
upon it otherwise)?
Either way, please:
- Create the appropriate tracker in the Red Hat Bugzilla. (The patch has
been picked to UDK as far back as UDK2015; we obviously need to fix
this yesterday.)
- Forward the issue to <https://seclists.org/oss-sec/>, so that other
organizations that distribute OVMF learn of this.
(I'm adding a few direct CC's now, but that list shouldn't be limited by
my imagination. I've briefly searched the oss-sec archive as well: also
no hits.)
Thank you,
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
