[PATCH v3 3/8] SecurityPkg: Create include file for default key content.


Grzegorz Bernacki
 

This commits add file which can be included by platform Flash
Description File. It allows to specify certificate files, which
will be embedded into binary file. The content of these files
can be used to initialize Secure Boot default keys and databases.

Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
---
SecurityPkg/SecureBootDefaultKeys.fdf.inc | 70 ++++++++++++++++++++
1 file changed, 70 insertions(+)
create mode 100644 SecurityPkg/SecureBootDefaultKeys.fdf.inc

diff --git a/SecurityPkg/SecureBootDefaultKeys.fdf.inc b/SecurityPkg/SecureBootDefaultKeys.fdf.inc
new file mode 100644
index 0000000000..bf4f2d42de
--- /dev/null
+++ b/SecurityPkg/SecureBootDefaultKeys.fdf.inc
@@ -0,0 +1,70 @@
+## @file
+# FDF include file which allows to embed Secure Boot keys
+#
+# Copyright (c) 2021, ARM Limited. All rights reserved.
+# Copyright (c) 2021, Semihalf. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+!if $(DEFAULT_KEYS) == TRUE
+ FILE FREEFORM = 85254ea7-4759-4fc4-82d4-5eed5fb0a4a0 {
+ !ifdef $(PK_DEFAULT_FILE)
+ SECTION RAW = $(PK_DEFAULT_FILE)
+ !endif
+ SECTION UI = "PK Default"
+ }
+
+ FILE FREEFORM = 6f64916e-9f7a-4c35-b952-cd041efb05a3 {
+ !ifdef $(KEK_DEFAULT_FILE1)
+ SECTION RAW = $(KEK_DEFAULT_FILE1)
+ !endif
+ !ifdef $(KEK_DEFAULT_FILE2)
+ SECTION RAW = $(KEK_DEFAULT_FILE2)
+ !endif
+ !ifdef $(KEK_DEFAULT_FILE3)
+ SECTION RAW = $(KEK_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "KEK Default"
+ }
+
+ FILE FREEFORM = c491d352-7623-4843-accc-2791a7574421 {
+ !ifdef $(DB_DEFAULT_FILE1)
+ SECTION RAW = $(DB_DEFAULT_FILE1)
+ !endif
+ !ifdef $(DB_DEFAULT_FILE2)
+ SECTION RAW = $(DB_DEFAULT_FILE2)
+ !endif
+ !ifdef $(DB_DEFAULT_FILE3)
+ SECTION RAW = $(DB_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "DB Default"
+ }
+
+ FILE FREEFORM = 36c513ee-a338-4976-a0fb-6ddba3dafe87 {
+ !ifdef $(DBT_DEFAULT_FILE1)
+ SECTION RAW = $(DBT_DEFAULT_FILE1)
+ !endif
+ !ifdef $(DBT_DEFAULT_FILE2)
+ SECTION RAW = $(DBT_DEFAULT_FILE2)
+ !endif
+ !ifdef $(DBT_DEFAULT_FILE3)
+ SECTION RAW = $(DBT_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "DBT Default"
+ }
+
+ FILE FREEFORM = 5740766a-718e-4dc0-9935-c36f7d3f884f {
+ !ifdef $(DBX_DEFAULT_FILE1)
+ SECTION RAW = $(DBX_DEFAULT_FILE1)
+ !endif
+ !ifdef $(DBX_DEFAULT_FILE2)
+ SECTION RAW = $(DBX_DEFAULT_FILE2)
+ !endif
+ !ifdef $(DBX_DEFAULT_FILE3)
+ SECTION RAW = $(DBX_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "DBX Default"
+ }
+
+!endif
--
2.25.1


Yao, Jiewen
 

Hi
I am not sure why we hardcode 3 items for each.

Can we move this fdf to platform pkg, instead of security pkg ?

Thank you
Yao Jiewen

-----Original Message-----
From: Grzegorz Bernacki <gjb@semihalf.com>
Sent: Monday, June 14, 2021 5:43 PM
To: devel@edk2.groups.io
Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer.El-Haj-
Mahmoud@arm.com; sunny.Wang@arm.com; mw@semihalf.com;
upstream@semihalf.com; Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
<jian.j.wang@intel.com>; Xu, Min M <min.m.xu@intel.com>;
lersek@redhat.com; sami.mujawar@arm.com; afish@apple.com; Ni, Ray
<ray.ni@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
rebecca@bsdio.com; grehan@freebsd.org; thomas.abraham@arm.com; Chiu,
Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>; gaoliming@byosoft.com.cn; Dong, Eric
<eric.dong@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Sun,
Zailiang <zailiang.sun@intel.com>; Qian, Yi <yi.qian@intel.com>;
graeme@nuviainc.com; rad@semihalf.com; pete@akeo.ie; Grzegorz Bernacki
<gjb@semihalf.com>
Subject: [PATCH v3 3/8] SecurityPkg: Create include file for default key content.

This commits add file which can be included by platform Flash
Description File. It allows to specify certificate files, which
will be embedded into binary file. The content of these files
can be used to initialize Secure Boot default keys and databases.

Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
---
SecurityPkg/SecureBootDefaultKeys.fdf.inc | 70 ++++++++++++++++++++
1 file changed, 70 insertions(+)
create mode 100644 SecurityPkg/SecureBootDefaultKeys.fdf.inc

diff --git a/SecurityPkg/SecureBootDefaultKeys.fdf.inc
b/SecurityPkg/SecureBootDefaultKeys.fdf.inc
new file mode 100644
index 0000000000..bf4f2d42de
--- /dev/null
+++ b/SecurityPkg/SecureBootDefaultKeys.fdf.inc
@@ -0,0 +1,70 @@
+## @file
+# FDF include file which allows to embed Secure Boot keys
+#
+# Copyright (c) 2021, ARM Limited. All rights reserved.
+# Copyright (c) 2021, Semihalf. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+!if $(DEFAULT_KEYS) == TRUE
+ FILE FREEFORM = 85254ea7-4759-4fc4-82d4-5eed5fb0a4a0 {
+ !ifdef $(PK_DEFAULT_FILE)
+ SECTION RAW = $(PK_DEFAULT_FILE)
+ !endif
+ SECTION UI = "PK Default"
+ }
+
+ FILE FREEFORM = 6f64916e-9f7a-4c35-b952-cd041efb05a3 {
+ !ifdef $(KEK_DEFAULT_FILE1)
+ SECTION RAW = $(KEK_DEFAULT_FILE1)
+ !endif
+ !ifdef $(KEK_DEFAULT_FILE2)
+ SECTION RAW = $(KEK_DEFAULT_FILE2)
+ !endif
+ !ifdef $(KEK_DEFAULT_FILE3)
+ SECTION RAW = $(KEK_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "KEK Default"
+ }
+
+ FILE FREEFORM = c491d352-7623-4843-accc-2791a7574421 {
+ !ifdef $(DB_DEFAULT_FILE1)
+ SECTION RAW = $(DB_DEFAULT_FILE1)
+ !endif
+ !ifdef $(DB_DEFAULT_FILE2)
+ SECTION RAW = $(DB_DEFAULT_FILE2)
+ !endif
+ !ifdef $(DB_DEFAULT_FILE3)
+ SECTION RAW = $(DB_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "DB Default"
+ }
+
+ FILE FREEFORM = 36c513ee-a338-4976-a0fb-6ddba3dafe87 {
+ !ifdef $(DBT_DEFAULT_FILE1)
+ SECTION RAW = $(DBT_DEFAULT_FILE1)
+ !endif
+ !ifdef $(DBT_DEFAULT_FILE2)
+ SECTION RAW = $(DBT_DEFAULT_FILE2)
+ !endif
+ !ifdef $(DBT_DEFAULT_FILE3)
+ SECTION RAW = $(DBT_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "DBT Default"
+ }
+
+ FILE FREEFORM = 5740766a-718e-4dc0-9935-c36f7d3f884f {
+ !ifdef $(DBX_DEFAULT_FILE1)
+ SECTION RAW = $(DBX_DEFAULT_FILE1)
+ !endif
+ !ifdef $(DBX_DEFAULT_FILE2)
+ SECTION RAW = $(DBX_DEFAULT_FILE2)
+ !endif
+ !ifdef $(DBX_DEFAULT_FILE3)
+ SECTION RAW = $(DBX_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "DBX Default"
+ }
+
+!endif
--
2.25.1


Grzegorz Bernacki
 

Hi,

Adding edk-devel group back in the loop...
I removed it by mistake.

greg

wt., 15 cze 2021 o 14:16 Grzegorz Bernacki <gjb@semihalf.com> napisał(a):


It was the original design, but it was changed when RFC was reviewed.
Please see:
https://edk2.groups.io/g/rfc/topic/edk2_devel_rfc_secure/82139806

I think that having an include file is better than duplicating the
snippet in many platform files. If someone wants to use 1 key, then
the include file can still be used. Of course, if someone wants to use
more, then they must add entries in platform FDF, but still I like the
idea of include file.

thanks,
greg

wt., 15 cze 2021 o 13:59 Yao, Jiewen <jiewen.yao@intel.com> napisał(a):

I think it is platform policy to decide how many keys. (it could be 1 or 3 or 10).

I recommend to move this to a platform fdf.

Thank you
Yao Jiewen


-----Original Message-----
From: Grzegorz Bernacki <gjb@semihalf.com>
Sent: Tuesday, June 15, 2021 7:07 PM
To: Yao, Jiewen <jiewen.yao@intel.com>
Subject: Re: [PATCH v3 3/8] SecurityPkg: Create include file for default key
content.

Hi,

Thanks for your comments.
The idea was to allow the user to specify more than one key. One can
use not only Microsoft or Canonical keys, but also generate new keys
and use them.
I can move the include file to another directory, but which place is
the best for it. I thought that since the rest of the functionality is
placed in SecurityPkg, I should also place that file there.
thanks,
greg


wt., 15 cze 2021 o 02:52 Yao, Jiewen <jiewen.yao@intel.com> napisał(a):

Hi
I am not sure why we hardcode 3 items for each.

Can we move this fdf to platform pkg, instead of security pkg ?

Thank you
Yao Jiewen

-----Original Message-----
From: Grzegorz Bernacki <gjb@semihalf.com>
Sent: Monday, June 14, 2021 5:43 PM
To: devel@edk2.groups.io
Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer.El-Haj-
Mahmoud@arm.com; sunny.Wang@arm.com; mw@semihalf.com;
upstream@semihalf.com; Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian
J
<jian.j.wang@intel.com>; Xu, Min M <min.m.xu@intel.com>;
lersek@redhat.com; sami.mujawar@arm.com; afish@apple.com; Ni, Ray
<ray.ni@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
rebecca@bsdio.com; grehan@freebsd.org; thomas.abraham@arm.com;
Chiu,
Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>; gaoliming@byosoft.com.cn; Dong, Eric
<eric.dong@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
Sun,
Zailiang <zailiang.sun@intel.com>; Qian, Yi <yi.qian@intel.com>;
graeme@nuviainc.com; rad@semihalf.com; pete@akeo.ie; Grzegorz
Bernacki
<gjb@semihalf.com>
Subject: [PATCH v3 3/8] SecurityPkg: Create include file for default key
content.

This commits add file which can be included by platform Flash
Description File. It allows to specify certificate files, which
will be embedded into binary file. The content of these files
can be used to initialize Secure Boot default keys and databases.

Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
---
SecurityPkg/SecureBootDefaultKeys.fdf.inc | 70 ++++++++++++++++++++
1 file changed, 70 insertions(+)
create mode 100644 SecurityPkg/SecureBootDefaultKeys.fdf.inc

diff --git a/SecurityPkg/SecureBootDefaultKeys.fdf.inc
b/SecurityPkg/SecureBootDefaultKeys.fdf.inc
new file mode 100644
index 0000000000..bf4f2d42de
--- /dev/null
+++ b/SecurityPkg/SecureBootDefaultKeys.fdf.inc
@@ -0,0 +1,70 @@
+## @file
+# FDF include file which allows to embed Secure Boot keys
+#
+# Copyright (c) 2021, ARM Limited. All rights reserved.
+# Copyright (c) 2021, Semihalf. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+!if $(DEFAULT_KEYS) == TRUE
+ FILE FREEFORM = 85254ea7-4759-4fc4-82d4-5eed5fb0a4a0 {
+ !ifdef $(PK_DEFAULT_FILE)
+ SECTION RAW = $(PK_DEFAULT_FILE)
+ !endif
+ SECTION UI = "PK Default"
+ }
+
+ FILE FREEFORM = 6f64916e-9f7a-4c35-b952-cd041efb05a3 {
+ !ifdef $(KEK_DEFAULT_FILE1)
+ SECTION RAW = $(KEK_DEFAULT_FILE1)
+ !endif
+ !ifdef $(KEK_DEFAULT_FILE2)
+ SECTION RAW = $(KEK_DEFAULT_FILE2)
+ !endif
+ !ifdef $(KEK_DEFAULT_FILE3)
+ SECTION RAW = $(KEK_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "KEK Default"
+ }
+
+ FILE FREEFORM = c491d352-7623-4843-accc-2791a7574421 {
+ !ifdef $(DB_DEFAULT_FILE1)
+ SECTION RAW = $(DB_DEFAULT_FILE1)
+ !endif
+ !ifdef $(DB_DEFAULT_FILE2)
+ SECTION RAW = $(DB_DEFAULT_FILE2)
+ !endif
+ !ifdef $(DB_DEFAULT_FILE3)
+ SECTION RAW = $(DB_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "DB Default"
+ }
+
+ FILE FREEFORM = 36c513ee-a338-4976-a0fb-6ddba3dafe87 {
+ !ifdef $(DBT_DEFAULT_FILE1)
+ SECTION RAW = $(DBT_DEFAULT_FILE1)
+ !endif
+ !ifdef $(DBT_DEFAULT_FILE2)
+ SECTION RAW = $(DBT_DEFAULT_FILE2)
+ !endif
+ !ifdef $(DBT_DEFAULT_FILE3)
+ SECTION RAW = $(DBT_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "DBT Default"
+ }
+
+ FILE FREEFORM = 5740766a-718e-4dc0-9935-c36f7d3f884f {
+ !ifdef $(DBX_DEFAULT_FILE1)
+ SECTION RAW = $(DBX_DEFAULT_FILE1)
+ !endif
+ !ifdef $(DBX_DEFAULT_FILE2)
+ SECTION RAW = $(DBX_DEFAULT_FILE2)
+ !endif
+ !ifdef $(DBX_DEFAULT_FILE3)
+ SECTION RAW = $(DBX_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "DBX Default"
+ }
+
+!endif
--
2.25.1


Yao, Jiewen
 

I don’t think it is a good idea to put it to security pkg, because it is a platform configuration.

If the goal is to create one include file, you can put it to other common platform pkg, such as https://github.com/tianocore/edk2-platforms/tree/master/Platform/Intel/MinPlatformPkg/Include/Fdf

Thank you
Yao Jiewen

-----Original Message-----
From: Grzegorz Bernacki <gjb@semihalf.com>
Sent: Tuesday, June 15, 2021 9:40 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
Subject: Re: [PATCH v3 3/8] SecurityPkg: Create include file for default key
content.

Hi,

Adding edk-devel group back in the loop...
I removed it by mistake.

greg

wt., 15 cze 2021 o 14:16 Grzegorz Bernacki <gjb@semihalf.com> napisał(a):

It was the original design, but it was changed when RFC was reviewed.
Please see:
https://edk2.groups.io/g/rfc/topic/edk2_devel_rfc_secure/82139806

I think that having an include file is better than duplicating the
snippet in many platform files. If someone wants to use 1 key, then
the include file can still be used. Of course, if someone wants to use
more, then they must add entries in platform FDF, but still I like the
idea of include file.

thanks,
greg

wt., 15 cze 2021 o 13:59 Yao, Jiewen <jiewen.yao@intel.com> napisał(a):

I think it is platform policy to decide how many keys. (it could be 1 or 3 or 10).

I recommend to move this to a platform fdf.

Thank you
Yao Jiewen


-----Original Message-----
From: Grzegorz Bernacki <gjb@semihalf.com>
Sent: Tuesday, June 15, 2021 7:07 PM
To: Yao, Jiewen <jiewen.yao@intel.com>
Subject: Re: [PATCH v3 3/8] SecurityPkg: Create include file for default key
content.

Hi,

Thanks for your comments.
The idea was to allow the user to specify more than one key. One can
use not only Microsoft or Canonical keys, but also generate new keys
and use them.
I can move the include file to another directory, but which place is
the best for it. I thought that since the rest of the functionality is
placed in SecurityPkg, I should also place that file there.
thanks,
greg


wt., 15 cze 2021 o 02:52 Yao, Jiewen <jiewen.yao@intel.com> napisał(a):

Hi
I am not sure why we hardcode 3 items for each.

Can we move this fdf to platform pkg, instead of security pkg ?

Thank you
Yao Jiewen

-----Original Message-----
From: Grzegorz Bernacki <gjb@semihalf.com>
Sent: Monday, June 14, 2021 5:43 PM
To: devel@edk2.groups.io
Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer.El-Haj-
Mahmoud@arm.com; sunny.Wang@arm.com; mw@semihalf.com;
upstream@semihalf.com; Yao, Jiewen <jiewen.yao@intel.com>; Wang,
Jian
J
<jian.j.wang@intel.com>; Xu, Min M <min.m.xu@intel.com>;
lersek@redhat.com; sami.mujawar@arm.com; afish@apple.com; Ni,
Ray
<ray.ni@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
rebecca@bsdio.com; grehan@freebsd.org;
thomas.abraham@arm.com;
Chiu,
Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>; gaoliming@byosoft.com.cn; Dong,
Eric
<eric.dong@intel.com>; Kinney, Michael D
<michael.d.kinney@intel.com>;
Sun,
Zailiang <zailiang.sun@intel.com>; Qian, Yi <yi.qian@intel.com>;
graeme@nuviainc.com; rad@semihalf.com; pete@akeo.ie; Grzegorz
Bernacki
<gjb@semihalf.com>
Subject: [PATCH v3 3/8] SecurityPkg: Create include file for default key
content.

This commits add file which can be included by platform Flash
Description File. It allows to specify certificate files, which
will be embedded into binary file. The content of these files
can be used to initialize Secure Boot default keys and databases.

Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
---
SecurityPkg/SecureBootDefaultKeys.fdf.inc | 70
++++++++++++++++++++
1 file changed, 70 insertions(+)
create mode 100644 SecurityPkg/SecureBootDefaultKeys.fdf.inc

diff --git a/SecurityPkg/SecureBootDefaultKeys.fdf.inc
b/SecurityPkg/SecureBootDefaultKeys.fdf.inc
new file mode 100644
index 0000000000..bf4f2d42de
--- /dev/null
+++ b/SecurityPkg/SecureBootDefaultKeys.fdf.inc
@@ -0,0 +1,70 @@
+## @file
+# FDF include file which allows to embed Secure Boot keys
+#
+# Copyright (c) 2021, ARM Limited. All rights reserved.
+# Copyright (c) 2021, Semihalf. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+!if $(DEFAULT_KEYS) == TRUE
+ FILE FREEFORM = 85254ea7-4759-4fc4-82d4-5eed5fb0a4a0 {
+ !ifdef $(PK_DEFAULT_FILE)
+ SECTION RAW = $(PK_DEFAULT_FILE)
+ !endif
+ SECTION UI = "PK Default"
+ }
+
+ FILE FREEFORM = 6f64916e-9f7a-4c35-b952-cd041efb05a3 {
+ !ifdef $(KEK_DEFAULT_FILE1)
+ SECTION RAW = $(KEK_DEFAULT_FILE1)
+ !endif
+ !ifdef $(KEK_DEFAULT_FILE2)
+ SECTION RAW = $(KEK_DEFAULT_FILE2)
+ !endif
+ !ifdef $(KEK_DEFAULT_FILE3)
+ SECTION RAW = $(KEK_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "KEK Default"
+ }
+
+ FILE FREEFORM = c491d352-7623-4843-accc-2791a7574421 {
+ !ifdef $(DB_DEFAULT_FILE1)
+ SECTION RAW = $(DB_DEFAULT_FILE1)
+ !endif
+ !ifdef $(DB_DEFAULT_FILE2)
+ SECTION RAW = $(DB_DEFAULT_FILE2)
+ !endif
+ !ifdef $(DB_DEFAULT_FILE3)
+ SECTION RAW = $(DB_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "DB Default"
+ }
+
+ FILE FREEFORM = 36c513ee-a338-4976-a0fb-6ddba3dafe87 {
+ !ifdef $(DBT_DEFAULT_FILE1)
+ SECTION RAW = $(DBT_DEFAULT_FILE1)
+ !endif
+ !ifdef $(DBT_DEFAULT_FILE2)
+ SECTION RAW = $(DBT_DEFAULT_FILE2)
+ !endif
+ !ifdef $(DBT_DEFAULT_FILE3)
+ SECTION RAW = $(DBT_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "DBT Default"
+ }
+
+ FILE FREEFORM = 5740766a-718e-4dc0-9935-c36f7d3f884f {
+ !ifdef $(DBX_DEFAULT_FILE1)
+ SECTION RAW = $(DBX_DEFAULT_FILE1)
+ !endif
+ !ifdef $(DBX_DEFAULT_FILE2)
+ SECTION RAW = $(DBX_DEFAULT_FILE2)
+ !endif
+ !ifdef $(DBX_DEFAULT_FILE3)
+ SECTION RAW = $(DBX_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "DBX Default"
+ }
+
+!endif
--
2.25.1


Samer El-Haj-Mahmoud
 

Maybe use the MinPlatformPkg for Intel (like what Jiewen recommended) and ArmPlatformPkg for Arm?

At least this reduced the per-platform duplication for every platform. But it may not cover all architectures/platform families (e.g. AMD, RISC-V?)

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao,
Jiewen via groups.io
Sent: Tuesday, June 15, 2021 10:23 AM
To: Grzegorz Bernacki <gjb@semihalf.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v3 3/8] SecurityPkg: Create include file for
default key content.

I don’t think it is a good idea to put it to security pkg, because it is a platform
configuration.

If the goal is to create one include file, you can put it to other common
platform pkg, such as https://github.com/tianocore/edk2-
platforms/tree/master/Platform/Intel/MinPlatformPkg/Include/Fdf

Thank you
Yao Jiewen


-----Original Message-----
From: Grzegorz Bernacki <gjb@semihalf.com>
Sent: Tuesday, June 15, 2021 9:40 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
Subject: Re: [PATCH v3 3/8] SecurityPkg: Create include file for default key
content.

Hi,

Adding edk-devel group back in the loop...
I removed it by mistake.

greg

wt., 15 cze 2021 o 14:16 Grzegorz Bernacki <gjb@semihalf.com> napisał(a):

It was the original design, but it was changed when RFC was reviewed.
Please see:
https://edk2.groups.io/g/rfc/topic/edk2_devel_rfc_secure/82139806

I think that having an include file is better than duplicating the
snippet in many platform files. If someone wants to use 1 key, then
the include file can still be used. Of course, if someone wants to use
more, then they must add entries in platform FDF, but still I like the
idea of include file.

thanks,
greg

wt., 15 cze 2021 o 13:59 Yao, Jiewen <jiewen.yao@intel.com> napisał(a):

I think it is platform policy to decide how many keys. (it could be 1 or 3
or 10).

I recommend to move this to a platform fdf.

Thank you
Yao Jiewen


-----Original Message-----
From: Grzegorz Bernacki <gjb@semihalf.com>
Sent: Tuesday, June 15, 2021 7:07 PM
To: Yao, Jiewen <jiewen.yao@intel.com>
Subject: Re: [PATCH v3 3/8] SecurityPkg: Create include file for default
key
content.

Hi,

Thanks for your comments.
The idea was to allow the user to specify more than one key. One can
use not only Microsoft or Canonical keys, but also generate new keys
and use them.
I can move the include file to another directory, but which place is
the best for it. I thought that since the rest of the functionality is
placed in SecurityPkg, I should also place that file there.
thanks,
greg


wt., 15 cze 2021 o 02:52 Yao, Jiewen <jiewen.yao@intel.com>
napisał(a):

Hi
I am not sure why we hardcode 3 items for each.

Can we move this fdf to platform pkg, instead of security pkg ?

Thank you
Yao Jiewen

-----Original Message-----
From: Grzegorz Bernacki <gjb@semihalf.com>
Sent: Monday, June 14, 2021 5:43 PM
To: devel@edk2.groups.io
Cc: leif@nuviainc.com; ardb+tianocore@kernel.org; Samer.El-Haj-
Mahmoud@arm.com; sunny.Wang@arm.com;
mw@semihalf.com;
upstream@semihalf.com; Yao, Jiewen <jiewen.yao@intel.com>;
Wang,
Jian
J
<jian.j.wang@intel.com>; Xu, Min M <min.m.xu@intel.com>;
lersek@redhat.com; sami.mujawar@arm.com; afish@apple.com;
Ni,
Ray
<ray.ni@intel.com>; Justen, Jordan L
<jordan.l.justen@intel.com>;
rebecca@bsdio.com; grehan@freebsd.org;
thomas.abraham@arm.com;
Chiu,
Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>; gaoliming@byosoft.com.cn;
Dong,
Eric
<eric.dong@intel.com>; Kinney, Michael D
<michael.d.kinney@intel.com>;
Sun,
Zailiang <zailiang.sun@intel.com>; Qian, Yi <yi.qian@intel.com>;
graeme@nuviainc.com; rad@semihalf.com; pete@akeo.ie;
Grzegorz
Bernacki
<gjb@semihalf.com>
Subject: [PATCH v3 3/8] SecurityPkg: Create include file for default
key
content.

This commits add file which can be included by platform Flash
Description File. It allows to specify certificate files, which
will be embedded into binary file. The content of these files
can be used to initialize Secure Boot default keys and databases.

Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
---
SecurityPkg/SecureBootDefaultKeys.fdf.inc | 70
++++++++++++++++++++
1 file changed, 70 insertions(+)
create mode 100644 SecurityPkg/SecureBootDefaultKeys.fdf.inc

diff --git a/SecurityPkg/SecureBootDefaultKeys.fdf.inc
b/SecurityPkg/SecureBootDefaultKeys.fdf.inc
new file mode 100644
index 0000000000..bf4f2d42de
--- /dev/null
+++ b/SecurityPkg/SecureBootDefaultKeys.fdf.inc
@@ -0,0 +1,70 @@
+## @file
+# FDF include file which allows to embed Secure Boot keys
+#
+# Copyright (c) 2021, ARM Limited. All rights reserved.
+# Copyright (c) 2021, Semihalf. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+!if $(DEFAULT_KEYS) == TRUE
+ FILE FREEFORM = 85254ea7-4759-4fc4-82d4-5eed5fb0a4a0 {
+ !ifdef $(PK_DEFAULT_FILE)
+ SECTION RAW = $(PK_DEFAULT_FILE)
+ !endif
+ SECTION UI = "PK Default"
+ }
+
+ FILE FREEFORM = 6f64916e-9f7a-4c35-b952-cd041efb05a3 {
+ !ifdef $(KEK_DEFAULT_FILE1)
+ SECTION RAW = $(KEK_DEFAULT_FILE1)
+ !endif
+ !ifdef $(KEK_DEFAULT_FILE2)
+ SECTION RAW = $(KEK_DEFAULT_FILE2)
+ !endif
+ !ifdef $(KEK_DEFAULT_FILE3)
+ SECTION RAW = $(KEK_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "KEK Default"
+ }
+
+ FILE FREEFORM = c491d352-7623-4843-accc-2791a7574421 {
+ !ifdef $(DB_DEFAULT_FILE1)
+ SECTION RAW = $(DB_DEFAULT_FILE1)
+ !endif
+ !ifdef $(DB_DEFAULT_FILE2)
+ SECTION RAW = $(DB_DEFAULT_FILE2)
+ !endif
+ !ifdef $(DB_DEFAULT_FILE3)
+ SECTION RAW = $(DB_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "DB Default"
+ }
+
+ FILE FREEFORM = 36c513ee-a338-4976-a0fb-6ddba3dafe87 {
+ !ifdef $(DBT_DEFAULT_FILE1)
+ SECTION RAW = $(DBT_DEFAULT_FILE1)
+ !endif
+ !ifdef $(DBT_DEFAULT_FILE2)
+ SECTION RAW = $(DBT_DEFAULT_FILE2)
+ !endif
+ !ifdef $(DBT_DEFAULT_FILE3)
+ SECTION RAW = $(DBT_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "DBT Default"
+ }
+
+ FILE FREEFORM = 5740766a-718e-4dc0-9935-c36f7d3f884f {
+ !ifdef $(DBX_DEFAULT_FILE1)
+ SECTION RAW = $(DBX_DEFAULT_FILE1)
+ !endif
+ !ifdef $(DBX_DEFAULT_FILE2)
+ SECTION RAW = $(DBX_DEFAULT_FILE2)
+ !endif
+ !ifdef $(DBX_DEFAULT_FILE3)
+ SECTION RAW = $(DBX_DEFAULT_FILE3)
+ !endif
+ SECTION UI = "DBX Default"
+ }
+
+!endif
--
2.25.1


IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.