Add reencryption by frederic-hoerni · Pull Request #569 · canonical/secboot

frederic-hoerni · 2026-09-08T16:35:58Z

This adds the feature of reencryption of active LUKS2 containers.

Main changes:

API functions:
- ReencryptionForActiveVolume
- Initialize
- Resume
- Status
Named tokens now may have 2 keyslots, which happens during reencryption.
Introduction of secboot-tool, that is a tool to test and illustrates high-level functions of secboot (mostly reencryption at the moment).

tlaurion · 2026-09-17T13:35:11Z

Would be awesome if this go cryptsetup api was in its own repository. u-root and other projects would benefit of this as well

frederic-hoerni · 2026-09-23T07:10:34Z

Would be awesome if this go cryptsetup api was in its own repository

@tlaurion, we have no plan for that at the moment.

frederic-hoerni · 2026-09-24T07:13:36Z

It lacks tests about Unmarshalling RecoveryToken and KeyDataToken with:

2 keyslots (nominal)

0 or 3 keyslots (error)

frederic-hoerni · 2026-09-24T07:14:38Z

+		"--resume-only",
+		"--progress-frequency", "1",
+		"--progress-json",
+		"--hotzone-size", "10M",


Please explain how/why this option and size "10M" are chosen.
How much does it impact performance? Does it have any other side effects?

frederic-hoerni · 2026-09-24T07:23:03Z

 }

