Interpretation of specification

Interpretation of specification

[phlamorim]

[-- Attachment #1: Type: text/plain, Size: 1625 bytes --]

The following commit on edk2 added a new a check on format of Authenticated variables: https://github.com/tianocore/edk2/commit/c035e37335ae43229d7e68de74a65f2c01ebc0af ( https://github.com/tianocore/edk2/commit/c035e37335ae43229d7e68de74a65f2c01ebc0af )

After this point some implementations started to have differences in the validation of the format of Authenticator Descriptor as we can se here: https://blog.hansenpartnership.com/uefi-secure-boot/#comment-48351

This case make me reach the following discussions: https://bugzilla.tianocore.org/show_bug.cgi?id=586 where i have seen lots of tools(more on linux) dont generate the correct format to use on the payload for TimeBased authenticated variables, at this time i was just trying to create a private authenticated variable, first thig which worked is to use this patch: https://patchew.org/EDK2/1525903747.5882.11.camel@HansenPartnership.com/ ( https://patchew.org/EDK2/1525903747.5882.11.camel@HansenPartnership.com/ )

But this patch never got merged, so i realized the tools on linux received upgrades to work properly with Authentication Variables of the edk2, but the same code just dont worked at a real machine using a ASROCK board. So my question is, where the developers should trust to consume the UEFI APIs in a trusted way. Another example i had is the path separator "\" or "/", edk2 uses "/" but the spec allow both, ASROCK mix both sometimes it can lead to some errors. I want to know if i can trust if my code work on EDK2 it should work on all other implementations too, and how we will try to remove these ambiguities of interpretation.

[-- Attachment #2: Type: text/html, Size: 1847 bytes --]

Re: [edk2-devel] Interpretation of specification

[Laszlo Ersek]

On 10/23/19 19:12, phlamorim@riseup.net wrote:
> The following commit on edk2 added a new a check on format of Authenticated variables: https://github.com/tianocore/edk2/commit/c035e37335ae43229d7e68de74a65f2c01ebc0af ( https://github.com/tianocore/edk2/commit/c035e37335ae43229d7e68de74a65f2c01ebc0af )
> 
> After this point some implementations started to have differences in the validation of the format of Authenticator Descriptor as we can se here: https://blog.hansenpartnership.com/uefi-secure-boot/#comment-48351
> 
> This case make me reach the following discussions: https://bugzilla.tianocore.org/show_bug.cgi?id=586 where i have seen lots of tools(more on linux) dont generate the correct format to use on the payload for TimeBased authenticated variables, at this time i was just trying to create a private authenticated variable, first thig which worked is to use this patch: https://patchew.org/EDK2/1525903747.5882.11.camel@HansenPartnership.com/ ( https://patchew.org/EDK2/1525903747.5882.11.camel@HansenPartnership.com/ )
> 
> But this patch never got merged, so i realized the tools on linux received upgrades to work properly with Authentication Variables of the edk2, but the same code just dont worked at a real machine using a ASROCK board. So my question is, where the developers should trust to consume the UEFI APIs in a trusted way. Another example i had is the path separator "\" or "/", edk2 uses "/" but the spec allow both, ASROCK mix both sometimes it can lead to some errors. I want to know if i can trust if my code work on EDK2 it should work on all other implementations too, and how we will try to remove these ambiguities of interpretation.

I suggest writing code against the published UEFI spec, and testing at
least with edk2. If edk2 does not behave in accordance with the UEFI
spec, then it could be a bug in edk2, or in the spec. Report the issue
on edk2-devel, and the community will try to figure out which half is wrong.

If your code seems correct, according to both the UEFI spec and to an
open source edk2 platform, but it breaks on a particular vendor
platform, I suggest talking to that vendor.

So, I would suggest the following order of "targets":
- spec
- reference implementation (edk2)
- open source platforms (edk2-platforms)
- vendor platform

Thanks
Laszlo

Re: [edk2-devel] Interpretation of specification

[phlamorim]

[-- Attachment #1: Type: text/plain, Size: 5609 bytes --]

Im implementing a mechanism which will depends on time based
authenticated variables, but i got stucked when the SetVariable from
RuntimeServices return me a code of Security Violation. I started to
develop my code using the EDK2 and OVMF on QEMU to test the correctness
of my UEFI Applications, and the code worked properly to set the
authenticated variables on OVMF, but when i tried the same at the Dell
Inspiron which uses the AMI implementation as base of UEFI, i always got
a Security Violation return.

UEFI Spec give a lot of verifications about setting authenticated
variables, and my code worked on the open source reference
implementation, so i got stucked because i cant found out which
validation is making the SetVariable fails on Dell's BIOS implementation.

Im using an UEFI Application to call the SetVariable twice one for set
the variable file_name and other to set variable file_hash, i used a
script to prepare the payload with the Auth Descriptor using well known
tools: openssl(1.1.1c 28 May 2019) and sbvarsign(0.9.2).

- genkeys.sh https://pastebin.com/jjrZGrAB
This is the script im using to create the keys

- create_auth_var_files.sh https://pastebin.com/jPQgG0nB
This is the script to generate the payloads used in the UEFI Application
to set the authenticated variables, the toc16 is just a code to convert
an char to CHAR16, the script receive one parameter, which is a file
name, so it create two files file_name.signed and file_hash.signed,
which are the payloads to set variable file_name and file_hash using
time based authentication.

- TestPkg.c https://pastebin.com/DnwhfuKk This one is the UEFI
Application which call the SetVariable using the payloads generated by
create_auth_var_files.sh, TestPkg is supposed to run with the
file_*.signed in the same path, so it can open and copy the payload to
use on SetVariable.

I tried to follow your suggestion (though i skipped the opensource
plataform "target" because i dont have access to one). As i said, i'm
using a Dell Inspiron, so i opened a thread in Dell Community:

https://www.dell.com/community/Inspiron/SetVariable-from-UEFI-RuntimeServices-is-not-working-as-expected/m-p/7411885.


I informed the BIOS Version which is 2.9.0 from Dell Inc, but later i
receive a private message saying the support in this case is limited.

I got an output of UEFI v2.40 (American Megatrends, 0x0005000B) from
command ver in the UEFI Shell, i tried to contact the technical support
of AMI, which gave me the following answer:

"Unless the motherboard is manufactured by AMI, we are not authorized to
provide support for it. The motherboard manufacturers license our
generic BIOS and modify it to fit their motherboard specifications;
thus, we do not have access to the changes made to the BIOS. For
support, BIOS, and driver updates for your motherboard, please contact
your motherboard manufacturer. You can also contact the place of
purchase and inquire if they can provide assistance for you."

I need to know where in the verification my code is returning a Security
Violation so i can fix, i can provide all the other files eg: inf and
dsc files used to compile the code and the directory tree im using too.

Regards

Em 24/10/2019 09:33, Laszlo Ersek escreveu:
> On 10/23/19 19:12, phlamorim@riseup.net wrote:
>> The following commit on edk2 added a new a check on format of Authenticated variables: https://github.com/tianocore/edk2/commit/c035e37335ae43229d7e68de74a65f2c01ebc0af ( https://github.com/tianocore/edk2/commit/c035e37335ae43229d7e68de74a65f2c01ebc0af )
>>
>> After this point some implementations started to have differences in the validation of the format of Authenticator Descriptor as we can se here: https://blog.hansenpartnership.com/uefi-secure-boot/#comment-48351
>>
>> This case make me reach the following discussions: https://bugzilla.tianocore.org/show_bug.cgi?id=586 where i have seen lots of tools(more on linux) dont generate the correct format to use on the payload for TimeBased authenticated variables, at this time i was just trying to create a private authenticated variable, first thig which worked is to use this patch: https://patchew.org/EDK2/1525903747.5882.11.camel@HansenPartnership.com/ ( https://patchew.org/EDK2/1525903747.5882.11.camel@HansenPartnership.com/ )
>>
>> But this patch never got merged, so i realized the tools on linux received upgrades to work properly with Authentication Variables of the edk2, but the same code just dont worked at a real machine using a ASROCK board. So my question is, where the developers should trust to consume the UEFI APIs in a trusted way. Another example i had is the path separator "\" or "/", edk2 uses "/" but the spec allow both, ASROCK mix both sometimes it can lead to some errors. I want to know if i can trust if my code work on EDK2 it should work on all other implementations too, and how we will try to remove these ambiguities of interpretation.
> I suggest writing code against the published UEFI spec, and testing at
> least with edk2. If edk2 does not behave in accordance with the UEFI
> spec, then it could be a bug in edk2, or in the spec. Report the issue
> on edk2-devel, and the community will try to figure out which half is wrong.
>
> If your code seems correct, according to both the UEFI spec and to an
> open source edk2 platform, but it breaks on a particular vendor
> platform, I suggest talking to that vendor.
>
> So, I would suggest the following order of "targets":
> - spec
> - reference implementation (edk2)
> - open source platforms (edk2-platforms)
> - vendor platform
>
> Thanks
> Laszlo
>
>
> 
>

[-- Attachment #2: Type: text/html, Size: 7846 bytes --]

Re: [edk2-devel] Interpretation of specification

[Laszlo Ersek]

Paulo,

On 11/15/19 19:51, Paulo Henrique Lacerda de Amorim wrote:
> Im implementing a mechanism which will depends on time based
> authenticated variables, but i got stucked when the SetVariable from
> RuntimeServices return me a code of Security Violation. I started to
> develop my code using the EDK2 and OVMF on QEMU to test the correctness
> of my UEFI Applications, and the code worked properly to set the
> authenticated variables on OVMF, but when i tried the same at the Dell
> Inspiron which uses the AMI implementation as base of UEFI, i always got
> a Security Violation return.
> 
> UEFI Spec give a lot of verifications about setting authenticated
> variables, and my code worked on the open source reference
> implementation, so i got stucked because i cant found out which
> validation is making the SetVariable fails on Dell's BIOS implementation.
> 
> Im using an UEFI Application to call the SetVariable twice one for set
> the variable file_name and other to set variable file_hash, i used a
> script to prepare the payload with the Auth Descriptor using well known
> tools: openssl(1.1.1c 28 May 2019) and sbvarsign(0.9.2).
> 
> - genkeys.sh https://pastebin.com/jjrZGrAB
> This is the script im using to create the keys
> 
> - create_auth_var_files.sh https://pastebin.com/jPQgG0nB
> This is the script to generate the payloads used in the UEFI Application
> to set the authenticated variables, the toc16 is just a code to convert
> an char to CHAR16, the script receive one parameter, which is a file
> name, so it create two files file_name.signed and file_hash.signed,
> which are the payloads to set variable file_name and file_hash using
> time based authentication.
> 
> - TestPkg.c https://pastebin.com/DnwhfuKk This one is the UEFI
> Application which call the SetVariable using the payloads generated by
> create_auth_var_files.sh, TestPkg is supposed to run with the
> file_*.signed in the same path, so it can open and copy the payload to
> use on SetVariable.
> 
> I tried to follow your suggestion (though i skipped the opensource
> plataform "target" because i dont have access to one). As i said, i'm
> using a Dell Inspiron, so i opened a thread in Dell Community:
> 
> https://www.dell.com/community/Inspiron/SetVariable-from-UEFI-RuntimeServices-is-not-working-as-expected/m-p/7411885.
> 
> 
> I informed the BIOS Version which is 2.9.0 from Dell Inc, but later i
> receive a private message saying the support in this case is limited.
> 
> I got an output of UEFI v2.40 (American Megatrends, 0x0005000B) from
> command ver in the UEFI Shell, i tried to contact the technical support
> of AMI, which gave me the following answer:
> 
> "Unless the motherboard is manufactured by AMI, we are not authorized to
> provide support for it. The motherboard manufacturers license our
> generic BIOS and modify it to fit their motherboard specifications;
> thus, we do not have access to the changes made to the BIOS. For
> support, BIOS, and driver updates for your motherboard, please contact
> your motherboard manufacturer. You can also contact the place of
> purchase and inquire if they can provide assistance for you."
> 
> I need to know where in the verification my code is returning a Security
> Violation so i can fix, i can provide all the other files eg: inf and
> dsc files used to compile the code and the directory tree im using too.

I'm sorry to say: I have no idea. Composing the payloads of
authenticated UEFI variables is one of the most difficult things I've
ever done in UEFI code, and whenever that fails for me, digging into
even the *open source* variable driver code, to find the exact reason,
is a nightmare. Unless you get direct help from Dell, I think your only
option is to dump the firmware from your motherboard, and then extract
some modules, and disassemble them. I've read that tools exist for this
kind of work, but I've never used them.

... Honestly, this is why open source should be the norm. :/

Perhaps you can experiment with other physical UEFI implementations,
especially development boards (I seeem to recall names such as
"Minnowboard" and "ValleyView" board).

I'm sorry that I can't help!
Laszlo



> 
> Regards
> 
> Em 24/10/2019 09:33, Laszlo Ersek escreveu:
>> On 10/23/19 19:12, phlamorim@riseup.net wrote:
>>> The following commit on edk2 added a new a check on format of Authenticated variables: https://github.com/tianocore/edk2/commit/c035e37335ae43229d7e68de74a65f2c01ebc0af ( https://github.com/tianocore/edk2/commit/c035e37335ae43229d7e68de74a65f2c01ebc0af )
>>>
>>> After this point some implementations started to have differences in the validation of the format of Authenticator Descriptor as we can se here: https://blog.hansenpartnership.com/uefi-secure-boot/#comment-48351
>>>
>>> This case make me reach the following discussions: https://bugzilla.tianocore.org/show_bug.cgi?id=586 where i have seen lots of tools(more on linux) dont generate the correct format to use on the payload for TimeBased authenticated variables, at this time i was just trying to create a private authenticated variable, first thig which worked is to use this patch: https://patchew.org/EDK2/1525903747.5882.11.camel@HansenPartnership.com/ ( https://patchew.org/EDK2/1525903747.5882.11.camel@HansenPartnership.com/ )
>>>
>>> But this patch never got merged, so i realized the tools on linux received upgrades to work properly with Authentication Variables of the edk2, but the same code just dont worked at a real machine using a ASROCK board. So my question is, where the developers should trust to consume the UEFI APIs in a trusted way. Another example i had is the path separator "\" or "/", edk2 uses "/" but the spec allow both, ASROCK mix both sometimes it can lead to some errors. I want to know if i can trust if my code work on EDK2 it should work on all other implementations too, and how we will try to remove these ambiguities of interpretation.
>> I suggest writing code against the published UEFI spec, and testing at
>> least with edk2. If edk2 does not behave in accordance with the UEFI
>> spec, then it could be a bug in edk2, or in the spec. Report the issue
>> on edk2-devel, and the community will try to figure out which half is wrong.
>>
>> If your code seems correct, according to both the UEFI spec and to an
>> open source edk2 platform, but it breaks on a particular vendor
>> platform, I suggest talking to that vendor.
>>
>> So, I would suggest the following order of "targets":
>> - spec
>> - reference implementation (edk2)
>> - open source platforms (edk2-platforms)
>> - vendor platform
>>
>> Thanks
>> Laszlo
>>
>>
>> 
>>
> 

Re: [edk2-devel] Interpretation of specification

[Eugene Khoruzhenko]

[-- Attachment #1: Type: text/plain, Size: 620 bytes --]

Hi Paulo,

Just to be clear - your variables have your own GUID and Name, so your variables are "common" Authenticated Variables, correct? What exactly is failing in your case:

* You cannot write your variable first time, so it does not get created?
* Or you can create, but cannot update after it's been created?

I seem to be able to create my Authenticated Variables on a number of production devices, including Dell, but then these variables cannot be deleted. I see exactly why deletion does not work - bug https://bugzilla.tianocore.org/show_bug.cgi?id=2374 , but this issue is specific to deletion only.

[-- Attachment #2: Type: text/html, Size: 727 bytes --]

Re: [edk2-devel] Interpretation of specification

[Paulo Henrique Lacerda de Amorim]

[-- Attachment #1: Type: text/plain, Size: 1456 bytes --]

Correct, im using my own GUID and Name, they started to name theses
variables Private Authenticated Variables since UEFI 2.7 as stated in
the session on how the firmware validate the payload to SetVariable
using EFI_VARIABLE_AUTHENTICATION2 descriptor:

"Otherwise, if the variable is none of the above, it shall be designated
a Private Authenticated Variable..."

In my case the first write is failing, i got a Security Violation return
when trying to create the variable, you used Key/cert which chains to
PK/KEK when creating variables on production devices? Maybe im missing
something. Let me know if i need to provide more information, as i
stated before i can provide the same scripts/sources im using.

Thanks in advance.

Em 23/11/2019 01:59, Eugene Khoruzhenko escreveu:
> Hi Paulo,
>
> Just to be clear - your variables have your own GUID and Name, so your
> variables are "common" Authenticated Variables, correct? What exactly
> is failing in your case:
>
>   * You cannot write your variable first time, so it does not get created?
>   * Or you can create, but cannot update after it's been created?
>
> I seem to be able to create my Authenticated Variables on a number of
> production devices, including Dell, but then these variables cannot be
> deleted. I see exactly why deletion does not work -
> bug https://bugzilla.tianocore.org/show_bug.cgi?id=2374, but this
> issue is specific to deletion only.
> 

[-- Attachment #2: Type: text/html, Size: 2732 bytes --]

Re: [edk2-devel] Interpretation of specification

[Eugene Khoruzhenko]

[-- Attachment #1: Type: text/plain, Size: 584 bytes --]

No, we do not have access to the manufacturer's PK/KEK, so I created my own keys and certs. Theoretically, to debug this you can send me the GUID/Name and payload you are trying to write, I can check if I can write your variable with my tool and signing. Then I could look at your code and compare with mine and see why it does not work. If your code works on my devices, maybe the specific model you have has some issue? BTW, try the other vendors, like Lenovo and HP. I only cannot promise when I will be able to get to it with holidays approaching and many other things to do...

[-- Attachment #2: Type: text/html, Size: 588 bytes --]

Re: [edk2-devel] Interpretation of specification

[Paulo Henrique Lacerda de Amorim]

[-- Attachment #1: Type: text/plain, Size: 1929 bytes --]

The GUID im using is 301d199a-4dc1-4b26-b557-a012d83d7a52 and the
variable names are file_name and file_hash, im using the following
script to generate my key/cert.
genkeys.sh: https://pastebin.com/iYEFLQD7

The payloads im trying to write is generated using a small script which
receive a single parameter which is a file name, the script just create
two files, file_name.txt with the file name converted to CHAR16 and
file_hash.txt with the SHA512 of the contents of the file. Then the
script uses sbvarsign to sign both, creating file_name.signed and
file_hash.signed using the previous generated keys.
create_auth_var_files.sh: https://pastebin.com/XhV9RbEB


Then with the payloads(file_name.signed and file_hash.signed) in the
same directory of my UEFI Application i run the application from the
UEFI Shell, which open these files, copy to a buffer and use them when
calling SetVariable.
TestPkg.c: https://pastebin.com/LbYvvrWH

The to16 is just a poor program to turn the passed parameter to
auth_create_var_files.sh in a valid CHAR16 string, as following
https://pastebin.com/AhjdzQrC.

The UEFI Application is just the TestPkg.c, i can upload the .inf and
.dsc files too if you want, and warn me if you want more information.

Em 26/11/2019 03:08, Eugene Khoruzhenko escreveu:
> No, we do not have access to the manufacturer's PK/KEK, so I created
> my own keys and certs. Theoretically, to debug this you can send me
> the GUID/Name and payload you are trying to write, I can check if I
> can write your variable with my tool and signing. Then I could look at
> your code and compare with mine and see why it does not work. If your
> code works on my devices, maybe the specific model you have has some
> issue? BTW, try the other vendors, like Lenovo and HP. I only cannot
> promise when I will be able to get to it with holidays approaching and
> many other things to do...
> 

[-- Attachment #2: Type: text/html, Size: 2857 bytes --]

Re: [edk2-devel] Interpretation of specification

[Eugene Khoruzhenko]

[-- Attachment #1: Type: text/plain, Size: 389 bytes --]

Hi Paulo, I finally got to looking into this and I was able to replicate your experiment. I created the payloads with your scripts and tried to write the "file_name" one with my firmware code and I get the error. So the problem may be not with the TestPkg.c but the way payloads are generated. I'll try to dig deeper and compare with my code that works OK, and figure it out if you want.

[-- Attachment #2: Type: text/html, Size: 389 bytes --]

Re: [edk2-devel] Interpretation of specification

[Paulo Henrique Lacerda de Amorim]

[-- Attachment #1: Type: text/plain, Size: 623 bytes --]

I Appreciate you replicated my experiment, and for sure it will be
helpful if you can figure out where the payload is wrong.

Let me know if i should provide more info.

Em 03/01/2020 16:52, Eugene Khoruzhenko escreveu:
> Hi Paulo, I finally got to looking into this and I was able to
> replicate your experiment. I created the payloads with your scripts
> and tried to write the "file_name" one with my firmware code and I get
> the error. So the problem may be not with the TestPkg.c but the way
> payloads are generated. I'll try to dig deeper and compare with my
> code that works OK, and figure it out if you want.
> 

[-- Attachment #2: Type: text/html, Size: 1054 bytes --]

Re: [edk2-devel] Interpretation of specification

[Eugene Khoruzhenko]

[-- Attachment #1: Type: text/plain, Size: 1065 bytes --]

I think I may have found the problem. I can write the file_name.signed created by your scripts in NT32 emulated environment and in EDKII on Minnow board that I build myself. However, I cannot write the file_name.signed on a commercial device. I can write the same authenticate variable with the same Name/GUID and same cert/key on a device when I create the payload in a UEFI Shell app. So the only difference is creating the signed payload by sbvarsign in Ubuntu vs doing it in UEFI. I compared both the working and non-working payloads and the main difference I see is in the timestamp. For some reason sbvarsign writes the Year as 0x0078 (120) vs the UEFI app writing 0x07e4 (2020). The month/day/hour/min seems to be OK, but the year is really off in the sbvarsign's payload. I cannot prove it, but I think the commercial firmware may be having a sanity check for the timestamp date/time, e.g. compare with the device manufacture date. Since sbvarsign does not allow setting a timestamp separately, I cannot force it to create a payload with the correct year.

[-- Attachment #2: Type: text/html, Size: 1193 bytes --]

Re: [edk2-devel] Interpretation of specification

[Laszlo Ersek]

(+James)

On 01/07/20 19:13, Eugene Khoruzhenko wrote:
> I think I may have found the problem. I can write the
> file_name.signed created by your scripts in NT32 emulated environment
> and in EDKII on Minnow board that I build myself. However, I cannot
> write the file_name.signed on a commercial device. I can write the
> same authenticate variable with the same Name/GUID and same cert/key
> on a device when I create the payload in a UEFI Shell app. So the
> only difference is creating the signed payload by sbvarsign in Ubuntu
> vs doing it in UEFI. I compared both the working and non-working
> payloads and the main difference I see is in the timestamp. For some
> reason sbvarsign writes the Year as 0x0078 (120) vs the UEFI app
> writing 0x07e4 (2020). The month/day/hour/min seems to be OK, but the
> year is really off in the sbvarsign's payload. I cannot prove it, but
> I think the commercial firmware may be having a sanity check for the
> timestamp date/time, e.g. compare with the device manufacture date.
> Since sbvarsign does not allow setting a timestamp separately, I
> cannot force it to create a payload with the correct year.

That looks like a bug in sbvarsign. In UEFI-2.8, the EFI_TIME structure
is defined under 8.3 "Time Services" / GetTime(). The Year field is
given like this:

  UINT16 Year; // 1900 - 9999

The comment indicates the valid range. It does not indicate that the
value stored should be relative to the year 1900 (which is what
sbvarsign appears to assume; 2020-1900=120).

The UEFI application likely uses GetTime(), for populating
"EFI_VARIABLE_AUTHENTICATION_2.TimeStamp" with the current time (which
is prescribed in 8.2.2 "Using the EFI_VARIABLE_AUTHENTICATION_2
descriptor").

After GetTime() returns successfully, the application may still need to
manually ensure that "Pad1, Nanosecond, TimeZone, Daylight and Pad2" are
zeroed. This could involve timezone translation (to the required UTC),
which in some cases could even imply a change to the Year field. But
that doesn't change the fact that GetTime() initally places an absolute
year number in the Year field, and not one relative to 1900.

Let me see the "sbsigntool" source code:

  https://git.kernel.org/pub/scm/linux/kernel/git/jejb/sbsigntools.git

(Repository URL via
<https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=920013>.)

Yup, here's the bug (at current master, that is, commit 0dc3d4b5210d,
"Fix PE/COFF checksum calculation", 2019-07-27):

  https://git.kernel.org/pub/scm/linux/kernel/git/jejb/sbsigntools.git/tree/src/sbvarsign.c?id=0dc3d4b5210dae158651d058f7ac68a9f178ae84#n215

Quote:

   199  static int set_timestamp(EFI_TIME *timestamp)
   200  {
   201          struct tm *tm;
   202          time_t t;
   203
   204          time(&t);
   205
   206          tm = gmtime(&t);
   207          if (!tm) {
   208                  perror("gmtime");
   209                  return -1;
   210          }
   211
   212          /* copy to our EFI-specific time structure. Other fields (Nanosecond,
   213           * TimeZone, Daylight and Pad) are defined to be zero */
   214          memset(timestamp, 0, sizeof(*timestamp));
   215          timestamp->Year = tm->tm_year;
   216          timestamp->Month = tm->tm_mon;
   217          timestamp->Day = tm->tm_mday;
   218          timestamp->Hour = tm->tm_hour;
   219          timestamp->Minute = tm->tm_min;
   220          timestamp->Second = tm->tm_sec;
   221
   222          return 0;
   223  }

Please refer to POSIX for time(), gmtime(), and "struct tm":

  https://pubs.opengroup.org/onlinepubs/9699919799/functions/time.html
  https://pubs.opengroup.org/onlinepubs/9699919799/functions/gmtime.html
  https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/time.h.html

Quoting the last reference:

> The <time.h> header shall declare the tm structure, which shall
> include at least the following members:
>
> int    tm_sec   Seconds [0,60].
> int    tm_min   Minutes [0,59].
> int    tm_hour  Hour [0,23].
> int    tm_mday  Day of month [1,31].
> int    tm_mon   Month of year [0,11].
> int    tm_year  Years since 1900.
> int    tm_wday  Day of week [0,6] (Sunday =0).
> int    tm_yday  Day of year [0,365].
> int    tm_isdst Daylight Savings flag.

That is, field "tm_year" is relative to 1900.

The bug is therefore in set_timestamp(), on line 215. It should be

  timestamp->Year = 1900 + tm->tm_year;

The bug goes back to commit 953b00481f39 ("sbvarsign: First cut of a
variable-signing tool", 2012-08-02).

(Otherwise, gmtime() is a good choice, because it gives us UTC at once,
which is what EFI_VARIABLE_AUTHENTICATION_2 requires, per UEFI spec
("GMT").)

I don't know where sbsigntools development occurs (mailing list, bug
tracker, ...). For now, I'm CC'ing James. (The git repo lives in his
space on <git.kernel.org>.)

James, the original thread on edk2-devel is here:

* [edk2-devel] Interpretation of specification
  https://edk2.groups.io/g/devel/message/49402

Thanks,
Laszlo

Re: [edk2-devel] Interpretation of specification

[James Bottomley]

On Wed, 2020-01-08 at 12:24 +0100, Laszlo Ersek wrote:
> (+James)
> 
> On 01/07/20 19:13, Eugene Khoruzhenko wrote:
> > I think I may have found the problem. I can write the
> > file_name.signed created by your scripts in NT32 emulated
> > environment and in EDKII on Minnow board that I build myself.
> > However, I cannot write the file_name.signed on a commercial
> > device. I can write the same authenticate variable with the same
> > Name/GUID and same cert/key on a device when I create the payload
> > in a UEFI Shell app. So the only difference is creating the signed
> > payload by sbvarsign in Ubuntu vs doing it in UEFI. I compared both
> > the working and non-working payloads and the main difference I see
> > is in the timestamp. For some reason sbvarsign writes the Year as
> > 0x0078 (120) vs the UEFI app writing 0x07e4 (2020). The
> > month/day/hour/min seems to be OK, but the year is really off in
> > the sbvarsign's payload. I cannot prove it, but I think the
> > commercial firmware may be having a sanity check for the
> > timestamp date/time, e.g. compare with the device manufacture date.
> > Since sbvarsign does not allow setting a timestamp separately, I
> > cannot force it to create a payload with the correct year.
> 
> That looks like a bug in sbvarsign. In UEFI-2.8, the EFI_TIME
> structure is defined under 8.3 "Time Services" / GetTime(). The Year
> field is given like this:
> 
>   UINT16 Year; // 1900 - 9999
> 
> The comment indicates the valid range. It does not indicate that the
> value stored should be relative to the year 1900 (which is what
> sbvarsign appears to assume; 2020-1900=120).
> 
> The UEFI application likely uses GetTime(), for populating
> "EFI_VARIABLE_AUTHENTICATION_2.TimeStamp" with the current time
> (which is prescribed in 8.2.2 "Using the
> EFI_VARIABLE_AUTHENTICATION_2 descriptor").
> 
> After GetTime() returns successfully, the application may still need
> to manually ensure that "Pad1, Nanosecond, TimeZone, Daylight and
> Pad2" are zeroed. This could involve timezone translation (to the
> required UTC), which in some cases could even imply a change to the
> Year field. But that doesn't change the fact that GetTime() initally
> places an absolute year number in the Year field, and not one
> relative to 1900.
> 
> Let me see the "sbsigntool" source code:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/sbsigntools.gi
> t
> 
> (Repository URL via
> <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=920013>.)
> 
> Yup, here's the bug (at current master, that is, commit 0dc3d4b5210d,
> "Fix PE/COFF checksum calculation", 2019-07-27):
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/sbsigntools.gi
> t/tree/src/sbvarsign.c?id=0dc3d4b5210dae158651d058f7ac68a9f178ae84#n2
> 15
> 
> Quote:
> 
>    199  static int set_timestamp(EFI_TIME *timestamp)
>    200  {
>    201          struct tm *tm;
>    202          time_t t;
>    203
>    204          time(&t);
>    205
>    206          tm = gmtime(&t);
>    207          if (!tm) {
>    208                  perror("gmtime");
>    209                  return -1;
>    210          }
>    211
>    212          /* copy to our EFI-specific time structure. Other
> fields (Nanosecond,
>    213           * TimeZone, Daylight and Pad) are defined to be zero
> */
>    214          memset(timestamp, 0, sizeof(*timestamp));
>    215          timestamp->Year = tm->tm_year;
>    216          timestamp->Month = tm->tm_mon;
>    217          timestamp->Day = tm->tm_mday;
>    218          timestamp->Hour = tm->tm_hour;
>    219          timestamp->Minute = tm->tm_min;
>    220          timestamp->Second = tm->tm_sec;
>    221
>    222          return 0;
>    223  }
> 
> Please refer to POSIX for time(), gmtime(), and "struct tm":
> 
>   https://pubs.opengroup.org/onlinepubs/9699919799/functions/time.htm
> l
>   https://pubs.opengroup.org/onlinepubs/9699919799/functions/gmtime.h
> tml
>   https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/time.h.ht
> ml
> 
> Quoting the last reference:
> 
> > The <time.h> header shall declare the tm structure, which shall
> > include at least the following members:
> > 
> > int    tm_sec   Seconds [0,60].
> > int    tm_min   Minutes [0,59].
> > int    tm_hour  Hour [0,23].
> > int    tm_mday  Day of month [1,31].
> > int    tm_mon   Month of year [0,11].
> > int    tm_year  Years since 1900.
> > int    tm_wday  Day of week [0,6] (Sunday =0).
> > int    tm_yday  Day of year [0,365].
> > int    tm_isdst Daylight Savings flag.
> 
> That is, field "tm_year" is relative to 1900.
> 
> The bug is therefore in set_timestamp(), on line 215. It should be
> 
>   timestamp->Year = 1900 + tm->tm_year;
> 
> The bug goes back to commit 953b00481f39 ("sbvarsign: First cut of a
> variable-signing tool", 2012-08-02).
> 
> (Otherwise, gmtime() is a good choice, because it gives us UTC at
> once, which is what EFI_VARIABLE_AUTHENTICATION_2 requires, per UEFI
> spec ("GMT").)
> 
> I don't know where sbsigntools development occurs (mailing list, bug
> tracker, ...). For now, I'm CC'ing James. (The git repo lives in his
> space on <git.kernel.org>.)

Heh, that's a sore point: since Jeremy Kerr left ubuntu, no-one except
me has been maintaining it; Jeremy owns the original tree on the Ubuntu
servers but can't update it any more.

I use groups.io for the openssl_tpm2_engine development list, so I
suppose I can set one up for sbsigntools ... OK, done.  If you want to
you can package the above up as a patch and send it there and I'll
apply it.

James

> James, the original thread on edk2-devel is here:
> 
> * [edk2-devel] Interpretation of specification
>   https://edk2.groups.io/g/devel/message/49402
> 
> Thanks,
> Laszlo
> 

Re: [edk2-devel] Interpretation of specification

[Laszlo Ersek]

Hello James,

On 01/08/20 20:13, James Bottomley wrote:
> On Wed, 2020-01-08 at 12:24 +0100, Laszlo Ersek wrote:

>> I don't know where sbsigntools development occurs (mailing list, bug
>> tracker, ...). For now, I'm CC'ing James. (The git repo lives in his
>> space on <git.kernel.org>.)
> 
> Heh, that's a sore point: since Jeremy Kerr left ubuntu, no-one except
> me has been maintaining it; Jeremy owns the original tree on the Ubuntu
> servers but can't update it any more.
> 
> I use groups.io for the openssl_tpm2_engine development list, so I
> suppose I can set one up for sbsigntools ... OK, done.  If you want to
> you can package the above up as a patch and send it there and I'll
> apply it.

I've now posted:

  [PATCH] sbvarsign: fix "EFI_VARIABLE_AUTHENTICATION_2.TimeStamp.Year" assignment
  Message-Id: <20200109171302.28782-1-lersek@redhat.com>

to <sbsigntools@groups.io>.

I didn't (and most likely, won't) subscribe to that list, so please whitelist this individual posting of mine, on the <https://sbsigntools.groups.io> admin interface.

Thanks!
Laszlo

Re: [edk2-devel] Interpretation of specification

[James Bottomley]

On Thu, 2020-01-09 at 18:17 +0100, Laszlo Ersek wrote:
> Hello James,
> 
> On 01/08/20 20:13, James Bottomley wrote:
> > On Wed, 2020-01-08 at 12:24 +0100, Laszlo Ersek wrote:
> > > I don't know where sbsigntools development occurs (mailing list,
> > > bug
> > > tracker, ...). For now, I'm CC'ing James. (The git repo lives in
> > > his
> > > space on <git.kernel.org>.)
> > 
> > Heh, that's a sore point: since Jeremy Kerr left ubuntu, no-one
> > except
> > me has been maintaining it; Jeremy owns the original tree on the
> > Ubuntu
> > servers but can't update it any more.
> > 
> > I use groups.io for the openssl_tpm2_engine development list, so I
> > suppose I can set one up for sbsigntools ... OK, done.  If you want
> > to
> > you can package the above up as a patch and send it there and I'll
> > apply it.
> 
> I've now posted:
> 
>   [PATCH] sbvarsign: fix
> "EFI_VARIABLE_AUTHENTICATION_2.TimeStamp.Year" assignment
>   Message-Id: <20200109171302.28782-1-lersek@redhat.com>
> 
> to <sbsigntools@groups.io>.
> 
> I didn't (and most likely, won't) subscribe to that list, so please
> whitelist this individual posting of mine, on the <https://sbsigntool
> s.groups.io> admin interface.

Heh, trying to dust off the groups.io setup memories as we speak.  I
can't allow fully unmoderated postings, unfortunately.  I also think
the group rejected this out of hand because it's not in the moderation
queue, but I think I've found the flag that holds non-member posts for
moderation and thus allows them to be posted instead of rejected.

James

Re: [edk2-devel] Interpretation of specification

[Laszlo Ersek]

On 01/09/20 18:20, James Bottomley wrote:
> On Thu, 2020-01-09 at 18:17 +0100, Laszlo Ersek wrote:
>> Hello James,
>>
>> On 01/08/20 20:13, James Bottomley wrote:
>>> On Wed, 2020-01-08 at 12:24 +0100, Laszlo Ersek wrote:
>>>> I don't know where sbsigntools development occurs (mailing list,
>>>> bug
>>>> tracker, ...). For now, I'm CC'ing James. (The git repo lives in
>>>> his
>>>> space on <git.kernel.org>.)
>>>
>>> Heh, that's a sore point: since Jeremy Kerr left ubuntu, no-one
>>> except
>>> me has been maintaining it; Jeremy owns the original tree on the
>>> Ubuntu
>>> servers but can't update it any more.
>>>
>>> I use groups.io for the openssl_tpm2_engine development list, so I
>>> suppose I can set one up for sbsigntools ... OK, done.  If you want
>>> to
>>> you can package the above up as a patch and send it there and I'll
>>> apply it.
>>
>> I've now posted:
>>
>>   [PATCH] sbvarsign: fix
>> "EFI_VARIABLE_AUTHENTICATION_2.TimeStamp.Year" assignment
>>   Message-Id: <20200109171302.28782-1-lersek@redhat.com>
>>
>> to <sbsigntools@groups.io>.
>>
>> I didn't (and most likely, won't) subscribe to that list, so please
>> whitelist this individual posting of mine, on the <https://sbsigntool
>> s.groups.io> admin interface.
> 
> Heh, trying to dust off the groups.io setup memories as we speak.  I
> can't allow fully unmoderated postings, unfortunately.

Right, that's my understanding too -- fully open lists are not supported
on groups.io (and at least in the edk2 community, most participants
don't like fully open posting -- I happen to be a fan of open posting,
FWIW). Worse, for non-subscribers, the best that groups.io offers is
white-listing per *message*, and not per *sender*.

> I also think
> the group rejected this out of hand because it's not in the moderation
> queue, but I think I've found the flag that holds non-member posts for
> moderation and thus allows them to be posted instead of rejected.

Ugh, I didn't know that just holding messages for moderation took a
separate setting!

Thanks!
Laszlo

Re: [edk2-devel] Interpretation of specification

[James Bottomley]

On Fri, 2020-01-10 at 11:55 +0100, Laszlo Ersek wrote:
> Right, that's my understanding too -- fully open lists are not
> supported on groups.io (and at least in the edk2 community, most
> participants don't like fully open posting -- I happen to be a fan of
> open posting, FWIW). Worse, for non-subscribers, the best that
> groups.io offers is white-listing per *message*, and not per
> *sender*.

Yes, that's the most annoying feature of groups.io

It can be worked around though: I've got an auto-reply on the moderator
email using procmail that auto-approves every message held for
moderation ... although I'm not sure that would work well on a higher
traffic list.

> > I also think the group rejected this out of hand because it's not
> > in the moderation queue, but I think I've found the flag that holds
> > non-member posts for moderation and thus allows them to be posted
> > instead of rejected.
> 
> Ugh, I didn't know that just holding messages for moderation took a
> separate setting!

Yes, apparently ... I remember having the same trouble with the
openssl-tpm2-engine group.

James