search

Foxboron / sbctl

:computer: :lock: :key: Secure Boot key manager
MIT License
1.49k stars 86 forks source link

Shouln't `/etc` be a place to store configuration instead of `/usr` #57

Closed igo95862 closed 3 months ago

igo95862 commented 3 years ago

/usr is the second major section of the filesystem. /usr is shareable, read-only data. That means that /usr should be shareable between various FHS-compliant hosts and must not be written to. Any information that is host-specific or varies with time is stored elsewhere.

From Linux Filesystem Hierarchy Standard

/usr might be useful if you have machines with same secure boot keys but I think for a host specific configuration /etc should be used.

Foxboron commented 3 years ago

It should. I went the easy route with sbkeysync since it doesn't use /etc. I have mostly put this off until I can replace sbsigntools with the go code and provide a configuration file for people.

igo95862 commented 3 years ago

Can I make a Pull Request? I think the /usr should be a fallback path for backwards compatibility.

Foxboron commented 3 years ago

I'm confused! I thought sbkeysync didn't use /etc, but that is because I think we have been discussing another directory.

But yes, please make a pull-request for the change :)

igo95862 commented 3 years ago

https://git.kernel.org/pub/scm/linux/kernel/git/jejb/sbsigntools.git/tree/src/sbkeysync.c#n94

It has both /etc and /usr under default paths. I think its better to pass --no-default-keystores option to prevent accidentally enrolling keys.

igo95862 commented 3 years ago

@Foxboron did you see my comment? https://github.com/Foxboron/sbctl/pull/58#discussion_r628931263

Foxboron commented 3 years ago

I did :) I need to just check it and verify it works properly, but work and other projects so won't have time before the weekend! Thanks for your patience

igo95862 commented 3 years ago

Sure. No rush.

Foxboron commented 3 years ago

So my current plan is to have a layout that looks like this:

Should also be provided with a sbctl migrate command so we can ensure people do not need to do any manual work for this.

igo95862 commented 3 years ago

So my current plan is to have a layout that looks like this:

Sounds good.

Should also be provided with a sbctl migrate command so we can ensure people do not need to do any manual work for this.

Maybe also post a warning when updating? Or even post on Arch news?

WhyNotHugo commented 3 years ago

I'd rather this be /var/lib/secureboot/keys/.

The application-agnostic naming of the path encourages other applications to adopt the same location for keys (improving interoperability) -- rather than each application having its own set of paths.

Foxboron commented 3 years ago

@igo95862

Maybe also post a warning when updating? Or even post on Arch news?

Arch news posts isn't for upstreams breaking stuff :smile:

I'd rather this be /var/lib/secureboot/keys/. The application-agnostic naming of the path encourages other applications to adopt the same location for keys (improving interoperability) -- rather than each application having its own set of paths.

This sounds nice in theory. But it would just give sbctl more issues. How should external tools deal with sbctl having keys in yubikeys or TPMs as an example? I think this is a nice thought when we are having plain key/cert files which are easy to deal with. But in more complicated scenarios it's not really a goal i want to accomplish.

WhyNotHugo commented 3 years ago

How should external tools deal with sbctl having keys in yubikeys or TPMs as an example?

Yeah, that kinda makes sense. I guess the metadata stored in those cases would be kinda non-standardish.

igo95862 commented 3 years ago

Hmm. Should keys really be in /var ? They probably need to be backed up but usually only stuff from /etc is backed up. (for example etckeeper) Another example is /var mounted without CoW on openSUSE.

beroal commented 2 years ago

I'd rather this be /var/lib/secureboot/keys/.

The application-agnostic naming of the path encourages other applications to adopt the same location for keys (improving interoperability) -- rather than each application having its own set of paths.