+// NewOnlineReencryption implements [secboot.StorageContainerBackend.NewOnlineReencryption].
+func (b *storageContainerBackend) NewOnlineReencryption(activeName string) (secboot.Reencryption, error) {


Shouldn't this detect if the underlying cryptsetup supports reencryption?
Eg: DetectCryptsetupFeatures()&FeatureReencrypt

.. and return (nil, nil) if it does not?

Returning (nil, nil) would make an unsupported cryptsetup version look like an unrecognized backend

frederic-hoerni · 2026-09-24T07:55:46Z

+	if len(token.Keyslots()) != 1 {
+		return nil, fmt.Errorf("token has %v keyslot(s)", len(token.Keyslots()))


Support for multiple keyslots is needed, for example for the "plainkey" mechanism.

Error seen after reencryption was interrupted:

./secboot-tool activate $DEVICE crypt02 30303030 -v Error with keyslot "default": invalid key data: cannot read keyslot: cannot obtain reader for "default": token has 2 keyslot(s)

frederic-hoerni · 2026-09-25T15:15:56Z

Please rebase on top of master (and expect conflicts).

ruifm

Looking great!

ruifm · 2026-09-25T15:39:57Z

+func ReencryptInitialize(ctx context.Context, activeName string, unlockKeys [][]byte) error {
+	log.Debugf("ReencryptInitialize: unlockKeys=%v", unlockKeys)
+	sizes := []string{}
+	// Prepare concatenated keys
+	var allKeys []byte
+	for _, unlockKey := range unlockKeys {
+		sizes = append(sizes, strconv.Itoa(len(unlockKey)))
+		allKeys = append(allKeys, unlockKey...)
+	}
+	log.Debugf("ReencryptInitialize: sizes=%v", strings.Join(sizes, ","))
+	log.Debugf("ReencryptInitialize: keys=%v", allKeys)


Won't this write every unlock key to stderr when debug logging is enabled?

ruifm · 2026-09-25T15:49:43Z

+func (r reencryptionImpl) Resume(ctx context.Context, unlockKey []byte) (<-chan secboot.ReencryptionProgressEvent, error) {
+	outChan := make(chan secboot.ReencryptionProgressEvent)
+
+	cmd, stdout, stderr, err := luks2.ReencryptResume(ctx, r.dmActiveName, unlockKey)
+	if err != nil {
+		return nil, err
+	}
+
+	go superviseReencryption(cmd, stdout, stderr, outChan)


Deactivation will select the first matching device. The existing lookup assumes one mapping per container. With this layout, it selects data-hotzone-forward instead of data`.

Maybe we should either change that or pass the active name directly?

ruifm · 2026-09-25T15:50:56Z

+	Status() (*ReencryptionStatus, error)
+	Initialize(ctx context.Context, unlockKeys map[string][]byte) error
+	Resume(ctx context.Context, unlockKey []byte) (<-chan ReencryptionProgressEvent, error)
+}


This should also require ActiveName()

ruifm · 2026-09-25T15:53:21Z

+func (v *mockLuksView) TokenNamesSortedByKeyslotId() ([]string, error) {
+	if v.data == nil {
+		return nil, errors.New("error while getting token names")
+	}
+
+	names := []string{}
+	for name, _ := range v.data.recoveryKeyslots {
+		names = append(names, name)
+	}
+	for name, _ := range v.data.platformKeyslots {
+		names = append(names, name)
+	}
+
+	return names, nil


The mock is returning names in map order but the tests expect a fixed order. This can lead to flaky tests, we should not allow this sort of indeterminism

ruifm · 2026-09-25T15:54:58Z

+			details, err := decodeReencryptionProgressDetails(rawBytes)
+			if err != nil {
+				outChan <- secboot.ReencryptionProgressEvent{Type: secboot.ReencryptionProgressRunning, Error: err}
+			} else {
+				outChan <- secboot.ReencryptionProgressEvent{Type: secboot.ReencryptionProgressRunning, Details: details}
+			}
+		}
+		err := scanner.Err()
+		if err != nil {
+			outChan <- secboot.ReencryptionProgressEvent{Type: secboot.ReencryptionProgressRunning, Error: err}


Progress sends can block after a caller cancels and stops reading. Can this be tested? (just a hypothetical but a risk nonetheless)

It is up to the caller to make sure that they read up to the end.
I will add documentation about it.

ruifm · 2026-09-25T15:58:34Z

+	tokenNames, err := view.TokenNamesSortedByKeyslotId()
+	log.Debugf("Initialize: tokenNames: %v", tokenNames)
+
+	if len(tokenNames) == 0 {
+		return fmt.Errorf("cannot get any token name")


return err here otherwise we are hiding the true error

frederic-hoerni · 2026-09-28T07:54:15Z

+		ks.keyslotId = token.Keyslots()[0]
+	} else {
+		// Two keyslots associated with this token (happens when reencryption is in progress)
+		ks.keyslotId = internal_luks2.AnySlot


Doesn't this break the detection done above?

if ks.keyslotId != internal_luks2.AnySlot { // We already have everything for this keyslot.

reworked and fixed

frederic-hoerni · 2026-09-28T16:06:55Z

+//
+// activeName: name of the active device mapper volume
+// unlockKeys: all unlock keys (passphrases) of the LUK2 container, in the order of the keyslots
+func ReencryptInitialize(ctx context.Context, activeName string, unlockKeys [][]byte) error {


Doesn't this require StorageContainer.OpenRead() and StorageContainer.Activate() to acquire a shared lock, so that they do not get their cached data outdated by ReencryptInitialize()?

done for StorageContainer.OpenRead()
No need identified for StorageContainer.Activate()

frederic-hoerni force-pushed the feature/luks-reencryption branch from d881e43 to 3815939 Compare September 11, 2026 06:37

log: add minimal logging framework

862d3ce

frederic-hoerni force-pushed the feature/luks-reencryption branch from 3815939 to 731117d Compare September 17, 2026 08:54

frederic-hoerni changed the title ~~Add reencryption API (MVP)~~ Add reencryption Sep 17, 2026

frederic-hoerni added 4 commits September 22, 2026 17:41

cmd: add reencrypt and secboot-tool

b7c1481

Makefile: handle <pkg> targets

b0fe0e0

Add README-reencrypt

2f9d5d6

Mark deprecated ActivateVolume*, DeactivateVolume

c4597d4

frederic-hoerni force-pushed the feature/luks-reencryption branch from 0ba1781 to d6ced40 Compare September 22, 2026 16:25

Add reencryption

a95d46c

frederic-hoerni force-pushed the feature/luks-reencryption branch from 81a124a to a95d46c Compare September 23, 2026 08:32

frederic-hoerni marked this pull request as ready for review September 24, 2026 06:34

frederic-hoerni requested a review from valentindavid September 24, 2026 06:34

frederic-hoerni self-assigned this Sep 24, 2026

frederic-hoerni requested a review from pedronis September 24, 2026 06:35

frederic-hoerni commented Sep 24, 2026

View reviewed changes

frederic-hoerni added 3 commits September 25, 2026 13:55

luks2: log full systemd-cryptsetup command line

7d4e1ee

README-reencypt: tidy up and reorganize sections

6d60e40

cmd/secboot-tool: add mechanism 'none'

eef8c67

ruifm reviewed Sep 25, 2026

View reviewed changes

frederic-hoerni commented Sep 28, 2026

View reviewed changes

luks2: fix activate error reencryption with plainkey

a60dad4

frederic-hoerni commented Sep 28, 2026

View reviewed changes

frederic-hoerni added 3 commits September 29, 2026 08:49

fix determinism of mocked func TokenNamesSortedByKeyslotId

e3f4bdd

luks2: acquire a shared lock in OpenRead

402b44d

cmd/secboot-tool: remove unnecessary debug message

5f920db

		if len(token.Keyslots()) != 1 {
		return nil, fmt.Errorf("token has %v keyslot(s)", len(token.Keyslots()))

Conversation

frederic-hoerni commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tlaurion commented Sep 17, 2026

Uh oh!

frederic-hoerni commented Sep 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frederic-hoerni Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frederic-hoerni commented Sep 25, 2026

Uh oh!

ruifm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

frederic-hoerni commented Sep 8, 2026 •

edited

Loading

frederic-hoerni Sep 24, 2026 •

edited

Loading