fix: properly support the PXE and ISO machines in the secure tokens flow #974

Unix4ever · 2025-02-27T16:50:48Z

The unique token flow was reworked to support machines running from PXE and ISO.

As they do not support META persistence, Omni doesn't enforce secure tokens for them.
But to distinguish machines and make the UUID conflict resolution to work, Omni now calculates the node fingerprints out of the mac addresses of all physical interfaces on the node.

So now each unique token consists of two parts:

fingerprint.
a random string.

Omni detects Talos installation on the machine in the following way:

check if the pending machine status exists and it detected the system disk.
overwrite the previous check if the existing link was labeled with the Talos being installed.
lastly if the MachineStatus exists, overwrite all checks with the installed label from it (ensures bare-metal provider workflow which goes to installed to not installed and PXE booted).

Then when a machine joins Omni with some token, Omni checks if the random part is equal. If it is equal, the machine is immediately accepted.

If the random part is different and the fingerprint matches:

if Talos is installed - reject the machine and log the warning in the logs.
if Talos is not installed - replace the existing link with the new one (only if the request has a valid join token).

Then if nothing matches, the UUID conflict resolution kicks in. Provisioner creates a PendingMachine which is marked with UUID conflict label and PendingMachineStatus controller generates a random UUID for the node.

Fixes: #944

The unique token flow was reworked to support machines running from PXE and ISO. As they do not support META persistence, Omni doesn't enforce secure tokens for them. But to distinguish machines and make the UUID conflict resolution to work, Omni now calculates the node fingerprints out of the mac addresses of all physical interfaces on the node. So now each unique token consists of two parts: - fingerprint. - a random string. Omni detects Talos installation on the machine in the following way: - check if the pending machine status exists and it detected the system disk. - overwrite the previous check if the existing link was labeled with the Talos being installed. - lastly if the `MachineStatus` exists, overwrite all checks with the installed label from it (ensures bare-metal provider workflow which goes to installed to not installed and PXE booted). Then when a machine joins Omni with some token, Omni checks if the random part is equal. If it is equal, the machine is immediately accepted. If the random part is different and the fingerprint matches: - if Talos is installed - reject the machine and log the warning in the logs. - if Talos is not installed - replace the existing link with the new one (only if the request has a valid join token). Then if nothing matches, the UUID conflict resolution kicks in. Provisioner creates a `PendingMachine` which is marked with UUID conflict label and `PendingMachineStatus` controller generates a random UUID for the node. Signed-off-by: Artem Chernyshev <[email protected]>

Unix4ever · 2025-02-28T12:12:45Z

internal/pkg/siderolink/provision.go

+	hasValidNodeUniqueToken   bool
+	nodeUniqueTokensEnabled   bool
+	forceValidNodeUniqueToken bool
+	supportsSecureJoinTokens  bool
 }

 // isAuthorized returns true if the request has valid creds for at least one of the auth flows.
 func (pc *provisionContext) isAuthorized() bool {


Maybe can split to smaller logical parts related to each action we do

Unix4ever · 2025-02-28T12:24:05Z

internal/pkg/siderolink/provision.go

+		}
+	}
+
+	machineStatus, err := safe.ReaderGetByID[*system.ResourceLabels[*omni.MachineStatus]](ctx, h.state, req.NodeUuid)


We can use InfraMachineStatus last wipe time?

Change the infra machine status resource to have the last wiped token value.
Populate it this way Link -> InfraMachine -> InfraMachineStatus.

Unix4ever · 2025-02-28T12:35:42Z

internal/pkg/siderolink/provision.go

+// - has matching fingerprint and Talos is not installed.
+//
+//nolint:gocognit
+func ensureResource[T res](ctx context.Context, logger *zap.Logger,


Try to split it to parts

Unix4ever · 2025-02-28T12:43:38Z

internal/pkg/siderolink/provision.go

+	if linkToken.IsSameFingerprint(provisionContext.requestNodeUniqueToken) {
+		logger.Warn("machine connection rejected: the machine has the correct fingerprint, but the random token part doesn't match")
+
+		return nil, status.Error(codes.PermissionDenied, "unauthorized")
+	}


Can move this part inside the ensureResource method probably.

Unix4ever · 2025-02-28T12:53:45Z

internal/backend/runtime/omni/controllers/omni/pending_machine_status.go

+			continue
+		}
+
+		macAddresses = append(macAddresses, string(link.TypedSpec().PermanentAddr))


can be empty, if not set use HardwareAddr.

