public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* Set "db" variable in secure boot setup mode still requires generating PKCS#7?
@ 2018-05-01 21:13 David F.
  2018-05-01 21:25 ` Bill Paul
  2018-05-02 10:21 ` Laszlo Ersek
  0 siblings, 2 replies; 8+ messages in thread
From: David F. @ 2018-05-01 21:13 UTC (permalink / raw)
  To: edk2 developers list

Hi,

Had a fairly simple task of wanting to install the latest MS .crt
files for KEK, and their two files for the "db" (the Windows CA and
UEFI CA) in a system placed in setup/custom mode.  However, even
though it seemed to take the KEK, it never took the "db", always had a
problem on a DH77KC mobo (dumped data headers looked as expected).
Now when I constructed it, I thought I could leave out any PKCS#7 data
(set the expected CertType but in the Hdr dwLength only included
CertType and not any CertData), but looking at the algo in UEFI Spec
2.6 page 245, it looks like we'd always have to generate the hash,
sign it, create all the PKCS stuff even in setup mode?    That would
surely unnecessarily bloat any apps that really only need to update
things in setup mode wouldn't it?   So to confirm, that is a
requirement even in setup mode?    If so, why?

TIA!!


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Set "db" variable in secure boot setup mode still requires generating PKCS#7?
  2018-05-01 21:13 Set "db" variable in secure boot setup mode still requires generating PKCS#7? David F.
@ 2018-05-01 21:25 ` Bill Paul
  2018-05-02  2:23   ` David F.
  2018-05-02 10:21 ` Laszlo Ersek
  1 sibling, 1 reply; 8+ messages in thread
From: Bill Paul @ 2018-05-01 21:25 UTC (permalink / raw)
  To: edk2-devel

Of all the gin joints in all the towns in all the world, David F. had to walk 
into mine at 14:13 on Tuesday 01 May 2018 and say:

> Hi,
> 
> Had a fairly simple task of wanting to install the latest MS .crt
> files for KEK, and their two files for the "db" (the Windows CA and
> UEFI CA) in a system placed in setup/custom mode.  However, even
> though it seemed to take the KEK, it never took the "db", always had a
> problem on a DH77KC mobo (dumped data headers looked as expected).
> Now when I constructed it, I thought I could leave out any PKCS#7 data
> (set the expected CertType but in the Hdr dwLength only included
> CertType and not any CertData), but looking at the algo in UEFI Spec
> 2.6 page 245, it looks like we'd always have to generate the hash,
> sign it, create all the PKCS stuff even in setup mode?    That would
> surely unnecessarily bloat any apps that really only need to update
> things in setup mode wouldn't it?   So to confirm, that is a
> requirement even in setup mode?    If so, why?
>

If I understand correctly, I think the issue is that the PK, KEK, db and dbx 
are always considered to be secure environment variables, which means when you 
try to update them with SetVariable(), you always have to include one of the 
authentication flags and a properly formated authentication header.

The difference between variable updates in secure mode vs. setup/custom mode 
is that in setup/custom mode, the signature is not validated. It still has to 
be there, but the firmware doesn't care what it says. So the db update could 
be signed with a completely different KEK than the one loaded into the KEK 
variable, and it would still be accepted.

-Bill



> TIA!!
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
-- 
=============================================================================
-Bill Paul            (510) 749-2329 | Senior Member of Technical Staff,
                 wpaul@windriver.com | Master of Unix-Fu - Wind River Systems
=============================================================================
   "I put a dollar in a change machine. Nothing changed." - George Carlin
=============================================================================


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Set "db" variable in secure boot setup mode still requires generating PKCS#7?
  2018-05-01 21:25 ` Bill Paul
