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

crypt.go: allow adding names to legacy keyslots #349

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

valentindavid
Copy link
Contributor

When reprovisioning with a newer snapd with old disks, we need to be able to convert old keyslots to new ones with names. Otherwise we cannot remove after reprovisioning is done.

@valentindavid valentindavid force-pushed the valentindavid/add-names-to-legacy-keys branch from f4304ad to 74675a7 Compare November 20, 2024 16:26
@valentindavid valentindavid marked this pull request as ready for review November 20, 2024 16:49
@valentindavid valentindavid marked this pull request as draft November 20, 2024 16:50
When reprovisioning with a newer snapd with old disks, we need to be
able to convert old keyslots to new ones with names. Otherwise we
cannot remove after reprovisioning is done.
@valentindavid valentindavid force-pushed the valentindavid/add-names-to-legacy-keys branch from 74675a7 to ae7dab0 Compare December 10, 2024 10:10
@valentindavid valentindavid marked this pull request as ready for review December 10, 2024 10:11
@valentindavid
Copy link
Contributor Author

This is used in canonical/snapd#14471

Copy link
Collaborator

@pedronis pedronis left a comment

Choose a reason for hiding this comment

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

did a quick pass

crypt.go Outdated
// KeyslotAlreadyHasAName may be returned bv
// NameLegacyLUKS2ContainerKey when trying to create a token for a
// keyslot that already used by a token.
var KeyslotAlreadyHasAName = errors.New("keyslot already has a name")
Copy link
Collaborator

Choose a reason for hiding this comment

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

this needs to end in Err to be consistent with golang naming conventions

crypt.go Outdated
@@ -934,3 +934,60 @@ func RenameLUKS2ContainerKey(devicePath, oldName, newName string) error {

return nil
}

// KeyslotAlreadyHasAName may be returned bv
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/bv/by/

Copy link
Collaborator

@pedronis pedronis left a comment

Choose a reason for hiding this comment

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

thanks

Copy link
Collaborator

@chrisccoulson chrisccoulson left a comment

Choose a reason for hiding this comment

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

LGTM

@chrisccoulson
Copy link
Collaborator

Oh, I've approved this but just realized that the tests fail because the code doesn't build:

Error: ./crypt.go:964:13: undefined: KeyslotAlreadyHasAName
Error: Process completed with exit code 2.

https://github.com/canonical/secboot/actions/runs/12768290685/job/35588356655?pr=349#step:5:51

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.

3 participants