| Message ID | 20210505035622.3871520-3-dja@axtens.net |
|---|---|
| State | Accepted |
| Headers | show |
| Series | secvar/backend changes for secvarctl | expand |
After going down a bit of a rabbit hole looking for the significance of `EFI_CERT_RSA2048_GUID` in secvarctl (a package which depends on skiboot) , I have come to the conclusion that the uuid is not directly imperative to any secvarctl or skiboot processes. This GUID is only contained in an ESL if it contains an RSA key, which is invalid since, for secvar purposes, we only accept ESLS which contain X509's (or hashes if the ESL is for the dbx secvar). That being said, it is useful for things like error messages (or logs in skiboots case) because we tell the user "your ESL is invalid because it contains an RSA key, go put this thing in an x509" or something like that. Its origins are from tianocore and I likely added it to secvarctl since it appeared relevant when I was just starting my project. Sorry for rambling but what I am getting at is: As of now, `EFI_CERT_RSA2048_GUID` is useful solely for telling the user what data we do not accept. This GUID may have been useful in the past and it may become useful in the future but for now it is not directly useful to any secvar update process. That being said, I see no harm in adding it to skiboot. Of course the decision is the maintainers to make, I just hope this gives some useful context. -Nick Child On Tue, May 4, 2021 at 11:56 PM Daniel Axtens <dja@axtens.net> wrote: > This isn't currently used in skiboot but may be used by external > users of skiboot's secvar code. > > Signed-off-by: Daniel Axtens <dja@axtens.net> > --- > libstb/secvar/backend/edk2.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libstb/secvar/backend/edk2.h b/libstb/secvar/backend/edk2.h > index 85e117861dff..ef6d7c79e7ff 100644 > --- a/libstb/secvar/backend/edk2.h > +++ b/libstb/secvar/backend/edk2.h > @@ -83,6 +83,8 @@ static const uuid_t EFI_CERT_SHA384_GUID = {{ 0x07, > 0x53, 0x3e, 0xff, 0xd0, 0x9f > > static const uuid_t EFI_CERT_SHA512_GUID = {{ 0xae, 0x0f, 0x3e, 0x09, > 0xc4, 0xa6, 0x50, 0x4f, 0x9f, 0x1b, 0xd4, 0x1e, 0x2b, 0x89, 0xc1, 0x9a }}; > > +static const uuid_t EFI_CERT_RSA2048_GUID = {{ 0xe8, 0x66, 0x57, 0x3c, > 0x9c, 0x26, 0x34, 0x4e, 0xaa, 0x14, 0xed, 0x77, 0x6e, 0x85, 0xb3, 0xb6 }}; > + > #define EFI_VARIABLE_NON_VOLATILE 0x00000001 > #define EFI_VARIABLE_BOOTSERVICE_ACCESS > 0x00000002 > #define EFI_VARIABLE_RUNTIME_ACCESS 0x00000004 > -- > 2.27.0 > > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot >
Hi Nick, > After going down a bit of a rabbit hole looking for the significance of > `EFI_CERT_RSA2048_GUID` in secvarctl (a package which depends on skiboot) , > I have come to the conclusion that the uuid is not directly imperative to > any secvarctl or skiboot processes. This GUID is only contained in an ESL > if it contains an RSA key, which is invalid since, for secvar purposes, we > only accept ESLS which contain X509's (or hashes if the ESL is for the dbx > secvar). That being said, it is useful for things like error messages (or > logs in skiboots case) because we tell the user "your ESL is invalid > because it contains an RSA key, go put this thing in an x509" or something > like that. Its origins are from tianocore and I likely added it to > secvarctl since it appeared relevant when I was just starting my project. > Sorry for rambling but what I am getting at is: As of now, > `EFI_CERT_RSA2048_GUID` is useful solely for telling the user what data we > do not accept. This GUID may have been useful in the past and it may become > useful in the future but for now it is not directly useful to any secvar > update process. That being said, I see no harm in adding it to skiboot. Of > course the decision is the maintainers to make, I just hope this gives some > useful context. > > -Nick Child Thanks for the background! I would say that given that the definition won't end up in skiboot binaries unless it's referenced in skiboot code, and that it is used to make error messages more helpful in secvarctl, it's worth having. But I'm OK either way. Kind regards, Daniel > On Tue, May 4, 2021 at 11:56 PM Daniel Axtens <dja@axtens.net> wrote: > >> This isn't currently used in skiboot but may be used by external >> users of skiboot's secvar code. >> >> Signed-off-by: Daniel Axtens <dja@axtens.net> >> --- >> libstb/secvar/backend/edk2.h | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/libstb/secvar/backend/edk2.h b/libstb/secvar/backend/edk2.h >> index 85e117861dff..ef6d7c79e7ff 100644 >> --- a/libstb/secvar/backend/edk2.h >> +++ b/libstb/secvar/backend/edk2.h >> @@ -83,6 +83,8 @@ static const uuid_t EFI_CERT_SHA384_GUID = {{ 0x07, >> 0x53, 0x3e, 0xff, 0xd0, 0x9f >> >> static const uuid_t EFI_CERT_SHA512_GUID = {{ 0xae, 0x0f, 0x3e, 0x09, >> 0xc4, 0xa6, 0x50, 0x4f, 0x9f, 0x1b, 0xd4, 0x1e, 0x2b, 0x89, 0xc1, 0x9a }}; >> >> +static const uuid_t EFI_CERT_RSA2048_GUID = {{ 0xe8, 0x66, 0x57, 0x3c, >> 0x9c, 0x26, 0x34, 0x4e, 0xaa, 0x14, 0xed, 0x77, 0x6e, 0x85, 0xb3, 0xb6 }}; >> + >> #define EFI_VARIABLE_NON_VOLATILE 0x00000001 >> #define EFI_VARIABLE_BOOTSERVICE_ACCESS >> 0x00000002 >> #define EFI_VARIABLE_RUNTIME_ACCESS 0x00000004 >> -- >> 2.27.0 >> >> _______________________________________________ >> Skiboot mailing list >> Skiboot@lists.ozlabs.org >> https://lists.ozlabs.org/listinfo/skiboot >>
On 5/10/21 12:25 PM, Daniel Axtens wrote: > Hi Nick, > >> After going down a bit of a rabbit hole looking for the significance of >> `EFI_CERT_RSA2048_GUID` in secvarctl (a package which depends on skiboot) , >> I have come to the conclusion that the uuid is not directly imperative to >> any secvarctl or skiboot processes. This GUID is only contained in an ESL >> if it contains an RSA key, which is invalid since, for secvar purposes, we >> only accept ESLS which contain X509's (or hashes if the ESL is for the dbx >> secvar). That being said, it is useful for things like error messages (or >> logs in skiboots case) because we tell the user "your ESL is invalid >> because it contains an RSA key, go put this thing in an x509" or something >> like that. Its origins are from tianocore and I likely added it to >> secvarctl since it appeared relevant when I was just starting my project. >> Sorry for rambling but what I am getting at is: As of now, >> `EFI_CERT_RSA2048_GUID` is useful solely for telling the user what data we >> do not accept. This GUID may have been useful in the past and it may become >> useful in the future but for now it is not directly useful to any secvar >> update process. That being said, I see no harm in adding it to skiboot. Of >> course the decision is the maintainers to make, I just hope this gives some >> useful context. >> >> -Nick Child > > > Thanks for the background! > > I would say that given that the definition won't end up in skiboot > binaries unless it's referenced in skiboot code, and that it is used to > make error messages more helpful in secvarctl, it's worth having. But > I'm OK either way. Thanks Daniel and Nick for patch and detailed background explanation. I decided to merge this series. Merged to master as of c44b8891ca. -Vasant
diff --git a/libstb/secvar/backend/edk2.h b/libstb/secvar/backend/edk2.h index 85e117861dff..ef6d7c79e7ff 100644 --- a/libstb/secvar/backend/edk2.h +++ b/libstb/secvar/backend/edk2.h @@ -83,6 +83,8 @@ static const uuid_t EFI_CERT_SHA384_GUID = {{ 0x07, 0x53, 0x3e, 0xff, 0xd0, 0x9f static const uuid_t EFI_CERT_SHA512_GUID = {{ 0xae, 0x0f, 0x3e, 0x09, 0xc4, 0xa6, 0x50, 0x4f, 0x9f, 0x1b, 0xd4, 0x1e, 0x2b, 0x89, 0xc1, 0x9a }}; +static const uuid_t EFI_CERT_RSA2048_GUID = {{ 0xe8, 0x66, 0x57, 0x3c, 0x9c, 0x26, 0x34, 0x4e, 0xaa, 0x14, 0xed, 0x77, 0x6e, 0x85, 0xb3, 0xb6 }}; + #define EFI_VARIABLE_NON_VOLATILE 0x00000001 #define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x00000002 #define EFI_VARIABLE_RUNTIME_ACCESS 0x00000004
This isn't currently used in skiboot but may be used by external users of skiboot's secvar code. Signed-off-by: Daniel Axtens <dja@axtens.net> --- libstb/secvar/backend/edk2.h | 2 ++ 1 file changed, 2 insertions(+)