raito/lanzaboote
0
0
Fork 0
Code Issues 45 Pull requests 6 Projects Releases 6 Packages Wiki Activity Actions

feat: remote signing server and signature generalization #278

Closed
RaitoBezarius wants to merge 15 commits from signing-client into master
pull from: signing-client
merge into: raito:master
Conversation 21 Commits 15 Files changed 26 +3103 -345
RaitoBezarius commented 2024-01-03 19:25:18 +00:00 (Migrated from github.com)
Copy link

This PR is composed of ~four changes:

  • generalization of signature schemes in the tool
  • generalization of how to assemble stubs in the shared library
  • a remote signature server that sends you signed stub in exchange for signature requests
  • an implementation inside lzbt-systemd of remote signing

This enables moving the local signature to another server… or even maybe a hardware security token!

In the future, the server can ask for attestations or any kind of things to give the signed stub.

Note

Initrd secrets are not supported by the remote signer

This is out of scope for this PR and will probably not supported as I plan to bring systemd credentials
first inside of NixOS and remove the legacy initrd secrets which are a broken feature.

What were alternatives?

  • Offer a way to send the secret to the signer: NAK because an attacker could present a secret being a trivial privilege escalation binary.
  • Force the secret to be present on the signer: NAK because this would require at least a policy to allow / disallow access to a given secret based on the requester, this is too big for this PR.
    Therefore, initrd secrets will NOT work with the remote signer.
This PR is composed of ~four changes: - generalization of signature schemes in the tool - generalization of how to assemble stubs in the shared library - a remote signature server that sends you signed stub in exchange for signature requests - an implementation inside lzbt-systemd of remote signing This enables moving the local signature to another server… or even maybe a hardware security token! In the future, the server can ask for attestations or any kind of things to give the signed stub. > [!NOTE] > #### Initrd secrets are not supported by the remote signer > This is out of scope for this PR and will probably not supported as I plan to bring systemd credentials > first inside of NixOS and remove the legacy initrd secrets which are a broken feature. > #### What were alternatives? > - Offer a way to send the secret to the signer: NAK because an attacker could present a secret being a trivial privilege escalation binary. > - Force the secret to be present on the signer: NAK because this would require at least a policy to allow / disallow access to a given secret based on the requester, this is too big for this PR. > Therefore, initrd secrets will *NOT* work with the remote signer.
RaitoBezarius commented 2024-01-03 19:26:36 +00:00 (Migrated from github.com)
Copy link

Replaces #228, I need to fix more conflicts and it should be good.

Replaces #228, I need to fix more conflicts and it should be good.
nikstur commented 2024-01-21 12:30:59 +00:00 (Migrated from github.com)
Copy link

I converted this to a draft since it depends on your fork.

I converted this to a draft since it depends on your fork.
nikstur (Migrated from github.com) reviewed 2024-01-21 13:44:09 +00:00
flake.nix
@ -200,3 +223,4 @@
checks =
let
nixosLib = import (pkgs.path + "/nixos/lib") { };
lanzaLib = import ./nix/tests/lib.nix {
nikstur (Migrated from github.com) commented 2024-01-21 12:46:31 +00:00
Copy link

We should have at least some rudimentary unit/integration tests for the server as well.

We should have at least some rudimentary unit/integration tests for the server as well.
nix/modules/lanzaboote.nix
nikstur (Migrated from github.com) commented 2024-01-21 12:45:13 +00:00
Copy link

These options will need a defaultText again otherwise the manual generation will cry again.

These options will need a `defaultText` again otherwise the manual generation will cry again.
nix/tests/remote-signing.nix
@ -0,0 +16,4 @@
server.wait_for_open_port(9999)
# Perform a switch to the remote configuration
# and contact the server to get the right bootables.
with subtest("Activation will request for remote signing"):
nikstur (Migrated from github.com) commented 2024-01-21 12:43:25 +00:00
Copy link

This means the server has to be available at the time re-building. This probably won't scale well but should be good enough for now.

This means the server has to be available at the time re-building. This probably won't scale well but should be good enough for now.
rust/tool/server/Cargo.toml
nikstur (Migrated from github.com) commented 2024-01-21 12:37:43 +00:00
Copy link

We don't need these dependencies here as we don't have tests, right?

We don't need these dependencies here as we don't have tests, right?
nikstur (Migrated from github.com) commented 2024-01-21 13:06:44 +00:00
Copy link
name = "lanzaboote_signd"
```suggestion name = "lanzaboote_signd" ```
rust/tool/server/src/main.rs
nikstur (Migrated from github.com) commented 2024-01-21 12:55:55 +00:00
Copy link

Wouldn't /sign/stub and /sign/store-path be cleaner?

            (POST) (/sign/stub) => {
Wouldn't `/sign/stub` and `/sign/store-path` be cleaner? ```suggestion (POST) (/sign/stub) => { ```
rust/tool/shared/Cargo.toml
nikstur (Migrated from github.com) commented 2024-01-21 13:04:55 +00:00
Copy link

This should be an optional depedency guarded by a compile time flag.

This should be an optional depedency guarded by a compile time flag.
rust/tool/shared/src/signature/README.md
nikstur (Migrated from github.com) commented 2024-01-21 12:49:08 +00:00
Copy link

I'd argue for keeping the future design parts to a minimum as they don't age well.

I'd argue for keeping the future design parts to a minimum as they don't age well.
rust/tool/shared/src/signature/README.md
@ -0,0 +1,12 @@
# Signatures capabilities of Lanzaboote
Currently, lanzaboote can perform signatures of PE binaries based on local keypairs present on disk.
nikstur (Migrated from github.com) commented 2024-01-21 12:49:38 +00:00
Copy link

Isn't the whole point of the PR that we can now also use keys from the remote server? ^^

Isn't the whole point of the PR that we can now also use keys from the remote server? ^^
rust/tool/shared/src/signature/mod.rs
nikstur (Migrated from github.com) commented 2024-01-21 12:52:00 +00:00
Copy link

These belong at the very top of the file.

These belong at the very top of the file.
rust/tool/shared/src/signature/mod.rs
@ -0,0 +19,4 @@
/// Verify the signature of a PE binary, provided as bytes.
/// Return true if the signature was verified.
fn verify(&self, pe_binary: &[u8]) -> Result<bool>;
nikstur (Migrated from github.com) commented 2024-01-21 12:52:49 +00:00
Copy link

It's a little odd that the Signer trait can also verify.

It's a little odd that the `Signer` trait can also verify.
RaitoBezarius (Migrated from github.com) reviewed 2024-01-21 15:15:31 +00:00
nix/tests/remote-signing.nix
@ -0,0 +16,4 @@
server.wait_for_open_port(9999)
# Perform a switch to the remote configuration
# and contact the server to get the right bootables.
with subtest("Activation will request for remote signing"):
RaitoBezarius (Migrated from github.com) commented 2024-01-21 15:15:31 +00:00
Copy link

Hm, why wouldn't it scale well?

Hm, why wouldn't it scale well?
RaitoBezarius (Migrated from github.com) reviewed 2024-01-21 15:16:06 +00:00
rust/tool/shared/src/signature/README.md
@ -0,0 +1,12 @@
# Signatures capabilities of Lanzaboote
Currently, lanzaboote can perform signatures of PE binaries based on local keypairs present on disk.
RaitoBezarius (Migrated from github.com) commented 2024-01-21 15:16:06 +00:00
Copy link

Yes, this is the whole point. But this is in addition to the old capabilities?

Yes, this is the whole point. But this is in addition to the old capabilities?
RaitoBezarius (Migrated from github.com) reviewed 2024-01-21 15:16:35 +00:00
rust/tool/shared/src/signature/mod.rs
@ -0,0 +19,4 @@
/// Verify the signature of a PE binary, provided as bytes.
/// Return true if the signature was verified.
fn verify(&self, pe_binary: &[u8]) -> Result<bool>;
RaitoBezarius (Migrated from github.com) commented 2024-01-21 15:16:35 +00:00
Copy link

I can make it like signature and derive a VerifyingKey handle and you can call verify on that but it seems like overengineering to me.

I can make it like `signature` and derive a `VerifyingKey` handle and you can call verify on that but it seems like overengineering to me.
RaitoBezarius (Migrated from github.com) reviewed 2024-01-21 15:16:45 +00:00
rust/tool/server/src/main.rs
RaitoBezarius (Migrated from github.com) commented 2024-01-21 15:16:45 +00:00
Copy link

True!

True!
RaitoBezarius (Migrated from github.com) reviewed 2024-01-21 15:16:54 +00:00
rust/tool/shared/Cargo.toml
RaitoBezarius (Migrated from github.com) commented 2024-01-21 15:16:54 +00:00
Copy link

Right.

Right.
nikstur (Migrated from github.com) reviewed 2024-01-21 15:25:17 +00:00
rust/tool/shared/src/signature/mod.rs
@ -0,0 +19,4 @@
/// Verify the signature of a PE binary, provided as bytes.
/// Return true if the signature was verified.
fn verify(&self, pe_binary: &[u8]) -> Result<bool>;
nikstur (Migrated from github.com) commented 2024-01-21 15:25:16 +00:00
Copy link

Yes, that'd be overengineering. Is there a higher-level concept that encapsulates both verifying and signing? Not a big issue if we keep the Signer terminology and just add a comment.

Yes, that'd be overengineering. Is there a higher-level concept that encapsulates both verifying and signing? Not a big issue if we keep the Signer terminology and just add a comment.
nikstur (Migrated from github.com) reviewed 2024-01-21 15:26:35 +00:00
nix/tests/remote-signing.nix
@ -0,0 +16,4 @@
server.wait_for_open_port(9999)
# Perform a switch to the remote configuration
# and contact the server to get the right bootables.
with subtest("Activation will request for remote signing"):
nikstur (Migrated from github.com) commented 2024-01-21 15:26:35 +00:00
Copy link

What if the server is unavailable at the time of re-build? Does the re-building just fail? Do you want to add client-side retries? None of the solutions I can think of are super elegant.

What if the server is unavailable at the time of re-build? Does the re-building just fail? Do you want to add client-side retries? None of the solutions I can think of are super elegant.
RaitoBezarius (Migrated from github.com) reviewed 2024-01-23 02:13:38 +00:00
rust/tool/shared/src/signature/mod.rs
@ -0,0 +19,4 @@
/// Verify the signature of a PE binary, provided as bytes.
/// Return true if the signature was verified.
fn verify(&self, pe_binary: &[u8]) -> Result<bool>;
RaitoBezarius (Migrated from github.com) commented 2024-01-23 02:13:38 +00:00
Copy link

A notary?

A notary?
RaitoBezarius (Migrated from github.com) reviewed 2024-01-23 02:13:53 +00:00
rust/tool/shared/src/signature/mod.rs
RaitoBezarius (Migrated from github.com) commented 2024-01-23 02:13:53 +00:00
Copy link

Ack

Ack
RaitoBezarius (Migrated from github.com) reviewed 2024-01-23 02:14:05 +00:00
flake.nix
@ -200,3 +223,4 @@
checks =
let
nixosLib = import (pkgs.path + "/nixos/lib") { };
lanzaLib = import ./nix/tests/lib.nix {
RaitoBezarius (Migrated from github.com) commented 2024-01-23 02:14:05 +00:00
Copy link

Ack

Ack
RaitoBezarius (Migrated from github.com) reviewed 2024-01-23 02:17:10 +00:00
nix/tests/remote-signing.nix
@ -0,0 +16,4 @@
server.wait_for_open_port(9999)
# Perform a switch to the remote configuration
# and contact the server to get the right bootables.
with subtest("Activation will request for remote signing"):
RaitoBezarius (Migrated from github.com) commented 2024-01-23 02:17:10 +00:00
Copy link

The normal person usecase would be:

I have a dozen of machines and they all have Secure Boot, I'd like to rebuild switch without all of them having access to the private key, I can just contact my secure server with TPM2 or HSM who has the machinery to perform secure signatures and send it my stuff its way.

If my secure server is down, I cannot sign my new binaries.

What is this really saying is that remote secure signing is load bearing and you need high availability if you don't want to fail rebuilding your system in those scenarios.

High availability can be facilitated on our side by having multiple fallbacks we can try in a certain order, e.g. remote secure signing (convenient, fast, secure!) and then the fallback is you pull your Yubikey and sign it on the "field recovery" certificate or whatever.

There will be, once those PRs land, a whole discussion on how do you seriously manage this at scale.

The normal person usecase would be: I have a dozen of machines and they all have Secure Boot, I'd like to rebuild switch without all of them having access to the private key, I can just contact my secure server with TPM2 or HSM who has the machinery to perform secure signatures and send it my stuff its way. If my secure server is down, I cannot sign my new binaries. What is this really saying is that remote secure signing is load bearing and you need high availability if you don't want to fail rebuilding your system in those scenarios. High availability can be facilitated on our side by having multiple fallbacks we can try in a certain order, e.g. remote secure signing (convenient, fast, secure!) and then the fallback is you pull your Yubikey and sign it on the "field recovery" certificate or whatever. There will be, once those PRs land, a whole discussion on how do you seriously manage this at scale.
RaitoBezarius (Migrated from github.com) reviewed 2024-01-23 02:17:33 +00:00
rust/tool/server/Cargo.toml
RaitoBezarius (Migrated from github.com) commented 2024-01-23 02:17:33 +00:00
Copy link

Yep

Yep
RaitoBezarius (Migrated from github.com) reviewed 2024-01-23 02:18:22 +00:00
rust/tool/shared/src/signature/README.md
RaitoBezarius (Migrated from github.com) commented 2024-01-23 02:18:22 +00:00
Copy link

Right, let's do that.

Right, let's do that.
RaitoBezarius commented 2024-01-25 02:15:42 +00:00 (Migrated from github.com)
Copy link

I converted this to a draft since it depends on your fork.

This is not dependent on a fork anymore, now.

> I converted this to a draft since it depends on your fork. This is not dependent on a fork anymore, now.
RaitoBezarius (Migrated from github.com) reviewed 2024-01-25 02:25:08 +00:00
rust/tool/shared/Cargo.toml
RaitoBezarius (Migrated from github.com) commented 2024-01-25 02:25:08 +00:00
Copy link

I did something, let me know how you like it.

I did something, let me know how you like it.
RaitoBezarius commented 2024-01-25 03:21:32 +00:00 (Migrated from github.com)
Copy link

Personal TODO and expectations for reviewers:

  • I need to update the flake build of signd.
  • I need to fix the wiring up for the integration testing.

@nikstur Do you have anything else you would like me to address for this?

Personal TODO and expectations for reviewers: - I need to update the flake build of signd. - I need to fix the wiring up for the integration testing. @nikstur Do you have anything else you would like me to address for this?
blitz commented 2024-02-10 19:10:19 +00:00 (Migrated from github.com)
Copy link

Given that this PR is currently gigantic and you say it consists of four distinct parts, can we possibly split it in multiple PRs for review?

From my perspective we are also missing overview documentation here.

Given that this PR is currently gigantic and you say it consists of four distinct parts, can we possibly split it in multiple PRs for review? From my perspective we are also missing overview documentation here.
RaitoBezarius commented 2024-02-10 19:55:06 +00:00 (Migrated from github.com)
Copy link

Given that this PR is currently gigantic and you say it consists of four distinct parts, can we possibly split it in multiple PRs for review?

From my perspective we are also missing overview documentation here.

Sure!

> Given that this PR is currently gigantic and you say it consists of four distinct parts, can we possibly split it in multiple PRs for review? > > From my perspective we are also missing overview documentation here. Sure!

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug

Something isn't working

  
dependency

Issues related to our dependencies

  
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
enhancement

New feature or request

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
question

Further information is requested

  
review-next

   
security

   
stub

   
tool

   
wontfix

This will not be worked on

No labels
bug
dependency
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
review-next
security
stub
tool
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: raito/lanzaboote#278
No description provided.
Delete branch "signing-client"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?