Standardizing the configuration format would be nice. (I even posted an issue that sbctl and another utility use different extensions for similar files. But that issue isn't actual now.) I'm not sure that using the name secureboot is a good idea. We can't say that the configuration format is recommended for all Secure Boot utilities. It isn't mentioned in the UEFI specifications ☺ which define Secure Boot. If other tools access sbctl configuration files, they can declare that they are sbctl-compatible.

beroal commented 2 years ago

Maybe, all files should be in /etc. “5.8. /var/lib : Variable state information”:

Users must never need to modify files in /var/lib to configure a package's operation, and the specific file hierarchy used to store the data must not be exposed to regular users.

The emphasis is mine. I think they mean that configuration files shouldn't be stored in /var/lib.

beroal commented 2 years ago

Should keys really be in /var ? They probably need to be backed up

In my another comment, I recommend not to back them up.

/usr might be useful if you have machines with same secure boot keys

And this should be a rare case. Sharing secret keys (any keys, not just Secure Boot keys) between computers is a bad practice: if one computer is compromised, all are.

WhyNotHugo commented 2 years ago

I try to set up my system in a declarative way, and I prefer to use drop-in file or alike for configuration.

An issue I come across with sbctl, is that files.db has two types of data: the settings for the file I want to sign (configuration), and the last signed checksum (state). I think it makes sense for these two bits of data to be stored separately so one file can be, e.g.: owned by some other package or placed by ansible, and the other is owned by sbctl.

Mentioning this here since it's related to breaking backwards compatibility with moving files around -- though it can potentially be done before moving everything else.

Foxboron commented 2 years ago

An issue I come across with sbctl, is that files.db has two types of data: the settings for the file I want to sign (configuration), and the last signed checksum (state). I think it makes sense for these two bits of data to be stored separately so one file can be, e.g.: owned by some other package or placed by ansible, and the other is owned by sbctl.

I just remembered, with the recent changes in go-uefi we can remove the state in the file since we can checksum the signed and unsigned files for verification. It was harder last year before go-uefi was mature.

Shished commented 2 years ago

Hey, any update on this issue?

conrad-heimbold commented 2 years ago

Any progress on this issue?

refind-install --localkeys uses /etc/refind.d/keys as folder for own secure boot keys: (see https://pagure.io/rEFInd-src/blob/master/f/refind-install#_224 and https://wiki.archlinux.org/title/REFInd#Using_your_own_keys ) but sbctl create-keys uses /usr/share/secureboot/keys: https://github.com/Foxboron/sbctl/blob/843fdc93b0813648d38969a09be456874ad74a6c/keys.go#L29

When the secure boot keys are manually user-generated, I also think it is better to place them under /etc/ .

For example sshd also saves their private host keys under /etc/: 

$ ls -al /etc/ssh/ssh_host_*_key
-rw-r-----.   1 root ssh_keys    227 25 Jul 20:50 ssh_host_ecdsa_key
-rw-r-----.   1 root ssh_keys    387 25 Jul 20:50 ssh_host_ed25519_key
-rw-r-----.   1 root ssh_keys   1675 25 Jul 20:50 ssh_host_rsa_key

As far as I know, everything under /usr should not be edited by humans.

My setup so far was to symlink all necessary secure boot keys in /etc/refind.d/keys: 

$ ls -al /etc/refind.d/keys/
... 
lrwxrwxrwx. 1 root root   34 25 Jul 20:50 refind_local.cer -> /etc/keys/secure-boot/keys/DB.cer
lrwxrwxrwx. 1 root root   34 25 Jul 20:50 refind_local.crt -> /etc/keys/secure-boot/keys/DB.crt
lrwxrwxrwx. 1 root root   34 25 Jul 20:50 refind_local.key -> /etc/keys/secure-boot/keys/DB.key
...
WhyNotHugo commented 2 years ago

sbctl could "support" refind by adding a symlink /etc/refind.d/keys/ -> /etc/sbctl/keys/

WhyNotHugo commented 2 years ago

I just remembered, with the recent changes in go-uefi we can remove the state in the file since we can checksum the signed and unsigned files for verification. It was harder last year before go-uefi was mature.

I tried to address this bit in https://github.com/Foxboron/sbctl/pull/157, but I'm not sure my approach is the best. Feedback on that is welcome.

dngray commented 2 years ago

The keys should definitely not be stored in /usr. This presents a problem for immutable distributions (silverblue, steam deck, nixos as /usr is read only.

WhyNotHugo commented 2 years ago

Any feedback on #157 is welcome; it's a necessary bit of work that, AFAIK, needs to happen before this can be addressed.

dkwo commented 6 months ago

@Foxboron Do you still plan to split files between /etc and /var/lib, or rather put everything in /etc, or else? if the second, would it make sense to build sbctl with go_ldflags="-X ${go_import_path}.DatabasePath=/etc/secureboot" or similar for now, while the change gets implemented? I tried it a few months ago, and it seems to work fine for me.

Foxboron commented 6 months ago

@dkwo

Yes, I need to find some more motivation to work on the less cool parts of sbctl soon I reckon.

Foxboron commented 3 months ago

I've started reworking the key handling, which lead me to make some abstractions over the key hierarchy, which lead me to writing state/config stuff to keep track of everything.

To make everything hard for myself I'm thus working on semi-rewriting parts of the storage backend and introducing a config. Which means I'd might as well introduce sbctl migrate to ensure people can migrate keys to new structures and so on.

I've settled on a yaml config file which is intended to be some form of declarative configuration for sbctl. It would make us less reliant on files.db and we could just issue sbctl setup upon install to sign our setup.

Default directories:

        Config:    "/etc/sbctl/config.yaml",
        Keydir:    "/etc/sbctl/keys",
        FilesDb:   "/var/lib/sbctl/files.db",
        BundlesDb: "/var/lib/sbctl/bundles.db",

Verbose config file example:

---
keydir: /etc/sbctl/keys
guid: /var/lib/sbctl/GUID
files_db: /var/lib/sbctl/files.db
bundles_db: /var/lib/sbctl/bundles.db
vendor_keys:
  - microsoft
files:
  - path: /boot/vmlinuz-linux-lts
  - path: /usr/lib/fwupd/efi/fwupdx64.efi
    output: /usr/lib/fwupd/efi/fwupdx64.efi.signed
keys:
  pk:
    privkey: /etc/sbctl/keys/PK/PK.key
    pubkey: /etc/sbctl/keys/PK/PK.pem
    type: file
  kek:
    privkey: /etc/sbctl/keys/KEK/KEK.key
    pubkey: /etc/sbctl/keys/KEK/KEK.pem
    type: file
  db:
    privkey: /etc/sbctl/keys/db/db.key
    pubkey: /etc/sbctl/keys/db/db.pem
    type: file

Thoughts or something ideas?

Foxboron commented 3 months ago

There is an argument that keys are not configuration, so they might belong into /var/lib/sbctl/keys instead of /etc. I'm inclined to agree?

MithicSpirit commented 3 months ago

There is an argument that keys are not configuration, so they might belong into /var/lib/sbctl/keys instead of /etc. I'm inclined to agree?

On NixOS this would be preferrable, since /etc is managed by nix and often ephemeral.

ml- commented 3 months ago 
keys:
  pk:
    privkey:
    pubkey:
    type: file
  kek:
    privkey:
    pubkey:
    type: file
  db:
    privkey:
    pubkey:
    type: file

Having a single type for privkey and pubkey implies both are retrieved using the same protocol. Is there a scenario where privkey is stored/retrieved in a more secure way than the pubkey?

What I have in mind:

keys:
  kek:
    privkey: tpm2://kek.key
    pubkey: /var/lib/sbctl/keys/pubkey.pem
Foxboron commented 3 months ago

Having a single type for privkey and pubkey implies both are retrieved using the same protocol. Is there a scenario where privkey is stored/retrieved in a more secure way than the pubkey?

No, type just defined the backend protocol and it's the privy of the protocol to interpret the strings. In the case of all of these backends they would need to produce a x509 certificate while the private key will be storage specific.

dkwo commented 3 months ago

Thanks for working on this. I hope the files part

files:
  - path: /boot/vmlinuz-linux-lts
  - path: /usr/lib/fwupd/efi/fwupdx64.efi
    output: /usr/lib/fwupd/efi/fwupdx64.efi.signed

is not mandatory, i.e. one can still sign versioned kernels from a custom script (e.g. to produce a UKI)?

Foxboron commented 3 months ago

@dkwo You can still use sign -s but the config can be used for initial setup. Think of it as a declarative thing.

Foxboron commented 3 months ago

Veryvery broken because of vendored stuff, but WIP branch with changes here: https://github.com/Foxboron/sbctl/pull/323

dngray commented 3 months ago

On NixOS this would be preferrable, since /etc is managed by nix and often ephemeral.

That is really the correct place like the keys for systemd-homed are in /var/lib/systemd/home, so yes should be /var/lib/sbctl

Foxboron commented 3 months ago

sbctl setup --print-config will serialize the config and optionally the state into yaml or json. It can be used to restore the sbctl settings across installs or to write scripts that needs to interact with sbctl.

Note the config it parses can be either yaml or json.

λ sbctl-config morten/config Ɇ » ./sbctl setup
keydir: /var/lib/sbctl/keys
guid: /var/lib/sbctl/GUID
files_db: /var/lib/sbctl/files.db
bundles_db: /var/lib/sbctl/bundles.db
files:
- path: /boot/vmlinuz-linux-lts
  output: /boot/vmlinuz-linux-lts
- path: /efi/EFI/BOOT/BOOTX64.EFI
  output: /efi/EFI/BOOT/BOOTX64.EFI
- path: /efi/EFI/Linux/arch-linux.efi
  output: /efi/EFI/Linux/arch-linux.efi
- path: /home/fox/Git/prosjekter/Go/sbctl.git/sbctl/vmlinuz-linux
  output: /home/fox/Git/prosjekter/Go/sbctl.git/sbctl/vmlinuz-linux
- path: /usr/lib/fwupd/efi/fwupdx64.efi
  output: /usr/lib/fwupd/efi/fwupdx64.efi.signed
keys:
  pk:
    privkey: /var/lib/sbctl/keys/PK/PK.key
    pubkey: /var/lib/sbctl/keys/PK/PK.pem
    type: file
  kek:
    privkey: /var/lib/sbctl/keys/KEK/KEK.key
    pubkey: /var/lib/sbctl/keys/KEK/KEK.pem
    type: file
  db:
    privkey: /var/lib/sbctl/keys/db/db.key
    pubkey: /var/lib/sbctl/keys/db/db.pem
    type: file
λ sbctl-config morten/config Ɇ » ./sbctl setup --json | jq
{
  "keydir": "/var/lib/sbctl/keys",
  "guid": "/var/lib/sbctl/GUID",
  "files_db": "/var/lib/sbctl/files.db",
  "bundles_db": "/var/lib/sbctl/bundles.db",
  "files": [
    {
      "path": "/efi/EFI/BOOT/BOOTX64.EFI",
      "output": "/efi/EFI/BOOT/BOOTX64.EFI"
    },
    {
      "path": "/efi/EFI/Linux/arch-linux.efi",
      "output": "/efi/EFI/Linux/arch-linux.efi"
    },
    {
      "path": "/home/fox/Git/prosjekter/Go/sbctl.git/sbctl/vmlinuz-linux",
      "output": "/home/fox/Git/prosjekter/Go/sbctl.git/sbctl/vmlinuz-linux"
    },
    {
      "path": "/usr/lib/fwupd/efi/fwupdx64.efi",
      "output": "/usr/lib/fwupd/efi/fwupdx64.efi.signed"
    },
    {
      "path": "/boot/vmlinuz-linux-lts",
      "output": "/boot/vmlinuz-linux-lts"
    }
  ],
  "keys": {
    "pk": {
      "privkey": "/var/lib/sbctl/keys/PK/PK.key",
      "pubkey": "/var/lib/sbctl/keys/PK/PK.pem",
      "type": "file"
    },
    "kek": {
      "privkey": "/var/lib/sbctl/keys/KEK/KEK.key",
      "pubkey": "/var/lib/sbctl/keys/KEK/KEK.pem",
      "type": "file"
    },
    "db": {
      "privkey": "/var/lib/sbctl/keys/db/db.key",
      "pubkey": "/var/lib/sbctl/keys/db/db.pem",
      "type": "file"
    }
  }
}
λ sbctl-config morten/config Ɇ » ./sbctl setup --json --with-state | jq
{
  "config": {
    "keydir": "/var/lib/sbctl/keys",
    "guid": "/var/lib/sbctl/GUID",
    "files_db": "/var/lib/sbctl/files.db",
    "bundles_db": "/var/lib/sbctl/bundles.db",
    "files": [
      {
        "path": "/efi/EFI/Linux/arch-linux.efi",
        "output": "/efi/EFI/Linux/arch-linux.efi"
      },
      {
        "path": "/home/fox/Git/prosjekter/Go/sbctl.git/sbctl/vmlinuz-linux",
        "output": "/home/fox/Git/prosjekter/Go/sbctl.git/sbctl/vmlinuz-linux"
      },
      {
        "path": "/usr/lib/fwupd/efi/fwupdx64.efi",
        "output": "/usr/lib/fwupd/efi/fwupdx64.efi.signed"
      },
      {
        "path": "/boot/vmlinuz-linux-lts",
        "output": "/boot/vmlinuz-linux-lts"
      },
      {
        "path": "/efi/EFI/BOOT/BOOTX64.EFI",
        "output": "/efi/EFI/BOOT/BOOTX64.EFI"
      }
    ],
    "keys": {
      "pk": {
        "privkey": "/var/lib/sbctl/keys/PK/PK.key",
        "pubkey": "/var/lib/sbctl/keys/PK/PK.pem",
        "type": "file"
      },
      "kek": {
        "privkey": "/var/lib/sbctl/keys/KEK/KEK.key",
        "pubkey": "/var/lib/sbctl/keys/KEK/KEK.pem",
        "type": "file"
      },
      "db": {
        "privkey": "/var/lib/sbctl/keys/db/db.key",
        "pubkey": "/var/lib/sbctl/keys/db/db.pem",
        "type": "file"
      }
    }
  },
  "installed": true
}
Foxboron commented 3 months ago

Fixed with https://github.com/Foxboron/sbctl/commit/4f31817e14063335e04984a167edf72777d089e0