@ 2018-05-02  2:23   ` David F.
  0 siblings, 0 replies; 8+ messages in thread
From: David F. @ 2018-05-02  2:23 UTC (permalink / raw)
  To: Bill Paul; +Cc: edk2 developers list

I'm with you, but the question I would have is how much checking does
it end up doing?  In other words, how can a fake PKCS#7 be created in
a small amount of code that would make the guard happy enough to let
it through in setup mode?   I just don't see the point of even having
to create one when it is going to be ignored anyway?

Thanks.

On Tue, May 1, 2018 at 2:25 PM, Bill Paul <wpaul@windriver.com> wrote:
> Of all the gin joints in all the towns in all the world, David F. had to walk
> into mine at 14:13 on Tuesday 01 May 2018 and say:
>
>> Hi,
>>
>> Had a fairly simple task of wanting to install the latest MS .crt
>> files for KEK, and their two files for the "db" (the Windows CA and
>> UEFI CA) in a system placed in setup/custom mode.  However, even
>> though it seemed to take the KEK, it never took the "db", always had a
>> problem on a DH77KC mobo (dumped data headers looked as expected).
>> Now when I constructed it, I thought I could leave out any PKCS#7 data
>> (set the expected CertType but in the Hdr dwLength only included
>> CertType and not any CertData), but looking at the algo in UEFI Spec
>> 2.6 page 245, it looks like we'd always have to generate the hash,
>> sign it, create all the PKCS stuff even in setup mode?    That would
>> surely unnecessarily bloat any apps that really only need to update
>> things in setup mode wouldn't it?   So to confirm, that is a
>> requirement even in setup mode?    If so, why?
>>
>
> If I understand correctly, I think the issue is that the PK, KEK, db and dbx
> are always considered to be secure environment variables, which means when you
> try to update them with SetVariable(), you always have to include one of the
> authentication flags and a properly formated authentication header.
>
> The difference between variable updates in secure mode vs. setup/custom mode
> is that in setup/custom mode, the signature is not validated. It still has to
> be there, but the firmware doesn't care what it says. So the db update could
> be signed with a completely different KEK than the one loaded into the KEK
> variable, and it would still be accepted.
>
> -Bill
>
>
>
>> TIA!!
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> --
> =============================================================================
> -Bill Paul            (510) 749-2329 | Senior Member of Technical Staff,
>                  wpaul@windriver.com | Master of Unix-Fu - Wind River Systems
> =============================================================================
>    "I put a dollar in a change machine. Nothing changed." - George Carlin
> =============================================================================


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Set "db" variable in secure boot setup mode still requires generating PKCS#7?
  2018-05-01 21:13 Set "db" variable in secure boot setup mode still requires generating PKCS#7? David F.
  2018-05-01 21:25 ` Bill Paul
@ 2018-05-02 10:21 ` Laszlo Ersek
  2018-05-02 16:26   ` David F.
  1 sibling, 1 reply; 8+ messages in thread
From: Laszlo Ersek @ 2018-05-02 10:21 UTC (permalink / raw)
  To: David F.; +Cc: edk2 developers list

On 05/01/18 23:13, David F. wrote:
> Hi,
>
> Had a fairly simple task of wanting to install the latest MS .crt
> files for KEK, and their two files for the "db" (the Windows CA and
> UEFI CA) in a system placed in setup/custom mode.  However, even
> though it seemed to take the KEK, it never took the "db", always had a
> problem on a DH77KC mobo (dumped data headers looked as expected). Now
> when I constructed it, I thought I could leave out any PKCS#7 data
> (set the expected CertType but in the Hdr dwLength only included
> CertType and not any CertData),

Right, I've stumbled upon that too. According to the UEFI spec, dwLength
should include CertData too, but edk2 does *not* accept that. This can
be seen e.g. in
"SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c",
function CreateTimeBasedPayload():

>   //
>   // In Setup mode or Custom mode, the variable does not need to be signed but the
>   // parameters to the SetVariable() call still need to be prepared as authenticated
>   // variable. So we create EFI_VARIABLE_AUTHENTICATED_2 descriptor without certificate
>   // data in it.
>   //
> ...
>   DescriptorData->AuthInfo.Hdr.dwLength         = OFFSET_OF (WIN_CERTIFICATE_UEFI_GUID, CertData);

Back to your email:

On 05/01/18 23:13, David F. wrote:
> but looking at the algo in UEFI Spec 2.6 page 245, it looks like we'd
> always have to generate the hash, sign it, create all the PKCS stuff
> even in setup mode?    That would surely unnecessarily bloat any apps
> that really only need to update things in setup mode wouldn't it?   So
> to confirm, that is a requirement even in setup mode?    If so, why?

It's not a requirement; see the code comment I quoted above.

