Hi Hans,
>
> > +
> > + "sure-admin"-type specific properties:
> > +
> > + settings:
> > + A file associated with Sure Admin. BIOS settings can
> > + be read or written through the Sure Admin settings file in sysfs.
> > +
> > + [BIOS setting],[new value],[auth token]
> > +
> > + Sample settings reported data
> > +
> > + {
> > + "Class": "HPBIOS_BIOSEnumeration",
> > + "Name": "USB Storage Boot",
> > + "Path": "\\Advanced\\Boot Options",
> > + "IsReadOnly": 0,
> > + ...
> > + "Value": "Enable",
> > + "Size": 2,
> > + "PossibleValues": [
> > + "Disable",
> > + "Enable"
> > + ]
> > + }
> > +
> > + This file can also be written to in order to update
> > + the value sof a <attr> using a CMSL generated payload.
> > + For example:
> > +
> > + <attr>,[value],[CMSL generated payload]
>
>
> This is basically a run-around around the actual firmware attributes API,
> so NACK for this.
>
> Looking at how sure_admin_settings_write() encodes things I don't see any
> difference between how auth-tokens/CFML-payloads are handled vs regular passwords
> inside hp_set_attribute().
Yes. The handling of auth-tokens/CMSL-payloads vs regular passwords
are similar with the exception of their size.
auth-tokens/CMSL-payloads are in the size larger than 64 bytes while
passwords are limited to 64 bytes.
>
> Except for the BEAM_PREFIX thing, which can be added to calculate_security_buffer() /
> populate_security_buffer() too and in that case all 3 of the following should simply
> work, taking a boot-delay integer attribute as example:
>
> echo "password" > /sys/class/firmware-attributes/hp-bioscfg/"Setup Password"/current_password
> echo 5 > /sys/class/firmware-attributes/hp-bioscfg/attributes/boot-delay/current_value
>
> echo "auth-token" > /sys/class/firmware-attributes/hp-bioscfg/"Setup Password"/current_password
> echo 5 > /sys/class/firmware-attributes/hp-bioscfg/attributes/boot-delay/current_value
>
> cat csml-payload.file > /sys/class/firmware-attributes/hp-bioscfg/"Setup Password"/current_password
> echo 5 > /sys/class/firmware-attributes/hp-bioscfg/attributes/boot-delay/current_value
Last two cases cannot be handled by passing auth-token/CMSL payloads
values to the current password due to their possible sizes.
Passwords are limited to 64 bytes in size while
auth-tokens/CFML-payloads are in the size larger than 64 bytes
Here are examples of CMSL flow for your records. Most of generated
payloads are stored remotely and signed by customer to ensure the data
integrity. The full benefit of this process is better represented
when configuring Security settings in BIOS. (Sure Run, Sure Start,
Sure Recovery, etc)
.
1. Customer generates a signed CMSL payload to set BIOS "USB Storage
Boot" value to "Enable"
CMSL command:
New-HPSureAdminBIOSSettingValuePayload -Name "USB Storage Boot" -Value
"Enable" -SigningKeyFile sk.pfx -SigningKeyPassword test
Generated payload:
{"timestamp":"2022-09-26T16:01:22.7887948-05:00","purpose":"hp:sureadmin:biossetting","Data":[123,10,32,32,34,78,97,109,101,34,58,32,34,85,83,66,32,83,116,111,114,97,103,101,32,66,111,111,116,34,44,10,32,32,34,86,97,108,117,101,34,58,32,34,69,110,97,98,108,101,34,44,10,32,32,34,65,117,116,104,83,116,114,105,110,103,34,58,32,34,60,66,69,65,77,47,62,65,83,65,65,68,65,65,65,73,120,77,121,89,119,72,47,47,47,47,47,47,47,47,47,47,47,47,47,47,47,47,47,47,47,47,47,68,85,78,87,111,74,67,117,122,51,73,69,71,57,75,51,80,113,107,103,121,76,52,108,100,119,83,67,79,69,65,83,47,98,108,68,122,111,116,47,105,77,43,86,103,122,90,77,69,116,43,48,75,87,75,78,103,119,89,99,86,102,121,97,70,56,99,104,102,57,85,47,86,82,106,51,52,122,112,53,111,43,85,112,97,119,97,116,106,50,74,103,115,97,105,104,113,54,90,53,84,48,101,49,114,56,78,80,109,69,105,119,103,113,118,77,51,108,97,86,83,98,116,101,66,112,80,81,88,119,102,66,56,101,105,112,119,120,104,121,67,68,65,85,111,118,69,113,82,51,84,71,79,54,109,101,75,119,71,100,73,104,72,103,102,50,112,72,75,90,50,107,47,66,116,104,97,98,110,115,118,78,77,114,98,74,69,103,76,72,76,121,87,84,119,98,76,74,87,108,101,106,112,71,121,47,70,100,83,105,74,114,101,66,114,81,115,114,106,90,113,100,76,75,72,90,71,117,108,52,70,108,43,117,47,72,102,107,48,119,121,81,69,106,121,97,99,51,52,57,105,90,55,97,104,49,72,56,50,85,97,50,49,55,109,74,51,88,56,87,113,80,111,50,86,72,65,85,57,71,102,47,113,107,97,85,101,75,76,84,97,75,87,101,121,122,116,68,109,105,68,98,115,114,47,86,51,51,106,114,101,67,88,65,65,109,120,68,76,73,73,66,103,83,88,79,82,82,117,98,80,53,113,80,78,69,53,67,76,120,104,119,61,61,34,10,125],"Meta1":null,"Meta2":null,"Meta3":null,"Meta4":null}
The application reads the CMSL payload and sends the command to the
driver. The payload is translated to [BIOS setting], [Value],
[Password || auth-token]
echo "USB Storage
Boot,Enable,<BEAM/>ASAADAAANZ9AYwH/////////////////////64bPg7ygUv4xanoHIFrzME9mIsxeJh32fkhR7sgHpXdEHjetMXxNVhEK/twhhXhHS93kp9JpGhsr+J6AMKV2ldE99iJHo6ul1IxJQuBSxBoN1mf49Mm/ROCNll+IhsAn4ow+xlDwKQn2EzKtQc2Wf1eC646KPcl+ZCtiFhvLzXZrGSXsB2hJy0+IzegUPzLY6jaN0lYyQMtQ0KpcyGnK6xZSKCKfotygWawWY8BD3oewyrVLdKMGjrtX4HtHaeo5A9VVXVt89i7lZAmV3VkRtu70LEv240ue/SOhwrxGtydgNmtpV3dSn/ancnY4ONbTxBRiw8cifObEiNOidYzhpQ=="
> /sys/class/firmware-attributes/hp-bioscfg/attributes/Sure_Admin/settings'
As you can see, the auth-token "<BEAN/>.....==" portion varies in
size depending on the signing certificate size used.
A method that can be used to address your comments and allow the
driver to handle both auth-tokens and password using command
echo "auth-token" >
/sys/class/firmware-attributes/hp-bioscfg/"Setup
Password"/current_password
is adding an modifier to indicate if the input is a password or auth-token
For instance... using [Token] modifier to identify the data is an
authentication token
echo "[token] auth-token" >
/sys/class/firmware-attributes/hp-bioscfg/"Setup
Password"/current_password
If the modifier is missing, the driver will treat the data as a password.
This new method will conform with the firmware class framework and
provide the flexibility needed for HP solution.
Please advise.
>
> The only reason why this would not work is if things may get bigger then the size of a single
> memory page (bigger then 4096 bytes).
>
> But in that case you should just change the code implementing current_password to
> use the bin_attribute interface and replace the static buffer for storing it
> with a pointer to kmalloc-ed memory, which is NULL when the password is not set.
>
> This will also remove a lot of finicky parsing which is currently done inside
> sure_admin_settings_write()
>
> ###
>
> As for the reading which returns a large large list of data blocks like this:
>
> {
> "Class": "HPBIOS_BIOSEnumeration",
> "Name": "USB Storage Boot",
> "Path": "\\Advanced\\Boot Options",
> "IsReadOnly": 0,
> ...
> "Value": "Enable",
> "Size": 2,
> "PossibleValues": [
> "Disable",
> "Enable"
> ]
> }
>
> This sort of thing really does not belong in the kernel.
I concur with your statement and behavior will be removed from driver code.
