search

microsoft / mu_feature_config

Project Mu - Feature Repo - UEFI User Config and Management Support
https://microsoft.github.io/mu/
Other
21 stars 28 forks source link

[Feature]: Prevent user loading wrong config xml file in ConfigEditor #418

Open kanechen66 opened 3 weeks ago

kanechen66 commented 3 weeks ago

Feature Overview

In the current ConfigEditor design, there is no version check for xml. It may lead to problems such as user loading wrong xml file so that the config is not taken effect or is not shown properly on UI.

Solution Overview

We have a proposal to add BIOS version and sha information in the xml file ex:

  <bios_info version="c219b.0.bs.3a02.gn.1" sha="ecd618ada779bb6c699f1d94009047c70bd78d3a" />

Then the configeditor can read BIOS sha from smbios type 11 oemstring and bios version from smbios type 0

it can provide a hint that user is using the wrong xml and unexpected behavior issues could happen.

Feel free to provide your thoughts on this :)

Alternatives Considered

No response

Urgency

Low

Are you going to implement the feature request?

I will implement the feature

Do you need maintainer feedback?

Maintainer feedback requested

Anything else?

No response

os-d commented 3 weeks ago

@kanechen66 , this is a good idea, thank you for implementing.

MarcChen46 commented 3 weeks ago

Since the BIOS SHA is the git commit id, and not all BIOS will implement it in SMBIOS, and it is possible that multiple BIOS are using the same XML content, which should still be allowed, so I have different thought about how to check it.

  1. Calculate the sha256 of the config XML file in PreBuild phase of plugin during BIOS build time and set it to MU_SCHEMA_FILE_SHA256 in PlatformBuild.py
  2. During POST, BIOS publish the MU_SCHEMA_FILE_SHA256 value to a MuScemaFileSha256 NVRAM variable.
  3. When ConfigEditor open the XML file, it also calculate the SHA256 of the XML file, and compare with the MuScemaFileSha256 NVRAM variable, then we can know if we are using the exact the same XML file when build the BIOS.
kanechen66 commented 2 weeks ago

Hi all, I have some other thoughts, and it extends Marc's idea. To prevent the below cases.

  1. Additional space or line break added in the xml
  2. Someone uses Linux to edit the xml, the line break btwn Linux and windows is different.

I propose using all the elements (including tag) in xml to hash and BIOS save this hash to a variable. When user opens ConfigEditor, it calc the hash and compare with the hash in BIOS var. Again, this version check won't block user using the ConfigEditor. it will just show some warning in the status box.

let me know if it's ok, if it's ok, i will implement in this way. thanks

kanechen66 commented 2 weeks ago

I created a PR here https://github.com/microsoft/mu_feature_config/pull/421 Saving hash in BIOS can also prevent adding new node in xml.
Adding new node in xml could break the backward compatibility because it requires SetupDataPkg\Tools\configschema.xsd change Let me know your thoughts on this. thank you