Thanks,
Laszlo


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Set "db" variable in secure boot setup mode still requires generating PKCS#7?
  2018-05-02 10:21 ` Laszlo Ersek
@ 2018-05-02 16:26   ` David F.
  2018-05-03  3:09     ` Long, Qin
  0 siblings, 1 reply; 8+ messages in thread
From: David F. @ 2018-05-02 16:26 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: edk2 developers list

This Intel mobo didn't like?  This is the code snippet that builds it:

// calc size of header (with no certdata) and crt file data to add
size_t authhdrsize;
size_t siglisthdrsize;

if (applyrawdata) {
  authhdrsize=0;
  siglisthdrsize=0;
}
else {
  authhdrsize=offsetof(EFI_VARIABLE_AUTHENTICATION_2,
AuthInfo)+offsetof(WIN_CERTIFICATE_UEFI_GUID, CertData);
  siglisthdrsize=sizeof(EFI_SIGNATURE_LIST)+offsetof(EFI_SIGNATURE_DATA,
SignatureData);
}
size_t tempbufsize=ffinfo.FileSize+authhdrsize+siglisthdrsize;

BYTE *tempbuf;
if ((tempbuf=new BYTE [tempbufsize])!=NULL) {
  // variable to determine where to read file
  BYTE *certdata=tempbuf;
  // determine if need to prefix .crt for kek/db entries
  if (!applyrawdata) {
    // zero header part of buffer so all are init to zero
    memset(tempbuf, 0, authhdrsize+siglisthdrsize);
    //
    // setup EFI_VARIABLE_AUTHENTICATION_2  header
    //
    EFI_VARIABLE_AUTHENTICATION_2
*efivarauth2=(EFI_VARIABLE_AUTHENTICATION_2 *) tempbuf;
    // setup time
    TimeTToUEFITimeGMT(time(NULL), &efivarauth2->TimeStamp);
    efivarauth2->TimeStamp.Nanosecond=0;
    // setup authinfo (without any CertData)
    efivarauth2->AuthInfo.Hdr.dwLength=offsetof(WIN_CERTIFICATE_UEFI_GUID,
CertData);
    efivarauth2->AuthInfo.Hdr.wRevision=0x200;
    efivarauth2->AuthInfo.Hdr.wCertificateType=WIN_CERT_TYPE_EFI_GUID;
    efivarauth2->AuthInfo.CertType=gEfiCertPkcs7Guid;
    //
    // setup EFI_SIGNATURE_LIST
    //
    EFI_SIGNATURE_LIST *efisiglist=(EFI_SIGNATURE_LIST *)
(tempbuf+authhdrsize);
    efisiglist->SignatureType=gEfiCertX509Guid;

efisiglist->SignatureListSize=(uint32_t)(ffinfo.FileSize+siglisthdrsize);
    efisiglist->SignatureHeaderSize=0;
    efisiglist->SignatureSize=ffinfo.FileSize+offsetof(EFI_SIGNATURE_DATA,
SignatureData);
    //
    // setup EFI_SIGNATURE_DATA  (no owner)
    //
    EFI_SIGNATURE_DATA *efisigdata=(EFI_SIGNATURE_DATA *)
((BYTE*)efisiglist+sizeof(EFI_SIGNATURE_LIST)+efisiglist->SignatureHeaderSize);
    certdata=efisigdata->SignatureData;
  }
  // Read file to buffer
  if ((errcode=FSOpenReadCloseFile(openpath, certdata, 0, ffinfo.FileSize,
NULL, filesys))==ERROR_NONE) {
    // have the data, now write it to the correct variable
    uint32_t varattr=EFI_VARIABLE_NON_VOLATILE|
                     EFI_VARIABLE_BOOTSERVICE_ACCESS|
                     EFI_VARIABLE_RUNTIME_ACCESS|
                     EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
    if (!rparam) {
      varattr|=EFI_VARIABLE_APPEND_WRITE;
    }

    // update variable
    errcode=UEFISetVariable(varname, guidstr, tempbuf, tempbufsize,
varattr);
  }
  // clean up
  delete[] tempbuf;
}


On Wed, May 2, 2018 at 3:21 AM, Laszlo Ersek <lersek@redhat.com> wrote:

> On 05/01/18 23:13, David F. wrote:
> > Hi,
> >
> > Had a fairly simple task of wanting to install the latest MS .crt
> > files for KEK, and their two files for the "db" (the Windows CA and
> > UEFI CA) in a system placed in setup/custom mode.  However, even
> > though it seemed to take the KEK, it never took the "db", always had a
> > problem on a DH77KC mobo (dumped data headers looked as expected). Now
> > when I constructed it, I thought I could leave out any PKCS#7 data
> > (set the expected CertType but in the Hdr dwLength only included
> > CertType and not any CertData),
>
> Right, I've stumbled upon that too. According to the UEFI spec, dwLength
> should include CertData too, but edk2 does *not* accept that. This can
> be seen e.g. in
> "SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/
> SecureBootConfigImpl.c",
> function CreateTimeBasedPayload():
>
> >   //
> >   // In Setup mode or Custom mode, the variable does not need to be
> signed but the
> >   // parameters to the SetVariable() call still need to be prepared as
> authenticated
> >   // variable. So we create EFI_VARIABLE_AUTHENTICATED_2 descriptor
> without certificate
> >   // data in it.
> >   //
> > ...
> >   DescriptorData->AuthInfo.Hdr.dwLength         = OFFSET_OF
> (WIN_CERTIFICATE_UEFI_GUID, CertData);
>
> Back to your email:
>
> On 05/01/18 23:13, David F. wrote:
> > but looking at the algo in UEFI Spec 2.6 page 245, it looks like we'd
> > always have to generate the hash, sign it, create all the PKCS stuff
> > even in setup mode?    That would surely unnecessarily bloat any apps
> > that really only need to update things in setup mode wouldn't it?   So
> > to confirm, that is a requirement even in setup mode?    If so, why?
>
> It's not a requirement; see the code comment I quoted above.
>
> Thanks,
> Laszlo
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Set "db" variable in secure boot setup mode still requires generating PKCS#7?
  2018-05-02 16:26   ` David F.