DmitriyMV · 2025-02-28T12:55:05Z

internal/backend/runtime/omni/controllers/omni/pending_machine_status.go

+	macAddresses := make([]string, 0, links.Len())
+
+	for link := range links.All() {
+		if !link.TypedSpec().Physical() {
+			continue
+		}
+
+		macAddresses = append(macAddresses, string(link.TypedSpec().PermanentAddr))
+	}
+
+	slices.Sort(macAddresses)
+
+	hash := sha256.New()
+
+	for _, addr := range macAddresses {
+		hash.Write([]byte(addr))
+	}


Suggested change

macAddresses := make([]string, 0, links.Len())

for link := range links.All() {

if !link.TypedSpec().Physical() {

continue

}

macAddresses = append(macAddresses, string(link.TypedSpec().PermanentAddr))

}

slices.Sort(macAddresses)

hash := sha256.New()

for _, addr := range macAddresses {

hash.Write([]byte(addr))

}

macAddresses := make([][]byte, 0, links.Len())

for link := range links.All() {

if !link.TypedSpec().Physical() {

continue

}

macAddresses = append(macAddresses, link.TypedSpec().PermanentAddr)

}

slices.SortFunc(macAddresses. bytes.Compare)

hash := sha256.New()

for _, addr := range macAddresses {

hash.Write(addr)

}

Unix4ever · 2025-02-28T13:00:14Z

internal/pkg/siderolink/provision.go

 			pointer.SafeDeref(req.TalosVersion),
 		)
 	}

-	return h.updateLink(ctx, provisionContext)
+	return h.provision(ctx, provisionContext)


Sanitize errors returned here.

DmitriyMV · 2025-02-28T13:06:00Z

internal/backend/runtime/omni/controllers/helpers/helpers.go

@@ -198,7 +199,7 @@ func HandleInput[T generic.ResourceWithRD, S generic.ResourceWithRD](ctx context

 // GetTalosClient for the machine id.
 // Automatically pick secure or insecure client.
-func GetTalosClient[T generic.ResourceWithRD](ctx context.Context, r controller.Reader, address string, machine T) (*client.Client, error) {
+func GetTalosClient[T generic.ResourceWithRD](ctx context.Context, r controller.Reader, address string, machineResource T) (*client.Client, error) {


Suggested change

func GetTalosClient[T generic.ResourceWithRD](ctx context.Context, r controller.Reader, address string, machineResource T) (*client.Client, error) {

func GetTalosClient[T interface {

*V

generic.ResourceWithRD

}, V any](ctx context.Context, r controller.Reader, address string, machineResource T) (*client.Client, error) {

DmitriyMV · 2025-02-28T13:06:14Z

internal/backend/runtime/omni/controllers/helpers/helpers.go

@@ -212,11 +213,20 @@ func GetTalosClient[T generic.ResourceWithRD](ctx context.Context, r controller.
 			)...)
 	}

-	if reflect.ValueOf(machine).IsNil() {
+	if reflect.ValueOf(machineResource).IsNil() {


Suggested change

if reflect.ValueOf(machineResource).IsNil() {

if machineResource == nil {

talos-bot added the status/ok-to-test label Feb 27, 2025

Unix4ever added the integration/e2e Triggers all e2e tests for Omni label Feb 27, 2025

Unix4ever force-pushed the secure-tokens-pxe-iso branch from ca8362a to f77cd97 Compare February 28, 2025 09:21

Unix4ever commented Feb 28, 2025

View reviewed changes

DmitriyMV reviewed Feb 28, 2025

View reviewed changes

Unix4ever commented Feb 28, 2025

View reviewed changes

DmitriyMV reviewed Feb 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: properly support the PXE and ISO machines in the secure tokens flow #974

fix: properly support the PXE and ISO machines in the secure tokens flow #974

Unix4ever commented Feb 27, 2025

Unix4ever Feb 28, 2025

Unix4ever Feb 28, 2025

Unix4ever Feb 28, 2025

Unix4ever Feb 28, 2025

Unix4ever Feb 28, 2025

DmitriyMV Feb 28, 2025

Unix4ever Feb 28, 2025

DmitriyMV Feb 28, 2025

DmitriyMV Feb 28, 2025

	if reflect.ValueOf(machineResource).IsNil() {
	if machineResource == nil {

fix: properly support the PXE and ISO machines in the secure tokens flow #974

Are you sure you want to change the base?

fix: properly support the PXE and ISO machines in the secure tokens flow #974

Conversation

Unix4ever commented Feb 27, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment