Re: [PATCH v7 04/17] efi_loader: variable: support variable authentication

On 4/20/20 9:22 PM, Sughosh Ganu wrote:

On Tue, 14 Apr 2020 at 08:23, AKASHI Takahiro <takahiro.akashi@linaro.org mailto:takahiro.akashi@linaro.org> wrote:

With this commit, EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS
is supported for authenticated variables and the system secure state
will transfer between setup mode and user mode as UEFI specification
section 32.3 describes.

Internally, authentication data is stored as part of authenticated
variable's value. It is nothing but a pkcs7 message (but we need some
wrapper, see efi_variable_parse_signature()) and will be validated by
efi_variable_authenticate(), hence efi_signature_verify_with_db().

Associated time value will be encoded in "{...,time=...}" along with
other UEFI variable's attributes.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org
<mailto:takahiro.akashi@linaro.org>>
---
 include/efi_loader.h          |   3 +
 lib/efi_loader/efi_variable.c | 666 ++++++++++++++++++++++++++++------
 2 files changed, 565 insertions(+), 104 deletions(-) 

<snip>

diff --git a/lib/efi_loader/efi_variable.c
b/lib/efi_loader/efi_variable.c
index fe2f26459136..adb78470f2d6 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -10,8 +10,14 @@
 #include <env_internal.h>
 #include <hexdump.h>
 #include <malloc.h>
+#include <rtc.h>
 #include <search.h>
+#include <linux/compat.h>
 #include <u-boot/crc.h>
+#include "../lib/crypto/pkcs7_parser.h"
+
+const efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
+static bool efi_secure_boot;

 #define READ_ONLY BIT(31)

@@ -106,9 +112,10 @@ static const char *prefix(const char *str,
const char *prefix)
  *
  * @str:       value of U-Boot variable
  * @attrp:     pointer to UEFI attributes
+ * @timep:     pointer to time attribute
  * Return:     pointer to remainder of U-Boot variable value
  */
-static const char *parse_attr(const char *str, u32 *attrp)
+static const char *parse_attr(const char *str, u32 *attrp, u64 *timep)
 {
        u32 attr = 0;
        char sep = '{';
@@ -131,6 +138,12 @@ static const char *parse_attr(const char *str,
u32 *attrp)
                        attr |= EFI_VARIABLE_BOOTSERVICE_ACCESS;
                } else if ((s = prefix(str, "run"))) {
                        attr |= EFI_VARIABLE_RUNTIME_ACCESS;
+               } else if ((s = prefix(str, "time="))) {
+                       attr |=
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
+                       hex2bin((u8 *)timep, s, sizeof(*timep));
+                       s += sizeof(*timep) * 2;
+               } else if (*str == '}') {
+                       break;
                } else {
                        printf("invalid attribute: %s\n", str);
                        break;
@@ -148,48 +161,291 @@ static const char *parse_attr(const char
*str, u32 *attrp)
 }

 /**
- * efi_get_variable() - retrieve value of a UEFI variable
+ * efi_secure_boot_enabled - return if secure boot is enabled or not
  *
- * This function implements the GetVariable runtime service.
+ * Return:     true if enabled, false if disabled
+ */
+bool efi_secure_boot_enabled(void)
+{
+       return efi_secure_boot;
+}
+
+#ifdef CONFIG_EFI_SECURE_BOOT
+static u8 pkcs7_hdr[] = {
+       /* SEQUENCE */
+       0x30, 0x82, 0x05, 0xc7,
+       /* OID: pkcs7-signedData */
+       0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07,
0x02,
+       /* Context Structured? */
+       0xa0, 0x82, 0x05, 0xb8,
+};
+
+/**
+ * efi_variable_parse_signature - parse a signature in variable
+ * @buf:       Pointer to variable's value
+ * @buflen:    Length of @buf
  *
- * See the Unified Extensible Firmware Interface (UEFI)
specification for
- * details.
+ * Parse a signature embedded in variable's value and instantiate
+ * a pkcs7_message structure. Since pkcs7_parse_message() accepts only
+ * pkcs7's signedData, some header needed be prepended for correctly
+ * parsing authentication data, particularly for variable's.
  *
- * @variable_name:     name of the variable
- * @vendor:            vendor GUID
- * @attributes:                attributes of the variable
- * @data_size:         size of the buffer to which the variable
value is copied
- * @data:              buffer to which the variable value is copied
- * Return:             status code
+ * Return:     Pointer to pkcs7_message structure on success, NULL
on error
  */
-efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
-                                    const efi_guid_t *vendor, u32
*attributes,
-                                    efi_uintn_t *data_size, void *data)
+static struct pkcs7_message *efi_variable_parse_signature(const
void *buf,
+                                                         size_t buflen)

This is a generic function used for parsing the pkcs7 header. This will also be used for capsule authentication. Can you move this under efi_signature.c as an api, and change the name to something like efi_parse_pkcs7_header.

-sughosh

Hello Sughosh,

the patch is already merged. Could you, please, provide a follow-up patch with the suggested change.

Best regards

Heinrich