status: Warn about firmware quirks

[dawidpotocki]

Related: #181

Just a rough idea, would like to hear your input.

{
  "installed": true,
  "guid": "6e2b1be3-6ecf-41e6-a59b-a30efdfa85ba",
  "setup_mode": false,
  "secure_boot": true,
  "vendors": [],
  "firmware_quirks": [
    {
      "id": "FQ0001",
      "name": "MSI defaults to executing on policy violation",
      "link": "https://github.com/Foxboron/sbctl/wiki/FQ0001",
      "severity": "CRITICAL"
    }
  ]
}

[Foxboron]

I like this!

Generally I would need help to maintain a list of quirks, we probably need a new section as well.

• /sys/devices/virtual/dmi/id/ reading should be serialized into a struct. Probably it's own module as well since we might want to use this for debug output or an status --expert thing.
• I think we can express fq0001.go entierly as a structs if the above is done.

Generally it's a very good first draft :) Just need to figure out how to best document all of this!

[dawidpotocki]

I found that it is possible to get the IFR data from the OS.

This method relies on reading HiiDB efivar which has the address and size
of the UEFI Human Interface Infrastructure that we can use to read data
we need from memory.

The only issue is that it requires reading from /dev/mem and thus root
access. Because of it, it won't work if Kernel Lockdown is enabled in
confidentiality mode, but it should work in integrity mode. Another
thing is that it doesn't tell us if the user has it enabled, it only
tells us the default values.

I wrote a very messy proof of concept in half Python, half shell:

#!/usr/bin/env python3

import os, shutil, struct

with open("/sys/firmware/efi/efivars/HiiDB-1b838190-4625-4ead-abc9-cd5e6af18fe0", "rb") as f:
    size, addr = struct.unpack('@xxxxII', f.read())
with open("/dev/mem", "rb") as f:
    f.seek(addr)
    hiidb = f.read(size)

if hiidb.replace(b"\x00",b"").find(b"Image Execution Policy") == -1:
    print("Image Execution Policy settings not found in HiiDB")
else:
    print("Image Execution Policy settings found in HiiDB")
    try:
        shutil.rmtree("/tmp/hiidb")
    except FileNotFoundError:
        pass
    os.mkdir("/tmp/hiidb")
    with open("/tmp/hiidb/hiidb", "wb") as f:
        f.write(hiidb)
	os.system("""
		cd /tmp/hiidb
		ifrextractor /tmp/hiidb/hiidb 1>/dev/null
		output="$(grep -A1 -E 'OneOf Prompt: "(Option ROM|Removable Media|Fixed Media)", Help: "Image Execution Policy' /tmp/hiidb/*ifr.txt)"

		if echo "$output" | grep -q "DefaultId: 0x0"; then
			printf "\033[1;31mInsecure defaults\033[0m\n" "$1"
		else
			printf "\033[1;32mSecure defaults\033[0m\n" "$1"
		fi
	""")
    shutil.rmtree("/tmp/hiidb")

Because of the issues with this approach, I don't think it would make
sense to add anything like this to sbctl, even though it would be more
accurate than checking dates of firmware.

I don't know how else I would be able to read this data if not from
/dev/mem. Suggestions are welcome.

[Foxboron]

I've started improving the end-to-end unit testing support in sbctl.

So if you want to try write unit tests for the CLI interface to test this feature it should be possible :)

https://github.com/Foxboron/sbctl/blob/master/cmd/sbctl/status_test.go

https://github.com/Foxboron/go-uefi/blob/master/efi/efitest/files.go

[dawidpotocki]

Sure thing, will do.

I just had some free time and tried, but I'm not really sure how I'm
supposed to set files.

SetFS(
	fstest.MapFS{
		"/sys/devices/virtual/dmi/id/board_vendor": {
			Data: []byte{0x0, 0x6, 0x0, 0x0, 0x1}},
	},
	fstest.MapFS{
		"/sys/firmware/efi/efivars/SetupMode-8be4df61-93ca-11d2-aa0d-00e098032b8c": {
			Data: []byte{0x0, 0x6, 0x0, 0x0, 0x1}},
	},
	efitest.SecureBootOn(),
)

I tried something like this and I get such output:

{false  true true [] [{FQ0001 Defaults to executing on Secure Boot policy violation https://github.com/Foxboron/sbctl/wiki/FQ0001 CRITICAL date}]}

When I change Setup Mode part to

Data: []byte{0x0, 0x6, 0x0, 0x0, 0x0}},

{false  false true [] [{FQ0001 Defaults to executing on Secure Boot policy violation https://github.com/Foxboron/sbctl/wiki/FQ0001 CRITICAL date}]}

When I change the check in fq0001.go

if dmi.Table.BoardVendor == "not Micro-Star International Co., Ltd." && dmi.Table.ChassisType == "3" {

{false  true true [] []}

So clearly it does set Setup Mode and Secure Boot efivar files, but it
doesn't change the board_vendor file at all. I assume I'm doing
something very wrong, but not sure what.

Will look more into it in the morning in my timezone.

[Foxboron]

Keep in mind you need to shim all filesystem calls with the fs module :)

e1ce225
57f51d9

EDIT: Also don't spend too much time on the testing if you dont have time for it. I thought it might just make it easier for you to test the CLI output of the code.

[dawidpotocki]

I had actually tried changing os to sbctl/fs… and I just now
realised what was the problem.

dmi.go had var Table = parseDMI(), which I guess was ran before the
SetFS() stuff.

If we add dmi.ParseDMI() inside fq0001.go's HasFQ0001() or
quirks.go's CheckFirmwareQuirks(), it works.

Idk what to do with this now, I guess I can just execute it in
quirks.go.

EDIT: Just checked strace and it was parsing DMI on every command, oh well,
dumb me.

Also don't spend too much time on the testing if you dont have time for it. I
thought it might just make it easier for you to test the CLI output of the
code.

It's fine, I don't mind.

[Foxboron]

Idk what to do with this now, I guess I can just execute it in quirks.go.

Please only parse it when we need it :) We probably don't need this as global state

[dawidpotocki]

Since there is no way to PR to wiki, here are my changes:
https://github.com/dawidpotocki/sbctl/wiki

EDIT: Maybe we should use someting else than the wiki? Some information will
require maintainance and contributing to wikis is a pain.

Could ship these docs with sbctl, but it would get outdated fast,

I think having some website would be a good idea.

[Foxboron]

I did buy the domain secureboot.dev a while ago :)

https://github.com/Foxboron/secureboot.dev

Might be an idea to figure out a usecase for that thing.

[Foxboron]

Thanks for working on this :)

[jdholtz]

Thanks for adding this feature! I just applied the changes from the wiki and it worked from my testing. Will these quirks still be displayed even after the changes in the BIOS are made? I want to make sure I have it set up properly and didn't know if this warning was still telling me otherwise.

[dawidpotocki]

Yes, they will still be displayed, at least this one.
This is because it's hard to reliably detect this setting at runtime.