@ 2018-05-03  3:09     ` Long, Qin
  2018-05-20 19:54       ` David F.
  0 siblings, 1 reply; 8+ messages in thread
From: Long, Qin @ 2018-05-03  3:09 UTC (permalink / raw)
  To: David F., Laszlo Ersek; +Cc: edk2 developers list

Hi, David,

Yes, in Setup / Custom mode, no need to generate the AuthData for verification. It's good enough to create the AUTH_2 descriptor / headers without CertData as the parameter for SetVariable() call.

Do you mean this code snippet can succeed to enroll KEK, but fail to enroll DB data?
The data initialization from code snippet looks good. What's the returned errcode value? (And one reminder is that KEK and DB are binding with different vendor GUID: gEfiGlobalVariableGuid, and gEfiImageSecurityDatabaseGuid).


Best Regards & Thanks,
LONG, Qin

From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of David F.
Sent: Thursday, May 3, 2018 12:26 AM
To: Laszlo Ersek <lersek@redhat.com>
Cc: edk2 developers list <edk2-devel@lists.01.org>
Subject: Re: [edk2] Set "db" variable in secure boot setup mode still requires generating PKCS#7?

This Intel mobo didn't like?  This is the code snippet that builds it:

// calc size of header (with no certdata) and crt file data to add
size_t authhdrsize;
size_t siglisthdrsize;

if (applyrawdata) {
  authhdrsize=0;
  siglisthdrsize=0;
}
else {
  authhdrsize=offsetof(EFI_VARIABLE_AUTHENTICATION_2,
AuthInfo)+offsetof(WIN_CERTIFICATE_UEFI_GUID, CertData);
  siglisthdrsize=sizeof(EFI_SIGNATURE_LIST)+offsetof(EFI_SIGNATURE_DATA,
SignatureData);
}
size_t tempbufsize=ffinfo.FileSize+authhdrsize+siglisthdrsize;

BYTE *tempbuf;
if ((tempbuf=new BYTE [tempbufsize])!=NULL) {
  // variable to determine where to read file
  BYTE *certdata=tempbuf;
  // determine if need to prefix .crt for kek/db entries
  if (!applyrawdata) {
    // zero header part of buffer so all are init to zero
    memset(tempbuf, 0, authhdrsize+siglisthdrsize);
    //
    // setup EFI_VARIABLE_AUTHENTICATION_2  header
    //
    EFI_VARIABLE_AUTHENTICATION_2
*efivarauth2=(EFI_VARIABLE_AUTHENTICATION_2 *) tempbuf;
    // setup time
    TimeTToUEFITimeGMT(time(NULL), &efivarauth2->TimeStamp);
    efivarauth2->TimeStamp.Nanosecond=0;
    // setup authinfo (without any CertData)
    efivarauth2->AuthInfo.Hdr.dwLength=offsetof(WIN_CERTIFICATE_UEFI_GUID,
CertData);
    efivarauth2->AuthInfo.Hdr.wRevision=0x200;
    efivarauth2->AuthInfo.Hdr.wCertificateType=WIN_CERT_TYPE_EFI_GUID;
    efivarauth2->AuthInfo.CertType=gEfiCertPkcs7Guid;
    //
    // setup EFI_SIGNATURE_LIST
    //
    EFI_SIGNATURE_LIST *efisiglist=(EFI_SIGNATURE_LIST *)
(tempbuf+authhdrsize);
    efisiglist->SignatureType=gEfiCertX509Guid;

efisiglist->SignatureListSize=(uint32_t)(ffinfo.FileSize+siglisthdrsize);
    efisiglist->SignatureHeaderSize=0;
    efisiglist->SignatureSize=ffinfo.FileSize+offsetof(EFI_SIGNATURE_DATA,
SignatureData);
    //
    // setup EFI_SIGNATURE_DATA  (no owner)
    //
    EFI_SIGNATURE_DATA *efisigdata=(EFI_SIGNATURE_DATA *)
((BYTE*)efisiglist+sizeof(EFI_SIGNATURE_LIST)+efisiglist->SignatureHeaderSize);
    certdata=efisigdata->SignatureData;
  }
  // Read file to buffer
  if ((errcode=FSOpenReadCloseFile(openpath, certdata, 0, ffinfo.FileSize,
NULL, filesys))==ERROR_NONE) {
    // have the data, now write it to the correct variable
    uint32_t varattr=EFI_VARIABLE_NON_VOLATILE|
                     EFI_VARIABLE_BOOTSERVICE_ACCESS|
                     EFI_VARIABLE_RUNTIME_ACCESS|
                     EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
    if (!rparam) {
      varattr|=EFI_VARIABLE_APPEND_WRITE;
    }

    // update variable
    errcode=UEFISetVariable(varname, guidstr, tempbuf, tempbufsize,
varattr);
  }
  // clean up
  delete[] tempbuf;
}


