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

cmd/snap-bootstrap: unlock save partition with protector key #14622

Draft
wants to merge 1 commit into
base: fde-manager-features
Choose a base branch
from

Conversation

valentindavid
Copy link
Contributor

@valentindavid valentindavid commented Oct 15, 2024

Related: SNAPDENG-34064

@valentindavid valentindavid added the FDE Manager Pull requests that target FDE manager branch label Oct 15, 2024
if len(slots) != 0 {
const allowRecovery = false
options := activateVolOpts(allowRecovery)
options.PassphraseTries = 0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be part of activateVolOpts maybe.

sb_plainkey.SetProtectorKeys(key)
defer sb_plainkey.SetProtectorKeys()

if err := sbActivateVolumeWithKeyData(mapperName, encdev, nil, options); err != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the nil we should maybe use a const. This is the auth requester I think.

unlockRes.UnlockMethod = UnlockedWithKey
return unlockRes, nil
}

// UnlockEncryptedVolumeUsingKey unlocks an existing volume using the provided key.
func UnlockEncryptedVolumeUsingKey(disk disks.Disk, name string, key []byte) (UnlockResult, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should see if this is still needed. Maybe we can remove this function.

@@ -129,6 +130,62 @@ func UnlockVolumeUsingSealedKeyIfEncrypted(disk disks.Disk, name string, sealedE
return res, err
}

func UnlockEncryptedVolumeUsingProtectorKey(disk disks.Disk, name string, key []byte) (UnlockResult, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not write doc here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And by the way, this is basically UnlockEncryptedVolumeUsingKey with some modifications.

return unlockRes, fmt.Errorf("cannot list slots in partition save partition: %v", err)
}

if len(slots) != 0 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have just realized this is not the right test at this point. Because we do not yet create "plainkey" tokens. So that means we should just try first activating as a protector key, and if that does not work try it as a key.

However the test that there are no "slots" can be used to skip the try as protector key first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Allowing that will also allow use to decide to switch to token vs files based on the version of the OS during installation. Which could be a way out for the missing features of cryptsetup.

@valentindavid valentindavid force-pushed the valentindavid/plainkey-unlocking branch from 3b9f37f to b80df6d Compare October 18, 2024 11:54
Copy link

codecov bot commented Oct 18, 2024

Codecov Report

Attention: Patch coverage is 5.66038% with 50 lines in your changes missing coverage. Please review.

Project coverage is 78.84%. Comparing base (24018ee) to head (b80df6d).
Report is 2 commits behind head on fde-manager-features.

Files with missing lines Patch % Lines
secboot/secboot_sb.go 0.00% 49 Missing ⚠️
...d/snap-bootstrap/cmd_initramfs_mounts_nosecboot.go 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@                   Coverage Diff                    @@
##           fde-manager-features   #14622      +/-   ##
========================================================
- Coverage                 78.87%   78.84%   -0.03%     
========================================================
  Files                      1092     1092              
  Lines                    147404   147453      +49     
========================================================
- Hits                     116270   116265       -5     
- Misses                    23886    23938      +52     
- Partials                   7248     7250       +2     
Flag Coverage Δ
unittests 78.84% <5.66%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FDE Manager Pull requests that target FDE manager branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants