Re: [PATCH v14 01/13] hp-bioscfg: Documentation

From: "Mark Pearson" <mpearson-lenovo@squebb.ca>
To: "Jorge Lopez" <jorgealtxwork@gmail.com>,
	"Hans de Goede" <hdegoede@redhat.com>,
	"platform-driver-x86@vger.kernel.org"
	<platform-driver-x86@vger.kernel.org>,
	linux-kernel@vger.kernel.org,
	"Thomas Weißschuh" <thomas@t-8ch.de>,
	ilpo.jarvinen@linux.intel.com
Subject: Re: [PATCH v14 01/13] hp-bioscfg: Documentation
Date: Fri, 19 May 2023 13:34:32 -0400	[thread overview]
Message-ID: <0abdb306-1e24-4653-9a14-e5db8d508a82@app.fastmail.com> (raw)
In-Reply-To: <20230517155026.28535-2-jorge.lopez2@hp.com>

Hi Jorge,

On Wed, May 17, 2023, at 11:50 AM, Jorge Lopez wrote:
> HP BIOS Configuration driver purpose is to provide a driver supporting
> the latest sysfs class firmware attributes framework allowing the user
> to change BIOS settings and security solutions on HP Inc.’s commercial
> notebooks.
>
> Many features of HP Commercial notebooks can be managed using Windows
> Management Instrumentation (WMI). WMI is an implementation of Web-Based
> Enterprise Management (WBEM) that provides a standards-based interface
> for changing and monitoring system settings. HP BIOSCFG driver provides
> a native Linux solution and the exposed features facilitates the
> migration to Linux environments.
>
> The Linux security features to be provided in hp-bioscfg driver enables
> managing the BIOS settings and security solutions via sysfs, a virtual
> filesystem that can be used by user-mode applications. The new
> documentation cover HP-specific firmware sysfs attributes such Secure
> Platform Management and Sure Start. Each section provides security
> feature description and identifies sysfs directories and files exposed
> by the driver.
>
> Many HP Commercial notebooks include a feature called Secure Platform
> Management (SPM), which replaces older password-based BIOS settings
> management with public key cryptography. PC secure product management
> begins when a target system is provisioned with cryptographic keys
> that are used to ensure the integrity of communications between system
> management utilities and the BIOS.
>
> HP Commercial notebooks have several BIOS settings that control its
> behaviour and capabilities, many of which are related to security.
> To prevent unauthorized changes to these settings, the system can
> be configured to use a cryptographic signature-based authorization
> string that the BIOS will use to verify authorization to modify the
> setting.
>
> Linux Security components are under development and not published yet.
> The only linux component is the driver (hp bioscfg) at this time.
> Other published security components are under Windows.
>
> Signed-off-by: Jorge Lopez <jorge.lopez2@hp.com>
>
> ---
> Based on the latest platform-drivers-x86.git/for-next
> ---
>  .../testing/sysfs-class-firmware-attributes   | 102 +++++++++++++++++-
>  1 file changed, 100 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-class-firmware-attributes 
> b/Documentation/ABI/testing/sysfs-class-firmware-attributes
> index 4cdba3477176..f8d6c089228b 100644
> --- a/Documentation/ABI/testing/sysfs-class-firmware-attributes
> +++ b/Documentation/ABI/testing/sysfs-class-firmware-attributes
<snip>
> +
> +
> +		HP specific class extensions - Secure Platform Manager (SPM)
> +		--------------------------------
> +
> +What:		/sys/class/firmware-attributes/*/authentication/SPM/kek
> +Date:		March 29
> +KernelVersion:	5.18
> +Contact:	"Jorge Lopez" <jorge.lopez2@hp.com>
> +Description:
> +		'kek' Key-Encryption-Key is a write-only file that can be used to 
> configure the
> +		RSA public key that will be used by the BIOS to verify
> +		signatures when setting the signing key.  When written,
> +		the bytes should correspond to the KEK certificate
> +		(x509 .DER format containing an OU).  The size of the
> +		certificate must be less than or equal to 4095 bytes.
> +
> +What:		/sys/class/firmware-attributes/*/authentication/SPM/sk
> +Date:		March 29
> +KernelVersion:	5.18
> +Contact:	"Jorge Lopez" <jorge.lopez2@hp.com>
> +Description:
> +		'sk' Signature Key is a write-only file that can be used to 
> configure the RSA
> +		public key that will be used by the BIOS to verify signatures
> +		when configuring BIOS settings and security features.  When
> +		written, the bytes should correspond to the modulus of the
> +		public key.  The exponent is assumed to be 0x10001.
> +
> +What:		/sys/class/firmware-attributes/*/authentication/SPM/status
> +Date:		March 29
> +KernelVersion:	5.18
> +Contact:	"Jorge Lopez" <jorge.lopez2@hp.com>
> +Description:
> +		'status' is a read-only file that returns ASCII text in JSON format 
> reporting
> +		the status information.
> +
> +		  "State": "not provisioned | provisioned | provisioning in progress 
> ",
> +		  "Version": " Major. Minor ",
> +		  "Nonce": <16-bit unsigned number display in base 10>,
> +		  "FeaturesInUse": <16-bit unsigned number display in base 10>,
> +		  "EndorsementKeyMod": "<256 bytes in base64>",
> +		  "SigningKeyMod": "<256 bytes in base64>"
> +
> +What:		/sys/class/firmware-attributes/*/attributes/Sure_Start/audit_log_entries
> +Date:		March 29
> +KernelVersion:	5.18
> +Contact:	"Jorge Lopez" <jorge.lopez2@hp.com>
> +Description:
> +		'audit_log_entries' is a read-only file that returns the events in 
> the log.
> +
> +			Audit log entry format
> +
> +			Byte 0-15:   Requested Audit Log entry  (Each Audit log is 16 bytes)
> +			Byte 16-127: Unused
> +
> +What:		/sys/class/firmware-attributes/*/attributes/Sure_Start/audit_log_entry_count
> +Date:		March 29
> +KernelVersion:	5.18
> +Contact:	"Jorge Lopez" <jorge.lopez2@hp.com>
> +Description:
> +		'audit_log_entry_count' is a read-only file that returns the number 
> of existing
> +		audit log events available to be read. Values are separated using 
> comma (``,``)
> +
> +			[No of entries],[log entry size],[Max number of entries supported]
> +
> +		log entry size identifies audit log size for the current BIOS 
> version.
> +		The current size is 16 bytes but it can be up to 128 bytes long in 
> future BIOS
> +		versions.
> -- 
> 2.34.1

Firstly apologies as I've done a poor job of following the updates to this series - so if this has already been discussed and agreed by more seasoned kernel contributors please feel free to disregard my comments :) I was catching up on my inbox and had some thoughts.

For SPM - as this replaces password usage, is it done for all account types? It seems a bit odd having it be a replacement for the passwords but in it's own location and not in the same place as (for example) Admin/current_password.
For the Lenovo implementation I put certificate, signature and save_signature in the authentication/Admin directory and I realise your implementation is different with the keys but if the kek and sk are only used with the Admin account then shouldn't they also be in that directory? It would be nice to have some commonality across vendors in my opinion.

For the Sure_Start I would propose de-branding this so it's generic and I don't think it fits under attributes as it doesn't support any of the other required attribute fields. I think your implementation of an audit log seems neat but if another vendor was to do similar it would be better to be able to reuse the same attribute name and enable common tooling. 
I propose having this as just log/audit_entries and log/audit_count and have the log folder in the top alongside authentication and attributes.
If someone wants to add other logs in the future it would be a good place to have them.

Thanks
Mark