On Wed, May 2, 2018 at 3:21 AM, Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>> wrote:

> On 05/01/18 23:13, David F. wrote:
> > Hi,
> >
> > Had a fairly simple task of wanting to install the latest MS .crt
> > files for KEK, and their two files for the "db" (the Windows CA and
> > UEFI CA) in a system placed in setup/custom mode.  However, even
> > though it seemed to take the KEK, it never took the "db", always had a
> > problem on a DH77KC mobo (dumped data headers looked as expected). Now
> > when I constructed it, I thought I could leave out any PKCS#7 data
> > (set the expected CertType but in the Hdr dwLength only included
> > CertType and not any CertData),
>
> Right, I've stumbled upon that too. According to the UEFI spec, dwLength
> should include CertData too, but edk2 does *not* accept that. This can
> be seen e.g. in
> "SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/
> SecureBootConfigImpl.c",
> function CreateTimeBasedPayload():
>
> >   //
> >   // In Setup mode or Custom mode, the variable does not need to be
> signed but the
> >   // parameters to the SetVariable() call still need to be prepared as
> authenticated
> >   // variable. So we create EFI_VARIABLE_AUTHENTICATED_2 descriptor
> without certificate
> >   // data in it.
> >   //
> > ...
> >   DescriptorData->AuthInfo.Hdr.dwLength         = OFFSET_OF
> (WIN_CERTIFICATE_UEFI_GUID, CertData);
>
> Back to your email:
>
> On 05/01/18 23:13, David F. wrote:
> > but looking at the algo in UEFI Spec 2.6 page 245, it looks like we'd
> > always have to generate the hash, sign it, create all the PKCS stuff
> > even in setup mode?    That would surely unnecessarily bloat any apps
> > that really only need to update things in setup mode wouldn't it?   So
> > to confirm, that is a requirement even in setup mode?    If so, why?
>
> It's not a requirement; see the code comment I quoted above.
>
> Thanks,
> Laszlo
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Set "db" variable in secure boot setup mode still requires generating PKCS#7?
  2018-05-03  3:09     ` Long, Qin
@ 2018-05-20 19:54       ` David F.
  2018-05-21  1:46         ` Zhang, Chao B
  0 siblings, 1 reply; 8+ messages in thread
From: David F. @ 2018-05-20 19:54 UTC (permalink / raw)
  To: Long, Qin; +Cc: Laszlo Ersek, edk2 developers list

>Do you mean this code snippet can succeed to enroll KEK, but fail to
enroll DB data?

Correct.


> What’s the returned errcode value? (And one reminder is that KEK and DB
are binding with different vendor GUID: gEfiGlobalVariableGuid, and
gEfiImageSecurityDatabaseGuid).

It was a access denied type of error code (I'd have to enable it again to
test the exact code).  Yes, the GUID was set different for the database
(wasn't able to save anything using the database guid).


On Wed, May 2, 2018 at 8:09 PM, Long, Qin <qin.long@intel.com> wrote:

> Hi, David,
>
>
>
> Yes, in Setup / Custom mode, no need to generate the AuthData for
> verification. It’s good enough to create the AUTH_2 descriptor / headers
> without CertData as the parameter for SetVariable() call.
>
>
>
> Do you mean this code snippet can succeed to enroll KEK, but fail to
> enroll DB data?
>
> The data initialization from code snippet looks good. What’s the returned
> errcode value? (And one reminder is that KEK and DB are binding with
> different vendor GUID: gEfiGlobalVariableGuid, and
> gEfiImageSecurityDatabaseGuid).
>
>
>
>
>
> Best Regards & Thanks,
>
> LONG, Qin
>
>
>
> *From:* edk2-devel [mailto:edk2-devel-bounces@lists.01.org] *On Behalf Of
> *David F.
> *Sent:* Thursday, May 3, 2018 12:26 AM
> *To:* Laszlo Ersek <lersek@redhat.com>
> *Cc:* edk2 developers list <edk2-devel@lists.01.org>
> *Subject:* Re: [edk2] Set "db" variable in secure boot setup mode still
> requires generating PKCS#7?
>
>
>
> This Intel mobo didn't like?  This is the code snippet that builds it:
>
>
> // calc size of header (with no certdata) and crt file data to add
> size_t authhdrsize;
> size_t siglisthdrsize;
>
> if (applyrawdata) {
>   authhdrsize=0;
>   siglisthdrsize=0;
> }
> else {
>   authhdrsize=offsetof(EFI_VARIABLE_AUTHENTICATION_2,
> AuthInfo)+offsetof(WIN_CERTIFICATE_UEFI_GUID, CertData);
>   siglisthdrsize=sizeof(EFI_SIGNATURE_LIST)+offsetof(EFI_SIGNATURE_DATA,
> SignatureData);
> }
> size_t tempbufsize=ffinfo.FileSize+authhdrsize+siglisthdrsize;
>
> BYTE *tempbuf;
> if ((tempbuf=new BYTE [tempbufsize])!=NULL) {
>   // variable to determine where to read file
>   BYTE *certdata=tempbuf;
>   // determine if need to prefix .crt for kek/db entries
>   if (!applyrawdata) {
>     // zero header part of buffer so all are init to zero
>     memset(tempbuf, 0, authhdrsize+siglisthdrsize);
>     //
>     // setup EFI_VARIABLE_AUTHENTICATION_2  header
>     //
>     EFI_VARIABLE_AUTHENTICATION_2
> *efivarauth2=(EFI_VARIABLE_AUTHENTICATION_2 *) tempbuf;
>     // setup time
>     TimeTToUEFITimeGMT(time(NULL), &efivarauth2->TimeStamp);
>     efivarauth2->TimeStamp.Nanosecond=0;
>     // setup authinfo (without any CertData)
>     efivarauth2->AuthInfo.Hdr.dwLength=offsetof(WIN_CERTIFICATE_UEFI_GUID,
> CertData);
>     efivarauth2->AuthInfo.Hdr.wRevision=0x200;
>     efivarauth2->AuthInfo.Hdr.wCertificateType=WIN_CERT_TYPE_EFI_GUID;
>     efivarauth2->AuthInfo.CertType=gEfiCertPkcs7Guid;
>     //
>     // setup EFI_SIGNATURE_LIST
>     //
>     EFI_SIGNATURE_LIST *efisiglist=(EFI_SIGNATURE_LIST *)
> (tempbuf+authhdrsize);
>     efisiglist->SignatureType=gEfiCertX509Guid;
>
> efisiglist->SignatureListSize=(uint32_t)(ffinfo.FileSize+siglisthdrsize);
>     efisiglist->SignatureHeaderSize=0;
>     efisiglist->SignatureSize=ffinfo.FileSize+offsetof(EFI_SIGNATURE_DATA,
> SignatureData);
>     //
>     // setup EFI_SIGNATURE_DATA  (no owner)
>     //
>     EFI_SIGNATURE_DATA *efisigdata=(EFI_SIGNATURE_DATA *)
> ((BYTE*)efisiglist+sizeof(EFI_SIGNATURE_LIST)+efisiglist->
> SignatureHeaderSize);
>     certdata=efisigdata->SignatureData;
>   }
>   // Read file to buffer
>   if ((errcode=FSOpenReadCloseFile(openpath, certdata, 0, ffinfo.FileSize,
> NULL, filesys))==ERROR_NONE) {
>     // have the data, now write it to the correct variable
>     uint32_t varattr=EFI_VARIABLE_NON_VOLATILE|
>                      EFI_VARIABLE_BOOTSERVICE_ACCESS|
>                      EFI_VARIABLE_RUNTIME_ACCESS|
>                      EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
>     if (!rparam) {
>       varattr|=EFI_VARIABLE_APPEND_WRITE;
>     }
>
>     // update variable
>     errcode=UEFISetVariable(varname, guidstr, tempbuf, tempbufsize,
> varattr);
>   }
>   // clean up
>   delete[] tempbuf;
> }
>
>
> On Wed, May 2, 2018 at 3:21 AM, Laszlo Ersek <lersek@redhat.com> wrote:
>
> > On 05/01/18 23:13, David F. wrote:
> > > Hi,
> > >
> > > Had a fairly simple task of wanting to install the latest MS .crt
> > > files for KEK, and their two files for the "db" (the Windows CA and
> > > UEFI CA) in a system placed in setup/custom mode.  However, even
> > > though it seemed to take the KEK, it never took the "db", always had a
> > > problem on a DH77KC mobo (dumped data headers looked as expected). Now
> > > when I constructed it, I thought I could leave out any PKCS#7 data
> > > (set the expected CertType but in the Hdr dwLength only included
> > > CertType and not any CertData),
> >
> > Right, I've stumbled upon that too. According to the UEFI spec, dwLength
> > should include CertData too, but edk2 does *not* accept that. This can
> > be seen e.g. in
> > "SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/
> > SecureBootConfigImpl.c",
> > function CreateTimeBasedPayload():
> >
> > >   //
> > >   // In Setup mode or Custom mode, the variable does not need to be
> > signed but the
> > >   // parameters to the SetVariable() call still need to be prepared as
> > authenticated
> > >   // variable. So we create EFI_VARIABLE_AUTHENTICATED_2 descriptor
> > without certificate
> > >   // data in it.
> > >   //
> > > ...
> > >   DescriptorData->AuthInfo.Hdr.dwLength         = OFFSET_OF
> > (WIN_CERTIFICATE_UEFI_GUID, CertData);
> >
> > Back to your email:
> >
> > On 05/01/18 23:13, David F. wrote:
> > > but looking at the algo in UEFI Spec 2.6 page 245, it looks like we'd
> > > always have to generate the hash, sign it, create all the PKCS stuff
> > > even in setup mode?    That would surely unnecessarily bloat any apps
> > > that really only need to update things in setup mode wouldn't it?   So
> > > to confirm, that is a requirement even in setup mode?    If so, why?
> >
> > It's not a requirement; see the code comment I quoted above.
> >
> > Thanks,
> > Laszlo
> >
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Set "db" variable in secure boot setup mode still requires generating PKCS#7?
  2018-05-20 19:54       ` David F.
