public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Long, Qin" <qin.long@intel.com>
To: Wim Vervoorn <wvervoorn@eltan.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Subject: Re: Timebased Auth Variable driver should ensure AuthAlgorithm is SHA256 before further verification
Date: Mon, 11 Dec 2017 15:55:40 +0000	[thread overview]
Message-ID: <BF2CCE9263284D428840004653A28B6E5403049D@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <5492e8b3cf5e4d48ab401e085a6a25e9@Eltsrv03.Eltan.local>

Hi, Wim Vervoorn,

Yes, the logic here is a little tricky. We wouldn't like to introduce the full ASN.1 parse interfaces to handle the encoding data check. So as the comments states, the digestAlgorithms field usually has the fixed offset (based on two bytes of length encoding) in one PKCS#7 signedData structure. So the new codes (added by that commit) used this assumption to check the Sha256 OID directly. 
  //
  // SignedData.digestAlgorithms shall contain the digest algorithm used when preparing the
  // signature. Only a digest algorithm of SHA-256 is accepted.
  //
  //    According to PKCS#7 Definition:
  //        SignedData ::= SEQUENCE {
  //            version Version,
  //            digestAlgorithms DigestAlgorithmIdentifiers,
  //            contentInfo ContentInfo,
  //            .... }
  //    The DigestAlgorithmIdentifiers can be used to determine the hash algorithm 
  //    in VARIABLE_AUTHENTICATION_2 descriptor.
  //    This field has the fixed offset (+13) and be calculated based on two bytes of length encoding.
  //
  ......

One typical ASN.1 structure of PKCS7 Signature is
  ContentInfo {
     contentType = 1.2.840.113549.1.7.2   //(signedData)
     content {
       SignedData {
         version = 1
         ...
       }
     }
  }
But please note, the PKCS#7 signedData definition for Authenticated Variable in UEFI spec didn't include the contentType fields. So if you used some third-party tool (e.g. OpenSSL) to generate the signedData, you need to strip-off some bytes. 

See more discussion & clarifications from https://bugzilla.tianocore.org/show_bug.cgi?id=586
And share us the binary data for more analysis if you still have verification issues. 


Best Regards & Thanks,
LONG, Qin

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Wim Vervoorn
Sent: Monday, December 11, 2017 6:40 PM
To: edk2-devel@lists.01.org
Subject: [edk2] Timebased Auth Variable driver should ensure AuthAlgorithm is SHA256 before further verification

Hello,

We ran into issues with the Timebased Authenticated variable handling.

In commit: c035e37335ae43229d7e68de74a65f2c01ebc0af

This was added. This assumed the very first tag will be the Sha256 Oid. We have noticed situations where this is the case.

The question is if the check below represents the specification and the tools generating the databuffer should be changed. Or if this check is not correct. It seems to me that the data should be parsed to check for the correct OID and not assume this is the first one

  if ((Attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) != 0) {
    if (SigDataSize >= (13 + sizeof (mSha256OidValue))) {
      if (((*(SigData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) || 
           (CompareMem (SigData + 13, &mSha256OidValue, sizeof (mSha256OidValue)) != 0)) {
          return EFI_SECURITY_VIOLATION;
        }
    }
  }


----
Modified: SecurityPkg/Library/AuthVariableLib/AuthService.c
Modified: SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h


Best Regards,
Wim Vervoorn

Eltan B.V.
Ambachtstraat 23
5481 SM Schijndel
The Netherlands

T : +31-(0)73-594 46 64
E : wvervoorn@eltan.com
W : http://www.eltan.com


"THIS MESSAGE CONTAINS CONFIDENTIAL INFORMATION. UNLESS YOU ARE THE INTENDED RECIPIENT OF THIS MESSAGE, ANY USE OF THIS MESSAGE IS STRICTLY PROHIBITED. IF YOU HAVE RECEIVED THIS MESSAGE IN ERROR, PLEASE IMMEDIATELY NOTIFY THE SENDER BY TELEPHONE +31-(0)73-5944664 OR REPLY EMAIL, AND IMMEDIATELY DELETE THIS MESSAGE AND ALL COPIES." 



_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


  reply	other threads:[~2017-12-11 15:51 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-11 10:40 Timebased Auth Variable driver should ensure AuthAlgorithm is SHA256 before further verification Wim Vervoorn
2017-12-11 15:55 ` Long, Qin [this message]
2017-12-12 14:14   ` Wim Vervoorn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BF2CCE9263284D428840004653A28B6E5403049D@SHSMSX103.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox