Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add method to validate signature of efi file #337

Merged
merged 12 commits into from
May 22, 2024
Merged

Conversation

Itxaka
Copy link
Member

@Itxaka Itxaka commented May 16, 2024

This seems rigth but needs testing. Also makes sense to have this in the sdk instead maybe?

Fixes kairos-io/kairos#2200

@Itxaka Itxaka requested a review from a team May 16, 2024 13:36
Signed-off-by: Itxaka <itxaka@kairos.io>
pkg/uki/common.go Outdated Show resolved Hide resolved
Copy link

codecov bot commented May 16, 2024

Codecov Report

Attention: Patch coverage is 52.68817% with 44 lines in your changes are missing coverage. Please review.

Project coverage is 54.71%. Comparing base (8646391) to head (e65d4d8).
Report is 12 commits behind head on main.

Files Patch % Lines
pkg/uki/common.go 58.33% 27 Missing and 8 partials ⚠️
pkg/uki/upgrade.go 0.00% 9 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #337      +/-   ##
==========================================
- Coverage   59.07%   54.71%   -4.37%     
==========================================
  Files          39       43       +4     
  Lines        4242     4670     +428     
==========================================
+ Hits         2506     2555      +49     
- Misses       1460     1831     +371     
- Partials      276      284       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Itxaka <itxaka@kairos.io>
Signed-off-by: Itxaka <itxaka@kairos.io>
pkg/uki/common.go Outdated Show resolved Hide resolved
pkg/uki/common.go Outdated Show resolved Hide resolved
pkg/uki/common.go Outdated Show resolved Hide resolved
pkg/uki/common.go Outdated Show resolved Hide resolved
pkg/uki/common.go Outdated Show resolved Hide resolved
pkg/uki/common_test.go Outdated Show resolved Hide resolved
Tests showed that the approach was wrong. Now we check for the list of
db certs and extract those properly

Signed-off-by: Itxaka <itxaka@kairos.io>
Signed-off-by: Itxaka <itxaka@kairos.io>
@Itxaka
Copy link
Member Author

Itxaka commented May 17, 2024

Seems to me like this has gottena bit big. Just to parse a couple of things, we need to bring a library for parsing pe files, so for me it makes more sense to have this in the sdk as a generic function that checks the signature validity instead of cluttering teh agent with this maybe... ?

pkg/uki/common_test.go Outdated Show resolved Hide resolved
pkg/uki/common_test.go Outdated Show resolved Hide resolved
pkg/uki/common_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@jimmykarily jimmykarily left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor wording suggestions in tests. Other than that looks good. I agree that it feels better suited for the sdk (not that it makes any difference regarding library imports and such).

Itxaka and others added 2 commits May 20, 2024 11:37
Co-authored-by: Dimitris Karakasilis <dimitris@karakasilis.me>
Signed-off-by: Itxaka <itxaka@kairos.io>
@Itxaka Itxaka marked this pull request as ready for review May 20, 2024 09:47
@Itxaka Itxaka requested review from jimmykarily and a team May 20, 2024 09:47
 - Check for zero size before parsing
 - mmap the file, seems to consume less memory
 - return on unknown errors when stat-ing the file
 - convert the type so we can keep using v1.FS
 - Add more debug logging
 - Extra test for zero size
 - Fix the location of the efi file during upgrade check

Signed-off-by: Itxaka <itxaka@kairos.io>
@Itxaka
Copy link
Member Author

Itxaka commented May 20, 2024

Seems to be working with a proper EFI:

2024-05-20T13:29:21Z INF Copying ttl.sh/kairosupgrade:latest source to /efi
2024-05-20T13:29:47Z INF Finished copying ttl.sh/kairosupgrade:latest into /efi
2024-05-20T13:29:47Z INF Checking artifact for valid signature what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:29:47Z DBG Reading artifact what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:29:47Z DBG Parsing PE artifact what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:29:47Z DBG Checking if its an EFI file what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:29:47Z DBG Getting DB certs what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:29:47Z DBG Getting signatures from artifact what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:29:47Z DBG Getting DBX certs what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:29:47Z DBG checking signature subject=ITXAKA what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:29:47Z INF verified subject=ITXAKA what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:29:48Z DBG Conf file /efi/loader/entries/passive.conf has values map[string]string{
  "efi": "/EFI/kairos/active.efi",

@Itxaka
Copy link
Member Author

Itxaka commented May 20, 2024

certs in the machine DB:

root@localhost:~# mokutil --db
[key 1]
SHA1 Fingerprint: a3:d8:33:c9:02:af:6a:a2:65:34:71:14:d5:0c:a4:9c:62:d7:87:b2
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            0a:24:ee:5f:00:53:3e:94:1a:03:18:31:32:c8:2c:63:3e:0b:94:4b
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: CN=ITXAKA
        Validity
            Not Before: Apr 26 08:11:52 2024 GMT
            Not After : Apr 26 08:11:52 2025 GMT
        Subject: CN=ITXAKA
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                Public-Key: (2048 bit)

@Itxaka
Copy link
Member Author

Itxaka commented May 20, 2024

Artifact checks out, signed by my default test key

root@localhost:~# sbverify /efi/EFI/kairos/active.efi --list
signature 1
image signature issuers:
 - /CN=ITXAKA
image signature certificates:
 - subject: /CN=ITXAKA

@Itxaka
Copy link
Member Author

Itxaka commented May 20, 2024

Now testing with an upgrade image signed with a different key (generic one created on the fly during artifact build):

$ sbverify --list EFI/kairos/norole.efi 
signature 1
image signature issuers:
 - /CN=Test
image signature certificates:
 - subject: /CN=Test
   issuer:  /CN=Test

Check fails as expected:

2024-05-20T13:38:40Z INF Copying ttl.sh/kairosupgrade2:latest source to /efi
2024-05-20T13:39:07Z INF Finished copying ttl.sh/kairosupgrade2:latest into /efi
2024-05-20T13:39:07Z INF Checking artifact for valid signature what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:39:07Z DBG Reading artifact what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:39:07Z DBG Parsing PE artifact what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:39:07Z DBG Checking if its an EFI file what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:39:07Z DBG Getting DB certs what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:39:07Z DBG Getting signatures from artifact what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:39:07Z DBG Getting DBX certs what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:39:07Z DBG checking signature subject=ITXAKA what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:39:07Z DBG checking signature subject="Microsoft Corporation UEFI CA 2011" what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:39:07Z DBG checking signature subject="Microsoft Windows Production PCA 2011" what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:39:07Z ERR Checking signature error="not ok"
2024-05-20T13:39:07Z DBG Mounting partition COS_GRUB
1 error occurred:
	* not ok

Need to fix the error message though as its pretty crude LOL

@Itxaka
Copy link
Member Author

Itxaka commented May 20, 2024

New error message a bit more extensive

2024-05-20T13:44:57Z INF Copying ttl.sh/kairosupgrade2:latest source to /efi
2024-05-20T13:45:29Z INF Finished copying ttl.sh/kairosupgrade2:latest into /efi
2024-05-20T13:45:29Z INF Checking artifact for valid signature what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:45:29Z DBG Reading artifact what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:45:29Z DBG Parsing PE artifact what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:45:29Z DBG Checking if its an EFI file what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:45:30Z DBG Getting DB certs what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:45:30Z DBG Getting signatures from artifact what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:45:30Z DBG Getting DBX certs what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:45:30Z DBG checking signature subject=ITXAKA what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:45:30Z DBG checking signature subject="Microsoft Corporation UEFI CA 2011" what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:45:30Z DBG checking signature subject="Microsoft Windows Production PCA 2011" what=/efi/EFI/Kairos/norole.efi
2024-05-20T13:45:30Z ERR Checking signature before upgrading error="could not find a signature in EFIVars DB that matches the upgrade artifact"
2024-05-20T13:45:30Z WRN Upgrade artifact signature does not match, upgrading to this source would result in an unbootable active system
Check the upgrade source and confirm that its signed with a valiod key, that key is in the machine DB and it has not been blacklisted.

Signed-off-by: Itxaka <itxaka@kairos.io>
pkg/uki/upgrade.go Outdated Show resolved Hide resolved
pkg/uki/upgrade.go Outdated Show resolved Hide resolved
Signed-off-by: Itxaka <itxaka@kairos.io>
@Itxaka Itxaka merged commit cfa2c61 into main May 22, 2024
11 of 13 checks passed
@Itxaka Itxaka deleted the verify_upgrade_artifacts branch May 22, 2024 07:49
renovate bot added a commit to kairos-io/provider-kairos that referenced this pull request May 24, 2024
…0.5 (#569)

[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[github.com/kairos-io/kairos-agent/v2](https://togithub.com/kairos-io/kairos-agent)
| `v2.10.4` -> `v2.10.5` |
[![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fkairos-io%2fkairos-agent%2fv2/v2.10.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fkairos-io%2fkairos-agent%2fv2/v2.10.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fkairos-io%2fkairos-agent%2fv2/v2.10.4/v2.10.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fkairos-io%2fkairos-agent%2fv2/v2.10.4/v2.10.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>kairos-io/kairos-agent
(github.com/kairos-io/kairos-agent/v2)</summary>

###
[`v2.10.5`](https://togithub.com/kairos-io/kairos-agent/releases/tag/v2.10.5)

[Compare
Source](https://togithub.com/kairos-io/kairos-agent/compare/v2.10.4...v2.10.5)

#### What's Changed

- Update dependency cypress to v13.9.0 by
[@&#8203;renovate](https://togithub.com/renovate) in
[kairos-io/kairos-agent#327
- Fix reboot/power off message by
[@&#8203;Itxaka](https://togithub.com/Itxaka) in
[kairos-io/kairos-agent#331
- Force go 1.19 for releases by
[@&#8203;mauromorales](https://togithub.com/mauromorales) in
[kairos-io/kairos-agent#333
- Update actions/checkout action to v4 by
[@&#8203;renovate](https://togithub.com/renovate) in
[kairos-io/kairos-agent#290
- Update module github.com/kairos-io/kairos-sdk to v0.1.6 by
[@&#8203;renovate](https://togithub.com/renovate) in
[kairos-io/kairos-agent#326
- Update module golang.org/x/net to v0.25.0 by
[@&#8203;renovate](https://togithub.com/renovate) in
[kairos-io/kairos-agent#328
- Update module golang.org/x/sys to v0.20.0 by
[@&#8203;renovate](https://togithub.com/renovate) in
[kairos-io/kairos-agent#330
- Update github/codeql-action action to v3 by
[@&#8203;renovate](https://togithub.com/renovate) in
[kairos-io/kairos-agent#291
- Update module github.com/google/go-github/v40 to v62 by
[@&#8203;renovate](https://togithub.com/renovate) in
[kairos-io/kairos-agent#335
- Update module github.com/google/go-github/v61 to v62 by
[@&#8203;renovate](https://togithub.com/renovate) in
[kairos-io/kairos-agent#336
- Update module github.com/mudler/yip to v1.7.0 by
[@&#8203;renovate](https://togithub.com/renovate) in
[kairos-io/kairos-agent#334
- Update module golang.org/x/oauth2 to v0.20.0 - autoclosed by
[@&#8203;renovate](https://togithub.com/renovate) in
[kairos-io/kairos-agent#329
- Update module github.com/google/go-github/v40 to v62 by
[@&#8203;renovate](https://togithub.com/renovate) in
[kairos-io/kairos-agent#338
- Update dependency alpinejs to v3.14.0 by
[@&#8203;renovate](https://togithub.com/renovate) in
[kairos-io/kairos-agent#340
- Add method to validate signature of efi file by
[@&#8203;Itxaka](https://togithub.com/Itxaka) in
[kairos-io/kairos-agent#337
- Update dependency cypress to v13.10.0 by
[@&#8203;renovate](https://togithub.com/renovate) in
[kairos-io/kairos-agent#342
- Update module github.com/kairos-io/kcrypt to v0.11.0 by
[@&#8203;renovate](https://togithub.com/renovate) in
[kairos-io/kairos-agent#343
- Update module github.com/rs/zerolog to v1.33.0 by
[@&#8203;renovate](https://togithub.com/renovate) in
[kairos-io/kairos-agent#345
- Update module github.com/google/go-github/v40 to v62 by
[@&#8203;renovate](https://togithub.com/renovate) in
[kairos-io/kairos-agent#346
- Bump kairos-sdk to v0.1.7 by
[@&#8203;jimmykarily](https://togithub.com/jimmykarily) in
[kairos-io/kairos-agent#347
- Move udevadm triggering in kcrypt by
[@&#8203;jimmykarily](https://togithub.com/jimmykarily) in
[kairos-io/kairos-agent#350
- Bump sdk to v0.1.8 by
[@&#8203;mauromorales](https://togithub.com/mauromorales) in
[kairos-io/kairos-agent#349

**Full Changelog**:
kairos-io/kairos-agent@v2.10.4...v2.10.5

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 11pm every weekday,before 7am
every weekday,every weekend" in timezone Europe/Brussels, Automerge - At
any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/kairos-io/provider-kairos).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zNjguMTAiLCJ1cGRhdGVkSW5WZXIiOiIzNy4zNjguMTAiLCJ0YXJnZXRCcmFuY2giOiJtYWluIiwibGFiZWxzIjpbXX0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UKI: Verify images signature before upgrade
3 participants