@ 2018-05-21  1:46         ` Zhang, Chao B
  0 siblings, 0 replies; 8+ messages in thread
From: Zhang, Chao B @ 2018-05-21  1:46 UTC (permalink / raw)
  To: David F., Long, Qin; +Cc: edk2 developers list, Laszlo Ersek

David:
   Have you tried to enroll .crt from HII Secure Boot Configure Page?
Basically when PK exists , PhysicalPresence and Customized Mode must be asserted in order  to enroll a signature without CertData to KEK/DB…,

From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of David F.
Sent: Monday, May 21, 2018 3:54 AM
To: Long, Qin <qin.long@intel.com>
Cc: edk2 developers list <edk2-devel@lists.01.org>; Laszlo Ersek <lersek@redhat.com>
Subject: Re: [edk2] Set "db" variable in secure boot setup mode still requires generating PKCS#7?

>Do you mean this code snippet can succeed to enroll KEK, but fail to
enroll DB data?

Correct.


> What’s the returned errcode value? (And one reminder is that KEK and DB
are binding with different vendor GUID: gEfiGlobalVariableGuid, and
gEfiImageSecurityDatabaseGuid).

It was a access denied type of error code (I'd have to enable it again to
test the exact code).  Yes, the GUID was set different for the database
(wasn't able to save anything using the database guid).


On Wed, May 2, 2018 at 8:09 PM, Long, Qin <qin.long@intel.com<mailto:qin.long@intel.com>> wrote:

> Hi, David,
>
>
>
> Yes, in Setup / Custom mode, no need to generate the AuthData for
> verification. It’s good enough to create the AUTH_2 descriptor / headers
> without CertData as the parameter for SetVariable() call.
>
>
>
> Do you mean this code snippet can succeed to enroll KEK, but fail to
> enroll DB data?
>
> The data initialization from code snippet looks good. What’s the returned
> errcode value? (And one reminder is that KEK and DB are binding with
> different vendor GUID: gEfiGlobalVariableGuid, and
> gEfiImageSecurityDatabaseGuid).
>
>
>
>
>
> Best Regards & Thanks,
>
> LONG, Qin
>
>
>
> *From:* edk2-devel [mailto:edk2-devel-bounces@lists.01.org] *On Behalf Of
> *David F.
> *Sent:* Thursday, May 3, 2018 12:26 AM
> *To:* Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
> *Cc:* edk2 developers list <edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>>
> *Subject:* Re: [edk2] Set "db" variable in secure boot setup mode still
> requires generating PKCS#7?
>
>
>
> This Intel mobo didn't like?  This is the code snippet that builds it:
>
>
> // calc size of header (with no certdata) and crt file data to add
> size_t authhdrsize;
> size_t siglisthdrsize;
>
> if (applyrawdata) {
>   authhdrsize=0;
>   siglisthdrsize=0;
> }
> else {
>   authhdrsize=offsetof(EFI_VARIABLE_AUTHENTICATION_2,
> AuthInfo)+offsetof(WIN_CERTIFICATE_UEFI_GUID, CertData);
>   siglisthdrsize=sizeof(EFI_SIGNATURE_LIST)+offsetof(EFI_SIGNATURE_DATA,
> SignatureData);
> }
> size_t tempbufsize=ffinfo.FileSize+authhdrsize+siglisthdrsize;
>
> BYTE *tempbuf;
> if ((tempbuf=new BYTE [tempbufsize])!=NULL) {
>   // variable to determine where to read file
>   BYTE *certdata=tempbuf;
>   // determine if need to prefix .crt for kek/db entries
>   if (!applyrawdata) {
>     // zero header part of buffer so all are init to zero
>     memset(tempbuf, 0, authhdrsize+siglisthdrsize);
>     //
>     // setup EFI_VARIABLE_AUTHENTICATION_2  header
>     //
>     EFI_VARIABLE_AUTHENTICATION_2
> *efivarauth2=(EFI_VARIABLE_AUTHENTICATION_2 *) tempbuf;
>     // setup time
>     TimeTToUEFITimeGMT(time(NULL), &efivarauth2->TimeStamp);
>     efivarauth2->TimeStamp.Nanosecond=0;
>     // setup authinfo (without any CertData)
>     efivarauth2->AuthInfo.Hdr.dwLength=offsetof(WIN_CERTIFICATE_UEFI_GUID,
> CertData);
>     efivarauth2->AuthInfo.Hdr.wRevision=0x200;
>     efivarauth2->AuthInfo.Hdr.wCertificateType=WIN_CERT_TYPE_EFI_GUID;
>     efivarauth2->AuthInfo.CertType=gEfiCertPkcs7Guid;
>     //
>     // setup EFI_SIGNATURE_LIST
>     //
>     EFI_SIGNATURE_LIST *efisiglist=(EFI_SIGNATURE_LIST *)
> (tempbuf+authhdrsize);
>     efisiglist->SignatureType=gEfiCertX509Guid;
>
> efisiglist->SignatureListSize=(uint32_t)(ffinfo.FileSize+siglisthdrsize);
>     efisiglist->SignatureHeaderSize=0;
>     efisiglist->SignatureSize=ffinfo.FileSize+offsetof(EFI_SIGNATURE_DATA,
> SignatureData);
>     //
>     // setup EFI_SIGNATURE_DATA  (no owner)
>     //
>     EFI_SIGNATURE_DATA *efisigdata=(EFI_SIGNATURE_DATA *)
> ((BYTE*)efisiglist+sizeof(EFI_SIGNATURE_LIST)+efisiglist->
> SignatureHeaderSize);
>     certdata=efisigdata->SignatureData;
>   }
>   // Read file to buffer
>   if ((errcode=FSOpenReadCloseFile(openpath, certdata, 0, ffinfo.FileSize,
> NULL, filesys))==ERROR_NONE) {
>     // have the data, now write it to the correct variable
>     uint32_t varattr=EFI_VARIABLE_NON_VOLATILE|
>                      EFI_VARIABLE_BOOTSERVICE_ACCESS|
>                      EFI_VARIABLE_RUNTIME_ACCESS|
>                      EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
>     if (!rparam) {
>       varattr|=EFI_VARIABLE_APPEND_WRITE;
>     }
>
>     // update variable
>     errcode=UEFISetVariable(varname, guidstr, tempbuf, tempbufsize,
> varattr);
>   }
>   // clean up
>   delete[] tempbuf;
> }
>
>
> On Wed, May 2, 2018 at 3:21 AM, Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>> wrote:
>
> > On 05/01/18 23:13, David F. wrote:
> > > Hi,
> > >
> > > Had a fairly simple task of wanting to install the latest MS .crt
> > > files for KEK, and their two files for the "db" (the Windows CA and
> > > UEFI CA) in a system placed in setup/custom mode.  However, even
> > > though it seemed to take the KEK, it never took the "db", always had a
> > > problem on a DH77KC mobo (dumped data headers looked as expected). Now
> > > when I constructed it, I thought I could leave out any PKCS#7 data
> > > (set the expected CertType but in the Hdr dwLength only included
> > > CertType and not any CertData),
> >
> > Right, I've stumbled upon that too. According to the UEFI spec, dwLength
> > should include CertData too, but edk2 does *not* accept that. This can
> > be seen e.g. in
> > "SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/
> > SecureBootConfigImpl.c",
> > function CreateTimeBasedPayload():
> >
> > >   //
> > >   // In Setup mode or Custom mode, the variable does not need to be
> > signed but the
> > >   // parameters to the SetVariable() call still need to be prepared as
> > authenticated
> > >   // variable. So we create EFI_VARIABLE_AUTHENTICATED_2 descriptor
> > without certificate
> > >   // data in it.
> > >   //
> > > ...
> > >   DescriptorData->AuthInfo.Hdr.dwLength         = OFFSET_OF
> > (WIN_CERTIFICATE_UEFI_GUID, CertData);
> >
> > Back to your email:
> >
> > On 05/01/18 23:13, David F. wrote:
> > > but looking at the algo in UEFI Spec 2.6 page 245, it looks like we'd
> > > always have to generate the hash, sign it, create all the PKCS stuff
> > > even in setup mode?    That would surely unnecessarily bloat any apps
> > > that really only need to update things in setup mode wouldn't it?   So
> > > to confirm, that is a requirement even in setup mode?    If so, why?
> >
> > It's not a requirement; see the code comment I quoted above.
> >
> > Thanks,
> > Laszlo
> >
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-05-21  1:46 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-01 21:13 Set "db" variable in secure boot setup mode still requires generating PKCS#7? David F.
2018-05-01 21:25 ` Bill Paul
2018-05-02  2:23   ` David F.
2018-05-02 10:21 ` Laszlo Ersek
2018-05-02 16:26   ` David F.
2018-05-03  3:09     ` Long, Qin
2018-05-20 19:54       ` David F.
2018-05-21  1:46         ` Zhang, Chao B

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox