diff --git a/cmd/machine-config-controller/start.go b/cmd/machine-config-controller/start.go index a7fc040288..a651614ad8 100644 --- a/cmd/machine-config-controller/start.go +++ b/cmd/machine-config-controller/start.go @@ -143,6 +143,7 @@ func runStartCmd(_ *cobra.Command, _ []string) { ctrlctx.InformerFactory.Machineconfiguration().V1alpha1().InternalReleaseImages(), ctrlctx.InformerFactory.Machineconfiguration().V1().ControllerConfigs(), ctrlctx.InformerFactory.Machineconfiguration().V1().MachineConfigs(), + ctrlctx.InformerFactory.Machineconfiguration().V1().MachineConfigPools(), ctrlctx.ConfigInformerFactory.Config().V1().ClusterVersions(), ctrlctx.KubeInformerFactory.Core().V1().Secrets(), ctrlctx.ClientBuilder.KubeClientOrDie("internalreleaseimage-controller"), diff --git a/docs/IRIAuthCredentialRotation.md b/docs/IRIAuthCredentialRotation.md new file mode 100644 index 0000000000..93823014f9 --- /dev/null +++ b/docs/IRIAuthCredentialRotation.md @@ -0,0 +1,218 @@ +# IRI Registry Authentication Credential Rotation + +This document describes how the Internal Release Image (IRI) registry +authentication credentials are managed and rotated by the +`InternalReleaseImageController`. + +## Background + +The IRI registry runs on each control plane node behind the `api-int` VIP +(port 22625). Both master and worker nodes pull images from it using +credentials stored in the global pull secret (`openshift-config/pull-secret`). +Because `api-int` load-balances across **all** master nodes, updating +credentials naively causes intermittent authentication failures: a request +may reach a node that still has the old htpasswd while the pull secret already +contains the new password, or vice versa. + +The distribution registry's htpasswd implementation stores entries in a +`map[string][]byte`, so duplicate usernames overwrite each other (last wins). +This means we cannot simply list the same username twice with different +passwords; instead, dual credentials must use **different usernames**. + +## Desired-vs-Current Pattern + +The controller follows the same desired-vs-current reconciliation pattern used +elsewhere in the MCO (e.g., rendered MachineConfig vs node annotations): + +| Secret | Role | +|---|---| +| `openshift-machine-config-operator/internal-release-image-registry-auth` (auth secret) | Holds the **desired** password in `data.password` and the htpasswd file in `data.htpasswd`. | +| `openshift-config/pull-secret` (global pull secret) | Holds the **current** active credentials (as deployed on nodes via rendered MachineConfig). | + +The controller reads the **deployed** state from the rendered MachineConfig's +ignition content (`/var/lib/kubelet/config.json`), not from the pull secret API +object, to accurately determine what is actually on nodes. + +## Generation-Numbered Usernames + +To support dual htpasswd entries during rotation, usernames carry a generation +counter: + +- `openshift` — generation 0 (initial) +- `openshift1` — generation 1 +- `openshift2` — generation 2 +- ... + +Each rotation increments the generation, producing a new username that coexists +with the previous one in the htpasswd file. + +## Rotation Phases + +When the auth secret's desired password differs from the deployed password, the +controller performs a three-phase rotation: + +### Phase 1: Deploy Dual Htpasswd + +The controller generates an htpasswd file containing **two entries**: one for +the currently deployed credentials (old username + old password) and one for +the new credentials (next-generation username + new password). It writes this +dual htpasswd to `data.htpasswd` in the auth secret. + +The renderer reads the htpasswd from the auth secret, so the next MachineConfig +render includes both credentials. As the MachineConfigPool rolls out, each node +receives the dual htpasswd. During this rollout, both old and new passwords are +accepted regardless of which node handles the request. + +### Phase 2: Update Pull Secret + +Once **all** MachineConfigPools report that every machine is updated +(`UpdatedMachineCount == MachineCount == ReadyMachineCount`), the controller +updates the global pull secret with the new username and password. This is safe +because all nodes now accept both passwords. + +The pull secret update triggers the template controller to re-render, creating +a new rendered MachineConfig. Another MCP rollout propagates the updated pull +secret to all nodes. + +### Phase 3: Clean Up to Single Htpasswd + +On subsequent syncs, the controller reads the deployed pull secret from the +rendered MachineConfig. Once the deployed password matches the desired password +(i.e., the Phase 2 rollout is complete), the controller cleans up the dual +htpasswd to a single entry containing only the new credentials. This removes +the old password, eliminating the security risk of leaving stale credentials. + +## State Diagram + +``` + ┌────────────────────────┐ + │ No IRI entry in pull │ + │ secret (first time) │ + └──────────┬─────────────┘ + │ merge directly + v + ┌────────────────────────┐ + │ Deployed == Desired │◄──────────────────┐ + │ (steady state) │ │ + └──────────┬─────────────┘ │ + │ auth secret password changes │ + v │ + ┌────────────────────────┐ │ + │ Phase 1: Write dual │ │ + │ htpasswd to auth secret│ │ + └──────────┬─────────────┘ │ + │ wait for MCP rollout │ + v │ + ┌────────────────────────┐ │ + │ Phase 2: Update pull │ │ + │ secret with new creds │ │ + └──────────┬─────────────┘ │ + │ wait for MCP rollout │ + v │ + ┌────────────────────────┐ │ + │ Phase 3: Clean up dual │ │ + │ htpasswd → single entry│───────────────────┘ + └────────────────────────┘ +``` + +## Triggering a Credential Rotation + +To rotate IRI registry credentials, update the `password` field in the auth +secret: + +```bash +oc -n openshift-machine-config-operator patch secret internal-release-image-registry-auth \ + --type merge -p '{"data":{"password":"'$(echo -n "new-password" | base64)'"}}' +``` + +The controller detects the mismatch between the desired password (auth secret) +and the deployed password (rendered MachineConfig) and automatically performs +the three-phase rotation. No manual intervention is required after updating +the secret. + +**Important:** Only modify the `password` field. The `htpasswd` field is +controller-managed. If the htpasswd is manually edited, the controller will +detect the invalid hash via `bcrypt.CompareHashAndPassword` and regenerate it. + +## User Edits During Rotation + +The `password` and `htpasswd` fields in the auth secret can be changed at any +point during rotation. The controller handles all cases safely: + +- **Password changes** are detected via `HtpasswdHasValidEntry`, which uses + `bcrypt.CompareHashAndPassword` to verify the htpasswd hash matches the + desired password. If it doesn't match (password changed since the dual + htpasswd was written), the dual htpasswd is regenerated. Clients are never + affected because the pull secret always contains the old deployed credentials, + which are preserved as one of the two htpasswd entries in every regeneration. + +- **Htpasswd edits** are detected by the same `HtpasswdHasValidEntry` check. + Invalid or tampered hashes cause regeneration. The controller is the sole + owner of the htpasswd field. + +- **Cross-pool inconsistency** (e.g., password changed after the pull secret + was updated but before all pools re-rendered) is detected by + `getDeployedIRICredentials`, which returns `ErrInconsistentIRICredentials`. + The controller waits for convergence before making any rotation decisions. + +### Per-Phase Analysis + +**Phase 1** (dual htpasswd written, MCP rolling out): Password or htpasswd +changes cause `HtpasswdHasValidEntry` to fail. Dual htpasswd is regenerated +with the latest desired password. New MCP rollout starts. Pull secret is +unchanged. Safe. + +**Phase 2** (all MCPs updated, pull secret about to be updated): Password or +htpasswd changes cause dual htpasswd regeneration **before** the pull secret +update code is reached, so the pull secret is never updated with a stale +password. Goes back to Phase 1. Safe. + +**Phase 2b** (pull secret updated, re-render in progress): Different pools may +have different rendered MCs. `getDeployedIRICredentials` returns +`ErrInconsistentIRICredentials`. Controller takes no action and waits for +convergence. Password or htpasswd edits during this window are ignored until +pools converge. After all pools converge, the next sync picks up the correct +deployed state and reconciles. Safe. + +**Phase 3** (deployed matches desired, cleaning up dual htpasswd): Password +changes cause Case 3 to fire instead of Case 2, skipping cleanup and starting +a new rotation. Htpasswd edits are detected by Case 2's +`HtpasswdHasValidEntry` check and repaired to a single valid entry. Safe. + +## Safety Guarantees + +- **No authentication deadlock**: Dual htpasswd ensures both old and new + passwords are accepted during the entire rollout window. The pull secret + (what clients use) always contains the old credentials, which are always + present in every version of the htpasswd. +- **No stale credentials**: The dual htpasswd is cleaned up to a single entry + once the rotation is fully deployed, removing the old password. +- **Accurate deployed state**: The controller reads credentials from the + rendered MachineConfig ignition content, not the API object, avoiding race + conditions between API updates and node rollouts. +- **Cross-pool consistency**: `getDeployedIRICredentials` checks all pools' + rendered MachineConfigs and returns `ErrInconsistentIRICredentials` if + different pools have different IRI credentials (e.g., one pool's rendered + MC was re-rendered with the updated pull secret while another still has + the old one). When this occurs, the controller treats it as a rollout in + progress and waits for convergence before making any rotation decisions. +- **All pools checked**: Both master and worker MachineConfigPools must + complete rollout before advancing phases, because workers are also IRI + registry clients via `api-int`. +- **Htpasswd self-healing**: If the htpasswd is manually edited, the + controller detects the invalid hash via `bcrypt.CompareHashAndPassword` + and regenerates it to match the expected credentials. +- **Mid-rotation resilience**: Changing the desired password during an active + rotation regenerates the dual htpasswd without causing deadlock. The + `HtpasswdHasValidEntry` check uses bcrypt hash comparison (not string + equality) to detect when the desired password has changed since the dual + htpasswd was written. + +## Related Files + +- `pkg/controller/internalreleaseimage/internalreleaseimage_controller.go` — + `reconcileAuthCredentials`, `getDeployedIRICredentials`, `areAllPoolsUpdated` +- `pkg/controller/internalreleaseimage/pullsecret.go` — + `MergeIRIAuthIntoPullSecret`, `ExtractIRICredentialsFromPullSecret`, + `NextIRIUsername`, `GenerateHtpasswdEntry`, `GenerateDualHtpasswd`, + `HtpasswdHasValidEntry` diff --git a/pkg/controller/bootstrap/bootstrap.go b/pkg/controller/bootstrap/bootstrap.go index 504ce97bb0..c2b3b716bf 100644 --- a/pkg/controller/bootstrap/bootstrap.go +++ b/pkg/controller/bootstrap/bootstrap.go @@ -115,6 +115,7 @@ func (b *Bootstrap) Run(destDir string) error { imageStream *imagev1.ImageStream iri *mcfgv1alpha1.InternalReleaseImage iriTLSCert *corev1.Secret + iriAuthSecret *corev1.Secret ) for _, info := range infos { if info.IsDir() { @@ -199,6 +200,9 @@ func (b *Bootstrap) Run(destDir string) error { if obj.GetName() == ctrlcommon.InternalReleaseImageTLSSecretName { iriTLSCert = obj } + if obj.GetName() == ctrlcommon.InternalReleaseImageAuthSecretName { + iriAuthSecret = obj + } default: klog.Infof("skipping %q [%d] manifest because of unhandled %T", file.Name(), idx+1, obji) } @@ -243,6 +247,24 @@ func (b *Bootstrap) Run(destDir string) error { } pullSecretBytes := pullSecret.Data[corev1.DockerConfigJsonKey] + + // If IRI auth is enabled, merge the IRI registry credentials into the pull + // secret before rendering template MCs. This ensures the bootstrap-rendered + // MCs use the same pull secret content as the in-cluster IRI controller + // will produce, avoiding a rendered MachineConfig hash mismatch. + if fgHandler != nil && fgHandler.Enabled(features.FeatureGateNoRegistryClusterInstall) { + if iriAuthSecret != nil && cconfig.Spec.DNS != nil { + password := string(iriAuthSecret.Data["password"]) + merged, mergeErr := internalreleaseimage.MergeIRIAuthIntoPullSecret(pullSecretBytes, password, cconfig.Spec.DNS.Spec.BaseDomain) + if mergeErr != nil { + klog.Warningf("Failed to merge IRI auth into pull secret during bootstrap: %v", mergeErr) + } else { + pullSecretBytes = merged + klog.Infof("Merged IRI registry auth into pull secret for bootstrap rendering") + } + } + } + iconfigs, err := template.RunBootstrap(b.templatesDir, cconfig, pullSecretBytes, apiServer) if err != nil { return err @@ -305,7 +327,7 @@ func (b *Bootstrap) Run(destDir string) error { if fgHandler != nil && fgHandler.Enabled(features.FeatureGateNoRegistryClusterInstall) { if iri != nil { - iriConfigs, err := internalreleaseimage.RunInternalReleaseImageBootstrap(iri, iriTLSCert, cconfig) + iriConfigs, err := internalreleaseimage.RunInternalReleaseImageBootstrap(iri, iriTLSCert, iriAuthSecret, cconfig) if err != nil { return err } diff --git a/pkg/controller/common/constants.go b/pkg/controller/common/constants.go index 31ba8b1040..cf27b799be 100644 --- a/pkg/controller/common/constants.go +++ b/pkg/controller/common/constants.go @@ -72,6 +72,9 @@ const ( // InternalReleaseImageTLSSecretName is the name of the secret manifest containing the InternalReleaseImage TLS certificate. InternalReleaseImageTLSSecretName = "internal-release-image-tls" + // InternalReleaseImageAuthSecretName is the name of the secret containing IRI registry htpasswd auth credentials. + InternalReleaseImageAuthSecretName = "internal-release-image-registry-auth" + // APIServerInstanceName is a singleton name for APIServer configuration APIServerBootstrapFileLocation = "/etc/mcs/bootstrap/api-server/api-server.yaml" diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap.go b/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap.go index 6d2a243eab..64b029f6db 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap.go @@ -8,11 +8,11 @@ import ( ) // RunInternalReleaseImageBootstrap generates the MachineConfig objects for InternalReleaseImage that would have been generated by syncInternalReleaseImage. -func RunInternalReleaseImageBootstrap(iri *mcfgv1alpha1.InternalReleaseImage, iriSecret *corev1.Secret, cconfig *mcfgv1.ControllerConfig) ([]*mcfgv1.MachineConfig, error) { +func RunInternalReleaseImageBootstrap(iri *mcfgv1alpha1.InternalReleaseImage, iriSecret *corev1.Secret, iriAuthSecret *corev1.Secret, cconfig *mcfgv1.ControllerConfig) ([]*mcfgv1.MachineConfig, error) { configs := []*mcfgv1.MachineConfig{} for _, role := range SupportedRoles { - r := NewRendererByRole(role, iri, iriSecret, cconfig) + r := NewRendererByRole(role, iri, iriSecret, iriAuthSecret, cconfig) mc, err := r.CreateEmptyMachineConfig() if err != nil { return nil, err diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap_test.go b/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap_test.go index be62296f9f..6ced16f731 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap_test.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap_test.go @@ -8,7 +8,15 @@ import ( ) func TestRunInternalReleaseImageBootstrap(t *testing.T) { - configs, err := RunInternalReleaseImageBootstrap(&mcfgv1alpha1.InternalReleaseImage{}, iriCertSecret().obj, cconfig().obj) + configs, err := RunInternalReleaseImageBootstrap(&mcfgv1alpha1.InternalReleaseImage{}, iriCertSecret().obj, nil, cconfig().obj) assert.NoError(t, err) verifyAllInternalReleaseImageMachineConfigs(t, configs) } + +func TestRunInternalReleaseImageBootstrapWithAuth(t *testing.T) { + configs, err := RunInternalReleaseImageBootstrap(&mcfgv1alpha1.InternalReleaseImage{}, iriCertSecret().obj, iriAuthSecret().obj, cconfig().obj) + assert.NoError(t, err) + assert.Len(t, configs, 2) + verifyInternalReleaseMasterMachineConfigWithAuth(t, configs[0]) + verifyInternalReleaseWorkerMachineConfig(t, configs[1]) +} diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go b/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go index 23a53d671c..9c295869c8 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go @@ -4,10 +4,12 @@ import ( "context" "fmt" "reflect" + "strings" "time" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/labels" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" clientset "k8s.io/client-go/kubernetes" @@ -40,6 +42,12 @@ const ( maxRetries = 15 ) +// ErrInconsistentIRICredentials is returned by getDeployedIRICredentials when +// different MachineConfigPools have different IRI credentials in their rendered +// MachineConfigs. This indicates a rollout is in progress and the rotation +// state machine should not advance. +var ErrInconsistentIRICredentials = fmt.Errorf("IRI credentials differ across MachineConfigPool rendered configs") + var ( // controllerKind contains the schema.GroupVersionKind for this controller type. controllerKind = mcfgv1alpha1.SchemeGroupVersion.WithKind("InternalReleaseImage") @@ -54,6 +62,7 @@ var ( // Controller defines the InternalReleaseImage controller. type Controller struct { client mcfgclientset.Interface + kubeClient clientset.Interface eventRecorder record.EventRecorder syncHandler func(mcp string) error @@ -68,6 +77,9 @@ type Controller struct { mcLister mcfglistersv1.MachineConfigLister mcListerSynced cache.InformerSynced + mcpLister mcfglistersv1.MachineConfigPoolLister + mcpListerSynced cache.InformerSynced + clusterVersionLister configlistersv1.ClusterVersionLister clusterVersionListerSynced cache.InformerSynced @@ -82,6 +94,7 @@ func New( iriInformer mcfginformersv1alpha1.InternalReleaseImageInformer, ccInformer mcfginformersv1.ControllerConfigInformer, mcInformer mcfginformersv1.MachineConfigInformer, + mcpInformer mcfginformersv1.MachineConfigPoolInformer, clusterVersionInformer configinformersv1.ClusterVersionInformer, secretInformer coreinformersv1.SecretInformer, kubeClient clientset.Interface, @@ -93,6 +106,7 @@ func New( ctrl := &Controller{ client: mcfgClient, + kubeClient: kubeClient, eventRecorder: ctrlcommon.NamespacedEventRecorder(eventBroadcaster.NewRecorder(scheme.Scheme, corev1.EventSource{Component: "machineconfigcontroller-internalreleaseimagecontroller"})), queue: workqueue.NewTypedRateLimitingQueueWithConfig( workqueue.DefaultTypedControllerRateLimiter[string](), @@ -130,6 +144,9 @@ func New( ctrl.mcLister = mcInformer.Lister() ctrl.mcListerSynced = mcInformer.Informer().HasSynced + ctrl.mcpLister = mcpInformer.Lister() + ctrl.mcpListerSynced = mcpInformer.Informer().HasSynced + ctrl.clusterVersionLister = clusterVersionInformer.Lister() ctrl.clusterVersionListerSynced = clusterVersionInformer.Informer().HasSynced @@ -144,7 +161,7 @@ func (ctrl *Controller) Run(workers int, stopCh <-chan struct{}) { defer utilruntime.HandleCrash() defer ctrl.queue.ShutDown() - if !cache.WaitForCacheSync(stopCh, ctrl.iriListerSynced, ctrl.ccListerSynced, ctrl.mcListerSynced, ctrl.clusterVersionListerSynced, ctrl.secretListerSynced) { + if !cache.WaitForCacheSync(stopCh, ctrl.iriListerSynced, ctrl.ccListerSynced, ctrl.mcListerSynced, ctrl.mcpListerSynced, ctrl.clusterVersionListerSynced, ctrl.secretListerSynced) { return } @@ -278,7 +295,8 @@ func (ctrl *Controller) updateSecret(obj, _ interface{}) { secret := obj.(*corev1.Secret) // Skip any event not related to the InternalReleaseImage secrets - if secret.Name != ctrlcommon.InternalReleaseImageTLSSecretName { + if secret.Name != ctrlcommon.InternalReleaseImageTLSSecretName && + secret.Name != ctrlcommon.InternalReleaseImageAuthSecretName { return } @@ -341,8 +359,22 @@ func (ctrl *Controller) syncInternalReleaseImage(key string) error { return fmt.Errorf("could not get Secret %s: %w", ctrlcommon.InternalReleaseImageTLSSecretName, err) } + // Auth secret may not exist during upgrades from non-auth clusters + iriAuthSecret, _ := ctrl.secretLister.Secrets(ctrlcommon.MCONamespace).Get(ctrlcommon.InternalReleaseImageAuthSecretName) + + // Handle credential rotation using the desired-vs-current pattern. + // The auth secret holds the desired password; the pull secret holds the current + // active credentials. When they differ, a multi-phase rotation is performed + // to avoid authentication failures during rolling MachineConfig updates. + // See docs/iri-auth-credential-rotation.md for the full design. + if iriAuthSecret != nil { + if err := ctrl.reconcileAuthCredentials(iriAuthSecret, cconfig); err != nil { + return fmt.Errorf("failed to reconcile auth credentials: %w", err) + } + } + for _, role := range SupportedRoles { - r := NewRendererByRole(role, iri, iriSecret, cconfig) + r := NewRendererByRole(role, iri, iriSecret, iriAuthSecret, cconfig) mc, err := ctrl.mcLister.Get(r.GetMachineConfigName()) isNotFound := errors.IsNotFound(err) @@ -449,6 +481,287 @@ func (ctrl *Controller) addFinalizerToInternalReleaseImage(iri *mcfgv1alpha1.Int return err } +// reconcileAuthCredentials implements the desired-vs-current credential rotation +// pattern. It compares the desired password (from the auth secret) with the +// current password (from the pull secret) and manages a safe multi-phase rotation: +// +// 1. Passwords match: credentials are in sync, ensure htpasswd uses a single entry. +// 2. No IRI entry in pull secret: first-time setup, merge credentials directly. +// 3. Passwords differ: rotation needed — generate dual htpasswd with both old and +// new credentials (using different generation usernames), update the auth secret's +// htpasswd field, and wait for all MachineConfigPools to finish rolling out before +// updating the pull secret with the new credentials. +// +// The "current" state is determined by reading the pull secret from the rendered +// MachineConfig (what's actually deployed on nodes), not from the API object, +// to avoid race conditions during rollout. +// +// See docs/iri-auth-credential-rotation.md for the full design. +func (ctrl *Controller) reconcileAuthCredentials(authSecret *corev1.Secret, cconfig *mcfgv1.ControllerConfig) error { + desiredPassword := string(authSecret.Data["password"]) + if desiredPassword == "" { + return nil + } + + if cconfig.Spec.DNS == nil { + return fmt.Errorf("ControllerConfig DNS is not set") + } + baseDomain := cconfig.Spec.DNS.Spec.BaseDomain + + // Read the deployed pull secret from the rendered MachineConfig to determine + // what credentials are actually on nodes, rather than the API object which + // may not have been rolled out yet. + deployedUsername, deployedPassword, err := ctrl.getDeployedIRICredentials(baseDomain) + if err == ErrInconsistentIRICredentials { + // Different pools have different IRI credentials in their rendered MCs. + // This means a pull secret rollout is in progress. Wait for it to + // complete before making any rotation decisions. + klog.V(4).Infof("IRI credentials inconsistent across pools, waiting for rollout to converge") + return nil + } + if err != nil { + return fmt.Errorf("failed to read deployed IRI credentials: %w", err) + } + + // Case 1: No IRI entry deployed yet — first-time setup + if deployedPassword == "" { + klog.Infof("No IRI credentials deployed, performing first-time merge into pull secret") + return ctrl.mergeIRIAuthIntoPullSecret(cconfig, authSecret) + } + + // Case 2: Deployed password matches desired — credentials are in sync. + if deployedPassword == desiredPassword { + currentHtpasswd := string(authSecret.Data["htpasswd"]) + var needsUpdate bool + + // Clean up dual htpasswd entries left over from a completed rotation. + if strings.Count(currentHtpasswd, "\n") > 1 { + klog.Infof("Cleaning up dual htpasswd after completed rotation (username=%s)", deployedUsername) + needsUpdate = true + } + + // Repair htpasswd if it was manually edited to contain an invalid hash + // (e.g., user directly modified the auth secret's htpasswd field). + if !HtpasswdHasValidEntry(currentHtpasswd, deployedUsername, desiredPassword) { + klog.Infof("Repairing htpasswd: hash does not match desired password (username=%s)", deployedUsername) + needsUpdate = true + } + + if needsUpdate { + singleHtpasswd, err := GenerateHtpasswdEntry(deployedUsername, desiredPassword) + if err != nil { + return fmt.Errorf("failed to generate single htpasswd: %w", err) + } + authSecret.Data["htpasswd"] = []byte(singleHtpasswd) + if _, err := ctrl.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Update( + context.TODO(), authSecret, metav1.UpdateOptions{}); err != nil { + return fmt.Errorf("failed to update auth secret htpasswd: %w", err) + } + } + return nil + } + + // Case 3: Passwords differ — rotation needed + klog.Infof("IRI auth credential rotation detected: deployed password differs from desired password") + + newUsername := NextIRIUsername(deployedUsername) + currentHtpasswd := string(authSecret.Data["htpasswd"]) + + // Check whether the htpasswd already has valid dual entries for both the + // deployed credentials and the desired credentials. We use + // bcrypt.CompareHashAndPassword rather than string comparison because bcrypt + // salts differ on every generation. This also handles mid-rotation password + // changes: if the desired password changed since the dual htpasswd was + // written, the new entry's hash won't match and we regenerate. + // + // Safety: the deployed credentials (oldUser, oldPass) are always preserved + // as one of the two htpasswd entries. The pull secret (what clients use to + // authenticate) always contains these old credentials. So even if the desired + // password changes mid-rotation, clients always authenticate successfully + // against every htpasswd version. + hasDualEntries := HtpasswdHasValidEntry(currentHtpasswd, deployedUsername, deployedPassword) && + HtpasswdHasValidEntry(currentHtpasswd, newUsername, desiredPassword) + + if !hasDualEntries { + // Generate dual htpasswd with both deployed and new credentials so that + // both passwords are accepted during the rolling MachineConfig update. + dualHtpasswd, err := GenerateDualHtpasswd(deployedUsername, deployedPassword, newUsername, desiredPassword) + if err != nil { + return fmt.Errorf("failed to generate dual htpasswd for rotation: %w", err) + } + authSecret.Data["htpasswd"] = []byte(dualHtpasswd) + if _, err := ctrl.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Update( + context.TODO(), authSecret, metav1.UpdateOptions{}); err != nil { + return fmt.Errorf("failed to update auth secret with dual htpasswd: %w", err) + } + klog.Infof("Updated auth secret htpasswd with dual credentials (%s + %s) for rotation", deployedUsername, newUsername) + // Return so the MC render on the next sync picks up the new htpasswd + return nil + } + + // Dual htpasswd is already set — check if ALL pools have rolled out the + // dual htpasswd MachineConfig. Only then is it safe to update the pull secret. + allUpdated, err := ctrl.areAllPoolsUpdated() + if err != nil { + return fmt.Errorf("failed to check MCP status: %w", err) + } + + if !allUpdated { + klog.V(4).Infof("Waiting for MachineConfigPool rollout to complete before advancing credential rotation") + return nil + } + + // All nodes have the dual htpasswd — safe to update the pull secret. + // After the pull secret API object is updated, the template controller will + // re-render and trigger another rollout. On subsequent syncs, the deployed + // pull secret (from the rendered MC) will eventually match the desired password, + // at which point Case 2 handles cleanup. + klog.Infof("MCP rollout complete, updating pull secret with new credentials (username=%s)", newUsername) + pullSecret, err := ctrl.kubeClient.CoreV1().Secrets(ctrlcommon.OpenshiftConfigNamespace).Get( + context.TODO(), ctrlcommon.GlobalPullSecretName, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("could not get pull-secret: %w", err) + } + + mergedBytes, err := MergeIRIAuthIntoPullSecretWithUsername( + pullSecret.Data[corev1.DockerConfigJsonKey], newUsername, desiredPassword, baseDomain) + if err != nil { + return fmt.Errorf("failed to merge new credentials into pull secret: %w", err) + } + + pullSecret.Data[corev1.DockerConfigJsonKey] = mergedBytes + if _, err := ctrl.kubeClient.CoreV1().Secrets(ctrlcommon.OpenshiftConfigNamespace).Update( + context.TODO(), pullSecret, metav1.UpdateOptions{}); err != nil { + return fmt.Errorf("failed to update pull secret with rotated credentials: %w", err) + } + klog.Infof("Updated pull secret with rotated IRI credentials (username=%s)", newUsername) + + // The pull secret update will trigger the template controller to re-render, + // creating a new rendered MC. On the next sync, getDeployedIRICredentials will + // still return the OLD credentials until that rollout completes. Once the new + // rendered MC (with updated pull secret) is fully deployed, deployedPassword + // will match desiredPassword, and Case 2 will clean up the dual htpasswd to + // a single entry. + return nil +} + +// getDeployedIRICredentials reads the IRI registry credentials from the pull +// secret that is actually deployed on nodes, by parsing the rendered +// MachineConfig for each pool. This is more accurate than reading the +// openshift-config/pull-secret API object, which may not have been rolled out yet. +// +// All pools must have consistent IRI credentials. If different pools have +// different passwords (e.g., one pool's rendered MC was re-rendered with a new +// pull secret while another pool still has the old one), this returns +// ErrInconsistentIRICredentials. The caller should treat this as "rollout in +// progress" and avoid advancing the rotation state machine. +func (ctrl *Controller) getDeployedIRICredentials(baseDomain string) (username, password string, err error) { + pools, err := ctrl.mcpLister.List(labels.Everything()) + if err != nil { + return "", "", fmt.Errorf("failed to list MachineConfigPools: %w", err) + } + + var foundUsername, foundPassword string + + for _, pool := range pools { + renderedMCName := pool.Status.Configuration.Name + if renderedMCName == "" { + continue + } + + renderedMC, err := ctrl.mcLister.Get(renderedMCName) + if err != nil { + return "", "", fmt.Errorf("failed to get rendered MachineConfig %s: %w", renderedMCName, err) + } + + ignConfig, err := ctrlcommon.ParseAndConvertConfig(renderedMC.Spec.Config.Raw) + if err != nil { + return "", "", fmt.Errorf("failed to parse ignition from rendered MachineConfig %s: %w", renderedMCName, err) + } + + pullSecretData, err := ctrlcommon.GetIgnitionFileDataByPath(&ignConfig, "/var/lib/kubelet/config.json") + if err != nil { + return "", "", fmt.Errorf("failed to extract pull secret from rendered MachineConfig %s: %w", renderedMCName, err) + } + if pullSecretData == nil { + continue + } + + u, p := ExtractIRICredentialsFromPullSecret(pullSecretData, baseDomain) + if p == "" { + continue + } + + if foundPassword == "" { + foundUsername = u + foundPassword = p + } else if foundPassword != p || foundUsername != u { + return "", "", ErrInconsistentIRICredentials + } + } + + return foundUsername, foundPassword, nil +} + +// areAllPoolsUpdated checks whether all MachineConfigPools have finished +// rolling out (all machines are at the desired configuration). Both master +// and worker pools must be checked because worker nodes are also clients +// of the IRI registry via api-int. +func (ctrl *Controller) areAllPoolsUpdated() (bool, error) { + pools, err := ctrl.mcpLister.List(labels.Everything()) + if err != nil { + return false, err + } + + for _, pool := range pools { + if pool.Status.MachineCount != pool.Status.UpdatedMachineCount { + return false, nil + } + if pool.Status.MachineCount != pool.Status.ReadyMachineCount { + return false, nil + } + } + + return true, nil +} + +func (ctrl *Controller) mergeIRIAuthIntoPullSecret(cconfig *mcfgv1.ControllerConfig, authSecret *corev1.Secret) error { + password := string(authSecret.Data["password"]) + if password == "" { + return nil + } + + if cconfig.Spec.DNS == nil { + return fmt.Errorf("ControllerConfig DNS is not set") + } + baseDomain := cconfig.Spec.DNS.Spec.BaseDomain + + // Fetch current pull secret from openshift-config + pullSecret, err := ctrl.kubeClient.CoreV1().Secrets(ctrlcommon.OpenshiftConfigNamespace).Get( + context.TODO(), ctrlcommon.GlobalPullSecretName, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("could not get pull-secret: %w", err) + } + + mergedBytes, err := MergeIRIAuthIntoPullSecret(pullSecret.Data[corev1.DockerConfigJsonKey], password, baseDomain) + if err != nil { + return err + } + + // No change needed + if string(mergedBytes) == string(pullSecret.Data[corev1.DockerConfigJsonKey]) { + return nil + } + + pullSecret.Data[corev1.DockerConfigJsonKey] = mergedBytes + _, err = ctrl.kubeClient.CoreV1().Secrets(ctrlcommon.OpenshiftConfigNamespace).Update( + context.TODO(), pullSecret, metav1.UpdateOptions{}) + if err == nil { + klog.Infof("Updated pull secret with IRI registry auth credentials from secret %s/%s (uid=%s, resourceVersion=%s)", authSecret.Namespace, authSecret.Name, authSecret.UID, authSecret.ResourceVersion) + } + return err +} + func (ctrl *Controller) cascadeDelete(iri *mcfgv1alpha1.InternalReleaseImage) error { mcName := iri.GetFinalizers()[0] err := ctrl.client.MachineconfigurationV1().MachineConfigs().Delete(context.TODO(), mcName, metav1.DeleteOptions{}) diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go b/pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go index 2743142c8a..7806755e69 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go @@ -2,6 +2,8 @@ package internalreleaseimage import ( "context" + "encoding/json" + "strings" "testing" "time" @@ -28,9 +30,10 @@ import ( func TestInternalReleaseImageCreate(t *testing.T) { cases := []struct { - name string - initialObjects func() []runtime.Object - verify func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) + name string + initialObjects func() []runtime.Object + verify func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) + verifyPullSecret func(t *testing.T, f *fixture) }{ { name: "feature inactive", @@ -72,6 +75,34 @@ func TestInternalReleaseImageCreate(t *testing.T) { verifyInternalReleaseWorkerMachineConfig(t, actualWorkerMC) }, }, + { + name: "generate iri machine-config with auth", + initialObjects: objs(iri(), clusterVersion(), cconfig().withDNS("example.com"), iriCertSecret(), iriAuthSecret(), pullSecret()), + verify: func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) { + verifyInternalReleaseMasterMachineConfigWithAuth(t, actualMasterMC) + verifyInternalReleaseWorkerMachineConfig(t, actualWorkerMC) + }, + }, + { + name: "merge iri auth into pull secret", + initialObjects: objs(iri(), clusterVersion(), cconfig().withDNS("example.com"), iriCertSecret(), iriAuthSecret(), pullSecret()), + verify: func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) { + verifyInternalReleaseMasterMachineConfigWithAuth(t, actualMasterMC) + verifyInternalReleaseWorkerMachineConfig(t, actualWorkerMC) + }, + verifyPullSecret: func(t *testing.T, f *fixture) { + ps, err := f.k8sClient.CoreV1().Secrets(ctrlcommon.OpenshiftConfigNamespace).Get( + context.TODO(), ctrlcommon.GlobalPullSecretName, metav1.GetOptions{}) + assert.NoError(t, err) + var dockerConfig map[string]interface{} + err = json.Unmarshal(ps.Data[corev1.DockerConfigJsonKey], &dockerConfig) + assert.NoError(t, err) + auths := dockerConfig["auths"].(map[string]interface{}) + iriEntry, ok := auths["api-int.example.com:22625"] + assert.True(t, ok, "IRI auth entry should be present in pull secret") + assert.NotNil(t, iriEntry) + }, + }, { name: "avoid machine-config drifting", initialObjects: objs( @@ -155,6 +186,9 @@ func TestInternalReleaseImageCreate(t *testing.T) { } tc.verify(t, actualIRI, actualMasterMC, actualWorkerMC) } + if tc.verifyPullSecret != nil { + tc.verifyPullSecret(t, f) + } }) } @@ -171,6 +205,7 @@ type fixture struct { iriLister []*mcfgv1alpha1.InternalReleaseImage ccLister []*mcfgv1.ControllerConfig mcLister []*mcfgv1.MachineConfig + mcpLister []*mcfgv1.MachineConfigPool secretLister []*corev1.Secret clusterVersionLister []*configv1.ClusterVersion @@ -207,6 +242,8 @@ func (f *fixture) setupObjects(objs []runtime.Object) { f.ccLister = append(f.ccLister, o) case *mcfgv1.MachineConfig: f.mcLister = append(f.mcLister, o) + case *mcfgv1.MachineConfigPool: + f.mcpLister = append(f.mcpLister, o) } } } @@ -225,6 +262,7 @@ func (f *fixture) newController() *Controller { i.Machineconfiguration().V1alpha1().InternalReleaseImages(), i.Machineconfiguration().V1().ControllerConfigs(), i.Machineconfiguration().V1().MachineConfigs(), + i.Machineconfiguration().V1().MachineConfigPools(), ci.Config().V1().ClusterVersions(), k.Core().V1().Secrets(), f.k8sClient, @@ -235,6 +273,7 @@ func (f *fixture) newController() *Controller { c.iriListerSynced = alwaysReady c.ccListerSynced = alwaysReady c.mcListerSynced = alwaysReady + c.mcpListerSynced = alwaysReady c.clusterVersionListerSynced = alwaysReady c.secretListerSynced = alwaysReady c.eventRecorder = &record.FakeRecorder{} @@ -258,6 +297,9 @@ func (f *fixture) newController() *Controller { for _, c := range f.mcLister { i.Machineconfiguration().V1().MachineConfigs().Informer().GetIndexer().Add(c) } + for _, c := range f.mcpLister { + i.Machineconfiguration().V1().MachineConfigPools().Informer().GetIndexer().Add(c) + } for _, c := range f.secretLister { k.Core().V1().Secrets().Informer().GetIndexer().Add(c) } @@ -280,3 +322,274 @@ func (f *fixture) runController(key string, expectError bool) { f.t.Error("expected error syncing internalreleaseimage, got nil") } } + +// getAuthSecret fetches the auth secret from the fake k8s client. +func (f *fixture) getAuthSecret() *corev1.Secret { + s, err := f.k8sClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get( + context.TODO(), ctrlcommon.InternalReleaseImageAuthSecretName, metav1.GetOptions{}) + if err != nil { + f.t.Fatalf("failed to get auth secret: %v", err) + } + return s +} + +// getPullSecret fetches the global pull secret from the fake k8s client. +func (f *fixture) getPullSecret() *corev1.Secret { + s, err := f.k8sClient.CoreV1().Secrets(ctrlcommon.OpenshiftConfigNamespace).Get( + context.TODO(), ctrlcommon.GlobalPullSecretName, metav1.GetOptions{}) + if err != nil { + f.t.Fatalf("failed to get pull secret: %v", err) + } + return s +} + +// TestCredentialRotation tests the reconcileAuthCredentials state machine. +// Each test case sets up state for a specific phase of the rotation and runs +// a single sync to verify the controller produces the correct output. +// See docs/IRIAuthCredentialRotation.md for the full rotation design. +func TestCredentialRotation(t *testing.T) { + const baseDomain = "example.com" + const oldPassword = "oldpassword" + const newPassword = "newpassword" + const changedPassword = "changedpassword" + + // Generate valid bcrypt htpasswd entries for test fixtures. + oldHtpasswd, err := GenerateHtpasswdEntry(IRIBaseUsername, oldPassword) + assert.NoError(t, err) + + dualHtpasswd, err := GenerateDualHtpasswd(IRIBaseUsername, oldPassword, "openshift1", newPassword) + assert.NoError(t, err) + + cases := []struct { + name string + initialObjects func() []runtime.Object + verify func(t *testing.T, f *fixture) + }{ + // ===== Phase 1: Dual htpasswd written, MCP rolling out ===== + { + name: "phase 1: rotation triggers dual htpasswd when deployed differs from desired", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(newPassword, oldHtpasswd), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker"), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + authSecret := f.getAuthSecret() + htpasswd := string(authSecret.Data["htpasswd"]) + assert.True(t, HtpasswdHasValidEntry(htpasswd, IRIBaseUsername, oldPassword), + "dual htpasswd should contain old credentials") + assert.True(t, HtpasswdHasValidEntry(htpasswd, "openshift1", newPassword), + "dual htpasswd should contain new credentials") + }, + }, + { + name: "phase 1: mid-rotation password change regenerates dual htpasswd", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(changedPassword, dualHtpasswd), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker"), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + authSecret := f.getAuthSecret() + htpasswd := string(authSecret.Data["htpasswd"]) + assert.True(t, HtpasswdHasValidEntry(htpasswd, IRIBaseUsername, oldPassword), + "dual htpasswd should preserve old credentials") + assert.True(t, HtpasswdHasValidEntry(htpasswd, "openshift1", changedPassword), + "dual htpasswd should be regenerated with changed password") + assert.False(t, HtpasswdHasValidEntry(htpasswd, "openshift1", newPassword), + "dual htpasswd should not contain stale new password") + }, + }, + { + name: "phase 1: user edits htpasswd directly triggers regeneration", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(newPassword, "openshift:invalidhash\n"), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker"), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + authSecret := f.getAuthSecret() + htpasswd := string(authSecret.Data["htpasswd"]) + assert.True(t, HtpasswdHasValidEntry(htpasswd, IRIBaseUsername, oldPassword), + "regenerated dual htpasswd should contain old credentials") + assert.True(t, HtpasswdHasValidEntry(htpasswd, "openshift1", newPassword), + "regenerated dual htpasswd should contain new credentials") + }, + }, + // ===== Phase 2: All MCPs updated, pull secret about to be updated ===== + { + name: "phase 2: all pools updated advances to pull secret update", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(newPassword, dualHtpasswd), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker"), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + ps := f.getPullSecret() + u, p := ExtractIRICredentialsFromPullSecret(ps.Data[corev1.DockerConfigJsonKey], baseDomain) + assert.Equal(t, "openshift1", u, "pull secret should have new username") + assert.Equal(t, newPassword, p, "pull secret should have new password") + }, + }, + { + name: "phase 2: pools not updated blocks pull secret update", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(newPassword, dualHtpasswd), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker").notUpdated(), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + ps := f.getPullSecret() + _, p := ExtractIRICredentialsFromPullSecret(ps.Data[corev1.DockerConfigJsonKey], baseDomain) + assert.Empty(t, p, "pull secret should NOT be updated while pools are rolling out") + }, + }, + { + name: "phase 2: password change before pull secret update prevents stale update", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(changedPassword, dualHtpasswd), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker"), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + ps := f.getPullSecret() + _, p := ExtractIRICredentialsFromPullSecret(ps.Data[corev1.DockerConfigJsonKey], baseDomain) + assert.Empty(t, p, "pull secret should not be updated with stale password") + authSecret := f.getAuthSecret() + htpasswd := string(authSecret.Data["htpasswd"]) + assert.True(t, HtpasswdHasValidEntry(htpasswd, "openshift1", changedPassword), + "htpasswd should be regenerated with changed password") + }, + }, + // ===== Phase 2b: Pull secret updated, re-render in progress ===== + { + name: "phase 2b: inconsistent credentials across pools causes wait", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(newPassword, dualHtpasswd), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, "openshift1", newPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker"), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + authSecret := f.getAuthSecret() + assert.Equal(t, dualHtpasswd, string(authSecret.Data["htpasswd"]), + "htpasswd should not be modified during cross-pool inconsistency") + }, + }, + // ===== Phase 3: Deployed matches desired, cleaning up ===== + { + name: "phase 3: deployed matches desired cleans up dual htpasswd", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(newPassword, dualHtpasswd), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, "openshift1", newPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, "openshift1", newPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker"), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + authSecret := f.getAuthSecret() + htpasswd := string(authSecret.Data["htpasswd"]) + assert.True(t, HtpasswdHasValidEntry(htpasswd, "openshift1", newPassword), + "cleaned up htpasswd should have valid entry for deployed credentials") + lines := strings.Split(strings.TrimSpace(htpasswd), "\n") + assert.Equal(t, 1, len(lines), "cleaned up htpasswd should have single entry") + }, + }, + { + name: "phase 3: user edits htpasswd in steady state triggers repair", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(newPassword, "openshift1:invalidhash\n"), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, "openshift1", newPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, "openshift1", newPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker"), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + authSecret := f.getAuthSecret() + htpasswd := string(authSecret.Data["htpasswd"]) + assert.True(t, HtpasswdHasValidEntry(htpasswd, "openshift1", newPassword), + "repaired htpasswd should have valid hash") + }, + }, + { + name: "phase 3: password change during cleanup starts new rotation", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(changedPassword, dualHtpasswd), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, "openshift1", newPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, "openshift1", newPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker"), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + authSecret := f.getAuthSecret() + htpasswd := string(authSecret.Data["htpasswd"]) + assert.True(t, HtpasswdHasValidEntry(htpasswd, "openshift1", newPassword), + "new rotation should preserve current deployed credentials") + assert.True(t, HtpasswdHasValidEntry(htpasswd, "openshift2", changedPassword), + "new rotation should use next generation username") + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + objs := tc.initialObjects() + f := newFixture(t, objs) + f.run(ctrlcommon.InternalReleaseImageInstanceName) + tc.verify(t, f) + }) + } +} diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_helpers_test.go b/pkg/controller/internalreleaseimage/internalreleaseimage_helpers_test.go index b0cfc7a1e1..254c3d6674 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_helpers_test.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_helpers_test.go @@ -3,6 +3,7 @@ package internalreleaseimage // Test builders and helper methods. import ( + "encoding/json" "fmt" "testing" @@ -35,10 +36,32 @@ func verifyInternalReleaseMasterMachineConfig(t *testing.T, mc *mcfgv1.MachineCo assert.Len(t, ignCfg.Systemd.Units, 1) assert.Contains(t, *ignCfg.Systemd.Units[0].Contents, "docker-registry-image-pullspec") - assert.Len(t, ignCfg.Storage.Files, 4, "Found an unexpected file") + assert.Len(t, ignCfg.Storage.Files, 5, "Found an unexpected file") verifyIgnitionFile(t, &ignCfg, "/etc/pki/ca-trust/source/anchors/iri-root-ca.crt", "iri-root-ca-data") verifyIgnitionFile(t, &ignCfg, "/etc/iri-registry/certs/tls.key", "iri-tls-key") verifyIgnitionFile(t, &ignCfg, "/etc/iri-registry/certs/tls.crt", "iri-tls-crt") + verifyIgnitionFile(t, &ignCfg, "/etc/iri-registry/auth/htpasswd", "") + verifyIgnitionFileContains(t, &ignCfg, "/usr/local/bin/load-registry-image.sh", "docker-registry-image-pullspec") +} + +func verifyInternalReleaseMasterMachineConfigWithAuth(t *testing.T, mc *mcfgv1.MachineConfig) { + assert.Equal(t, masterName(), mc.Name) + assert.Equal(t, ctrlcommon.MachineConfigPoolMaster, mc.Labels[mcfgv1.MachineConfigRoleLabelKey]) + assert.Equal(t, controllerKind.Kind, mc.OwnerReferences[0].Kind) + + ignCfg, err := ctrlcommon.ParseAndConvertConfig(mc.Spec.Config.Raw) + assert.NoError(t, err, mc.Name) + + assert.Len(t, ignCfg.Systemd.Units, 1) + assert.Contains(t, *ignCfg.Systemd.Units[0].Contents, "docker-registry-image-pullspec") + assert.Contains(t, *ignCfg.Systemd.Units[0].Contents, "REGISTRY_AUTH_HTPASSWD_REALM") + assert.Contains(t, *ignCfg.Systemd.Units[0].Contents, "REGISTRY_AUTH_HTPASSWD_PATH") + + assert.Len(t, ignCfg.Storage.Files, 5, "Found an unexpected file") + verifyIgnitionFile(t, &ignCfg, "/etc/pki/ca-trust/source/anchors/iri-root-ca.crt", "iri-root-ca-data") + verifyIgnitionFile(t, &ignCfg, "/etc/iri-registry/certs/tls.key", "iri-tls-key") + verifyIgnitionFile(t, &ignCfg, "/etc/iri-registry/certs/tls.crt", "iri-tls-crt") + verifyIgnitionFile(t, &ignCfg, "/etc/iri-registry/auth/htpasswd", "openshift:$2y$05$testhash") verifyIgnitionFileContains(t, &ignCfg, "/usr/local/bin/load-registry-image.sh", "docker-registry-image-pullspec") } @@ -140,6 +163,15 @@ func cconfig() *controllerConfigBuilder { } } +func (ccb *controllerConfigBuilder) withDNS(baseDomain string) *controllerConfigBuilder { + ccb.obj.Spec.DNS = &configv1.DNS{ + Spec: configv1.DNSSpec{ + BaseDomain: baseDomain, + }, + } + return ccb +} + func (ccb *controllerConfigBuilder) dockerRegistryImage(image string) *controllerConfigBuilder { ccb.obj.Spec.Images[templatectrl.DockerRegistryKey] = image return ccb @@ -224,6 +256,35 @@ func (sb *secretBuilder) build() runtime.Object { return sb.obj } +func pullSecret() *secretBuilder { + return &secretBuilder{ + obj: &corev1.Secret{ + ObjectMeta: v1.ObjectMeta{ + Namespace: ctrlcommon.OpenshiftConfigNamespace, + Name: ctrlcommon.GlobalPullSecretName, + }, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: []byte(`{"auths":{"quay.io":{"auth":"dGVzdDp0ZXN0"}}}`), + }, + }, + } +} + +func iriAuthSecret() *secretBuilder { + return &secretBuilder{ + obj: &corev1.Secret{ + ObjectMeta: v1.ObjectMeta{ + Namespace: ctrlcommon.MCONamespace, + Name: ctrlcommon.InternalReleaseImageAuthSecretName, + }, + Data: map[string][]byte{ + "htpasswd": []byte("openshift:$2y$05$testhash"), + "password": []byte("testpassword"), + }, + }, + } +} + // clusterVersionBuilder simplifies the creation of a Secret resource in the test. type clusterVersionBuilder struct { obj *configv1.ClusterVersion @@ -247,3 +308,97 @@ func clusterVersion() *clusterVersionBuilder { func (cvb *clusterVersionBuilder) build() runtime.Object { return cvb.obj } + +// mcpBuilder simplifies the creation of a MachineConfigPool resource in the test. +type mcpBuilder struct { + obj *mcfgv1.MachineConfigPool +} + +func mcp(name string, renderedMCName string) *mcpBuilder { + return &mcpBuilder{ + obj: &mcfgv1.MachineConfigPool{ + ObjectMeta: v1.ObjectMeta{ + Name: name, + }, + Status: mcfgv1.MachineConfigPoolStatus{ + Configuration: mcfgv1.MachineConfigPoolStatusConfiguration{ + ObjectReference: corev1.ObjectReference{ + Name: renderedMCName, + }, + }, + MachineCount: 3, + UpdatedMachineCount: 3, + ReadyMachineCount: 3, + }, + }, + } +} + +func (mb *mcpBuilder) notUpdated() *mcpBuilder { + mb.obj.Status.UpdatedMachineCount = 1 + mb.obj.Status.ReadyMachineCount = 1 + return mb +} + +func (mb *mcpBuilder) build() runtime.Object { + return mb.obj +} + +// renderedMCBuilder creates a rendered MachineConfig with a pull secret embedded +// in ignition at /var/lib/kubelet/config.json. +type renderedMCBuilder struct { + obj *mcfgv1.MachineConfig + pullSecret string +} + +func renderedMC(name string) *renderedMCBuilder { + return &renderedMCBuilder{ + obj: &mcfgv1.MachineConfig{ + ObjectMeta: v1.ObjectMeta{ + Name: name, + }, + Spec: mcfgv1.MachineConfigSpec{}, + }, + } +} + +func (rb *renderedMCBuilder) withIRICredentials(baseDomain string, username string, password string) *renderedMCBuilder { + rb.pullSecret = pullSecretWithIRIAuthAndUsername(baseDomain, username, password) + return rb +} + +func (rb *renderedMCBuilder) build() runtime.Object { + if rb.pullSecret != "" { + ignCfg := ctrlcommon.NewIgnConfig() + ignCfg.Storage.Files = append(ignCfg.Storage.Files, + ctrlcommon.NewIgnFile("/var/lib/kubelet/config.json", rb.pullSecret)) + raw, _ := json.Marshal(ignCfg) + rb.obj.Spec.Config.Raw = raw + } + return rb.obj +} + +// iriAuthSecretBuilder simplifies the creation of an IRI auth secret with +// customizable password and htpasswd fields. +type iriAuthSecretBuilder struct { + obj *corev1.Secret +} + +func iriAuthSecretCustom(password string, htpasswd string) *iriAuthSecretBuilder { + return &iriAuthSecretBuilder{ + obj: &corev1.Secret{ + ObjectMeta: v1.ObjectMeta{ + Namespace: ctrlcommon.MCONamespace, + Name: ctrlcommon.InternalReleaseImageAuthSecretName, + }, + Data: map[string][]byte{ + "htpasswd": []byte(htpasswd), + "password": []byte(password), + }, + }, + } +} + +func (ab *iriAuthSecretBuilder) build() runtime.Object { + return ab.obj +} diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_renderer.go b/pkg/controller/internalreleaseimage/internalreleaseimage_renderer.go index 36806638fc..2f11d19bd0 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_renderer.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_renderer.go @@ -37,20 +37,22 @@ var ( // the InternalReleaseImage machine config resources. It can also create // a MachineConfig instance when required. type Renderer struct { - role string - iri *mcfgv1alpha1.InternalReleaseImage - iriSecret *corev1.Secret - cconfig *mcfgv1.ControllerConfig + role string + iri *mcfgv1alpha1.InternalReleaseImage + iriSecret *corev1.Secret + iriAuthSecret *corev1.Secret // may be nil + cconfig *mcfgv1.ControllerConfig } // NewRendererByRole creates a new Renderer instance for generating // the machine config for the given role. -func NewRendererByRole(role string, iri *mcfgv1alpha1.InternalReleaseImage, iriSecret *corev1.Secret, cconfig *mcfgv1.ControllerConfig) *Renderer { +func NewRendererByRole(role string, iri *mcfgv1alpha1.InternalReleaseImage, iriSecret *corev1.Secret, iriAuthSecret *corev1.Secret, cconfig *mcfgv1.ControllerConfig) *Renderer { return &Renderer{ - role: role, - iri: iri, - iriSecret: iriSecret, - cconfig: cconfig, + role: role, + iri: iri, + iriSecret: iriSecret, + iriAuthSecret: iriAuthSecret, + cconfig: cconfig, } } @@ -103,6 +105,7 @@ type renderContext struct { IriTLSKey string IriTLSCert string RootCA string + IriHtpasswd string } // newRenderContext creates a new renderContext instance. @@ -115,11 +118,19 @@ func (r *Renderer) newRenderContext() (*renderContext, error) { if err != nil { return nil, err } + iriHtpasswd := "" + if r.iriAuthSecret != nil { + if raw, found := r.iriAuthSecret.Data["htpasswd"]; found { + iriHtpasswd = string(raw) + } + } + return &renderContext{ DockerRegistryImage: r.cconfig.Spec.Images[templatectrl.DockerRegistryKey], IriTLSKey: iriTLSKey, IriTLSCert: iriTLSCert, RootCA: string(r.cconfig.Spec.RootCAData), + IriHtpasswd: iriHtpasswd, }, nil } diff --git a/pkg/controller/internalreleaseimage/pullsecret.go b/pkg/controller/internalreleaseimage/pullsecret.go new file mode 100644 index 0000000000..0f02a6687d --- /dev/null +++ b/pkg/controller/internalreleaseimage/pullsecret.go @@ -0,0 +1,181 @@ +package internalreleaseimage + +import ( + "encoding/base64" + "encoding/json" + "fmt" + "strconv" + "strings" + + "golang.org/x/crypto/bcrypt" +) + +// IRIBaseUsername is the base username for IRI registry authentication (generation 0). +// During credential rotation, a generation counter is appended to form successive +// usernames: "openshift" (generation 0), "openshift1" (generation 1), +// "openshift2" (generation 2), etc. +const IRIBaseUsername = "openshift" + +// MergeIRIAuthIntoPullSecret merges IRI registry authentication credentials +// into a dockerconfigjson pull secret using the default username (generation 0). +// It adds an auth entry for the IRI registry host (api-int.:22625) +// so that kubelet can pull from it. +// +// This must be called during both bootstrap and in-cluster rendering to ensure +// the pull secret content is consistent, avoiding a rendered MachineConfig +// hash mismatch between bootstrap and in-cluster. +func MergeIRIAuthIntoPullSecret(pullSecretRaw []byte, password string, baseDomain string) ([]byte, error) { + return MergeIRIAuthIntoPullSecretWithUsername(pullSecretRaw, IRIBaseUsername, password, baseDomain) +} + +// MergeIRIAuthIntoPullSecretWithUsername merges IRI registry authentication credentials +// into a dockerconfigjson pull secret using the specified username. This is used during +// credential rotation to update the pull secret with the new generation username +// (e.g., "openshift1") after all nodes have been updated with the new htpasswd. +func MergeIRIAuthIntoPullSecretWithUsername(pullSecretRaw []byte, username string, password string, baseDomain string) ([]byte, error) { + if password == "" { + return pullSecretRaw, nil + } + + iriRegistryHost := fmt.Sprintf("api-int.%s:22625", baseDomain) + + var dockerConfig map[string]interface{} + if err := json.Unmarshal(pullSecretRaw, &dockerConfig); err != nil { + return nil, fmt.Errorf("could not parse pull secret: %w", err) + } + + auths, ok := dockerConfig["auths"].(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("pull secret missing 'auths' field") + } + + authValue := base64.StdEncoding.EncodeToString([]byte(username + ":" + password)) + + // Check if IRI entry already exists and is current + if existing, ok := auths[iriRegistryHost].(map[string]interface{}); ok { + if existing["auth"] == authValue { + return pullSecretRaw, nil + } + } + + auths[iriRegistryHost] = map[string]interface{}{ + "auth": authValue, + } + + mergedBytes, err := json.Marshal(dockerConfig) + if err != nil { + return nil, fmt.Errorf("could not marshal merged pull secret: %w", err) + } + + return mergedBytes, nil +} + +// ExtractIRICredentialsFromPullSecret extracts the current IRI registry username +// and password from a dockerconfigjson pull secret. It looks for the auth entry +// keyed by api-int.:22625 and decodes the credentials from the +// base64 "username:password" auth value. +// Returns empty strings if the entry is not found or cannot be parsed. +func ExtractIRICredentialsFromPullSecret(pullSecretRaw []byte, baseDomain string) (username, password string) { + iriRegistryHost := fmt.Sprintf("api-int.%s:22625", baseDomain) + + var dockerConfig map[string]interface{} + if err := json.Unmarshal(pullSecretRaw, &dockerConfig); err != nil { + return "", "" + } + + auths, ok := dockerConfig["auths"].(map[string]interface{}) + if !ok { + return "", "" + } + + entry, ok := auths[iriRegistryHost].(map[string]interface{}) + if !ok { + return "", "" + } + + authValue, ok := entry["auth"].(string) + if !ok { + return "", "" + } + + decoded, err := base64.StdEncoding.DecodeString(authValue) + if err != nil { + return "", "" + } + + // Auth format is "username:password" + parts := strings.SplitN(string(decoded), ":", 2) + if len(parts) != 2 { + return "", "" + } + + return parts[0], parts[1] +} + +// NextIRIUsername computes the next generation username for credential rotation. +// "openshift" (generation 0) becomes "openshift1" (generation 1), +// "openshift1" becomes "openshift2", and so on. +func NextIRIUsername(currentUsername string) string { + if currentUsername == IRIBaseUsername { + return IRIBaseUsername + "1" + } + + suffix := strings.TrimPrefix(currentUsername, IRIBaseUsername) + gen, err := strconv.Atoi(suffix) + if err != nil { + // Unrecognized format, start at generation 1 + return IRIBaseUsername + "1" + } + + return fmt.Sprintf("%s%d", IRIBaseUsername, gen+1) +} + +// HtpasswdHasValidEntry checks whether the htpasswd content contains an entry +// for the given username whose bcrypt hash matches the given password. +func HtpasswdHasValidEntry(htpasswd string, username string, password string) bool { + for _, line := range strings.Split(strings.TrimSpace(htpasswd), "\n") { + parts := strings.SplitN(line, ":", 2) + if len(parts) != 2 { + continue + } + if parts[0] == username { + if bcrypt.CompareHashAndPassword([]byte(parts[1]), []byte(password)) == nil { + return true + } + } + } + return false +} + +// GenerateHtpasswdEntry generates a single htpasswd entry in the format +// "username:bcrypt-hash\n". +func GenerateHtpasswdEntry(username, password string) (string, error) { + hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) + if err != nil { + return "", fmt.Errorf("failed to generate bcrypt hash: %w", err) + } + return fmt.Sprintf("%s:%s\n", username, string(hash)), nil +} + +// GenerateDualHtpasswd generates an htpasswd file with two entries: one for the +// current credentials (from the pull secret) and one for the new credentials +// (from the auth secret). This is used during credential rotation to ensure +// that both old and new passwords are accepted during the MachineConfig rollout. +// +// The distribution registry's htpasswd implementation uses a map keyed by +// username, so the two entries MUST have different usernames. The rotation +// scheme uses generation-numbered usernames: "openshift" (generation 0), +// "openshift1" (generation 1), "openshift2" (generation 2), etc. +func GenerateDualHtpasswd(currentUsername, currentPassword, newUsername, newPassword string) (string, error) { + currentEntry, err := GenerateHtpasswdEntry(currentUsername, currentPassword) + if err != nil { + return "", fmt.Errorf("failed to generate current htpasswd entry: %w", err) + } + + newEntry, err := GenerateHtpasswdEntry(newUsername, newPassword) + if err != nil { + return "", fmt.Errorf("failed to generate new htpasswd entry: %w", err) + } + + return currentEntry + newEntry, nil +} diff --git a/pkg/controller/internalreleaseimage/pullsecret_test.go b/pkg/controller/internalreleaseimage/pullsecret_test.go new file mode 100644 index 0000000000..5e52b01108 --- /dev/null +++ b/pkg/controller/internalreleaseimage/pullsecret_test.go @@ -0,0 +1,276 @@ +package internalreleaseimage + +import ( + "encoding/base64" + "encoding/json" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMergeIRIAuthIntoPullSecret(t *testing.T) { + basePullSecret := `{"auths":{"quay.io":{"auth":"dGVzdDp0ZXN0"}}}` + + tests := []struct { + name string + pullSecret string + password string + baseDomain string + expectChanged bool + expectError bool + verifyAuthHost string + }{ + { + name: "adds IRI auth entry", + pullSecret: basePullSecret, + password: "testpassword", + baseDomain: "example.com", + expectChanged: true, + verifyAuthHost: "api-int.example.com:22625", + }, + { + name: "empty password returns unchanged", + pullSecret: basePullSecret, + password: "", + baseDomain: "example.com", + expectChanged: false, + }, + { + name: "already up-to-date returns unchanged", + pullSecret: pullSecretWithIRIAuth("example.com", "testpassword"), + password: "testpassword", + baseDomain: "example.com", + expectChanged: false, + }, + { + name: "updates stale entry", + pullSecret: pullSecretWithIRIAuth("example.com", "oldpassword"), + password: "newpassword", + baseDomain: "example.com", + expectChanged: true, + verifyAuthHost: "api-int.example.com:22625", + }, + { + name: "invalid JSON returns error", + pullSecret: "not-json", + password: "testpassword", + baseDomain: "example.com", + expectError: true, + }, + { + name: "missing auths field returns error", + pullSecret: `{"registry":"quay.io"}`, + password: "testpassword", + baseDomain: "example.com", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := MergeIRIAuthIntoPullSecret([]byte(tt.pullSecret), tt.password, tt.baseDomain) + + if tt.expectError { + assert.Error(t, err) + return + } + assert.NoError(t, err) + + if !tt.expectChanged { + assert.Equal(t, tt.pullSecret, string(result), "pull secret should not change") + return + } + + // Verify the IRI auth entry was added + var dockerConfig map[string]interface{} + err = json.Unmarshal(result, &dockerConfig) + assert.NoError(t, err) + + auths := dockerConfig["auths"].(map[string]interface{}) + iriEntry, ok := auths[tt.verifyAuthHost].(map[string]interface{}) + assert.True(t, ok, "IRI auth entry should be present") + + expectedAuth := base64.StdEncoding.EncodeToString([]byte("openshift:" + tt.password)) + assert.Equal(t, expectedAuth, iriEntry["auth"]) + + // Verify original entries are preserved + _, hasQuay := auths["quay.io"] + assert.True(t, hasQuay, "original quay.io entry should be preserved") + }) + } +} + +func TestExtractIRICredentialsFromPullSecret(t *testing.T) { + tests := []struct { + name string + pullSecret string + baseDomain string + expectedUsername string + expectedPassword string + }{ + { + name: "extracts credentials from valid entry", + pullSecret: pullSecretWithIRIAuth("example.com", "mypassword"), + baseDomain: "example.com", + expectedUsername: "openshift", + expectedPassword: "mypassword", + }, + { + name: "extracts credentials with generation username", + pullSecret: pullSecretWithIRIAuthAndUsername("example.com", "openshift2", "mypassword"), + baseDomain: "example.com", + expectedUsername: "openshift2", + expectedPassword: "mypassword", + }, + { + name: "returns empty for missing IRI entry", + pullSecret: `{"auths":{"quay.io":{"auth":"dGVzdDp0ZXN0"}}}`, + baseDomain: "example.com", + expectedUsername: "", + expectedPassword: "", + }, + { + name: "returns empty for wrong domain", + pullSecret: pullSecretWithIRIAuth("other.com", "mypassword"), + baseDomain: "example.com", + expectedUsername: "", + expectedPassword: "", + }, + { + name: "returns empty for invalid JSON", + pullSecret: "not-json", + baseDomain: "example.com", + expectedUsername: "", + expectedPassword: "", + }, + { + name: "returns empty for missing auths", + pullSecret: `{"registry":"quay.io"}`, + baseDomain: "example.com", + expectedUsername: "", + expectedPassword: "", + }, + { + name: "returns empty for invalid base64", + pullSecret: `{"auths":{"api-int.example.com:22625":{"auth":"!!!invalid"}}}`, + baseDomain: "example.com", + expectedUsername: "", + expectedPassword: "", + }, + { + name: "returns empty for auth without colon", + pullSecret: `{"auths":{"api-int.example.com:22625":{"auth":"bm9jb2xvbg=="}}}`, + baseDomain: "example.com", + expectedUsername: "", + expectedPassword: "", + }, + { + name: "handles password containing colon", + pullSecret: pullSecretWithIRIAuth("example.com", "pass:word"), + baseDomain: "example.com", + expectedUsername: "openshift", + expectedPassword: "pass:word", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + username, password := ExtractIRICredentialsFromPullSecret([]byte(tt.pullSecret), tt.baseDomain) + assert.Equal(t, tt.expectedUsername, username) + assert.Equal(t, tt.expectedPassword, password) + }) + } +} + +func TestNextIRIUsername(t *testing.T) { + tests := []struct { + current string + expected string + }{ + {"openshift", "openshift1"}, + {"openshift1", "openshift2"}, + {"openshift2", "openshift3"}, + {"openshift99", "openshift100"}, + {"unknown", "openshift1"}, + } + + for _, tt := range tests { + t.Run(tt.current+"->"+tt.expected, func(t *testing.T) { + assert.Equal(t, tt.expected, NextIRIUsername(tt.current)) + }) + } +} + +func TestHtpasswdHasValidEntry(t *testing.T) { + entry, err := GenerateHtpasswdEntry("openshift", "mypassword") + assert.NoError(t, err) + + dual, err := GenerateDualHtpasswd("openshift", "oldpass", "openshift1", "newpass") + assert.NoError(t, err) + + tests := []struct { + name string + htpasswd string + username string + password string + expected bool + }{ + {"valid single entry", entry, "openshift", "mypassword", true}, + {"wrong password", entry, "openshift", "wrongpassword", false}, + {"wrong username", entry, "other", "mypassword", false}, + {"dual entry matches first", dual, "openshift", "oldpass", true}, + {"dual entry matches second", dual, "openshift1", "newpass", true}, + {"dual entry wrong password", dual, "openshift1", "oldpass", false}, + {"empty htpasswd", "", "openshift", "mypassword", false}, + {"malformed line", "nocolon", "openshift", "mypassword", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, HtpasswdHasValidEntry(tt.htpasswd, tt.username, tt.password)) + }) + } +} + +func TestGenerateHtpasswdEntry(t *testing.T) { + entry, err := GenerateHtpasswdEntry("openshift", "testpassword") + assert.NoError(t, err) + assert.True(t, strings.HasPrefix(entry, "openshift:$2"), "should start with username and bcrypt prefix") + assert.True(t, strings.HasSuffix(entry, "\n"), "should end with newline") +} + +func TestGenerateDualHtpasswd(t *testing.T) { + dual, err := GenerateDualHtpasswd("openshift", "oldpass", "openshift1", "newpass") + assert.NoError(t, err) + + lines := strings.Split(strings.TrimSpace(dual), "\n") + assert.Equal(t, 2, len(lines), "should have two htpasswd entries") + assert.True(t, strings.HasPrefix(lines[0], "openshift:$2"), "first entry should be current username") + assert.True(t, strings.HasPrefix(lines[1], "openshift1:$2"), "second entry should be new username") +} + +// pullSecretWithIRIAuth creates a pull secret JSON with an IRI auth entry using +// the default "openshift" username (generation 0). +func pullSecretWithIRIAuth(baseDomain string, password string) string { + return pullSecretWithIRIAuthAndUsername(baseDomain, "openshift", password) +} + +// pullSecretWithIRIAuthAndUsername creates a pull secret JSON with an IRI auth +// entry using the specified username. +func pullSecretWithIRIAuthAndUsername(baseDomain string, username string, password string) string { + authValue := base64.StdEncoding.EncodeToString([]byte(username + ":" + password)) + host := "api-int." + baseDomain + ":22625" + dockerConfig := map[string]interface{}{ + "auths": map[string]interface{}{ + "quay.io": map[string]interface{}{ + "auth": "dGVzdDp0ZXN0", + }, + host: map[string]interface{}{ + "auth": authValue, + }, + }, + } + b, _ := json.Marshal(dockerConfig) + return string(b) +} diff --git a/pkg/controller/internalreleaseimage/templates/master/files/iri-registry-auth-htpasswd.yaml b/pkg/controller/internalreleaseimage/templates/master/files/iri-registry-auth-htpasswd.yaml new file mode 100644 index 0000000000..37d62b1b71 --- /dev/null +++ b/pkg/controller/internalreleaseimage/templates/master/files/iri-registry-auth-htpasswd.yaml @@ -0,0 +1,5 @@ +mode: 0600 +path: "/etc/iri-registry/auth/htpasswd" +contents: + inline: |- +{{indent 4 .IriHtpasswd}} diff --git a/pkg/controller/internalreleaseimage/templates/master/files/usr-local-bin-load-registry-image-sh.yaml b/pkg/controller/internalreleaseimage/templates/master/files/usr-local-bin-load-registry-image-sh.yaml index 2d93e79a1f..49f3e07bcc 100644 --- a/pkg/controller/internalreleaseimage/templates/master/files/usr-local-bin-load-registry-image-sh.yaml +++ b/pkg/controller/internalreleaseimage/templates/master/files/usr-local-bin-load-registry-image-sh.yaml @@ -37,7 +37,7 @@ contents: # As a fallback, let's try to fetch the registry image remotely echo "Trying to pull ${registryImage} from remote registry..." - if podman pull '${registryImage}'; then + if podman pull --authfile /var/lib/kubelet/config.json "${registryImage}"; then echo "Successfully pulled ${registryImage} from remote registry" exit 0 fi diff --git a/pkg/controller/internalreleaseimage/templates/master/units/iri-registry.service.yaml b/pkg/controller/internalreleaseimage/templates/master/units/iri-registry.service.yaml index e3635e2f85..f7b86dcc3c 100644 --- a/pkg/controller/internalreleaseimage/templates/master/units/iri-registry.service.yaml +++ b/pkg/controller/internalreleaseimage/templates/master/units/iri-registry.service.yaml @@ -11,7 +11,13 @@ contents: | Environment=PODMAN_SYSTEMD_UNIT=%n ExecStartPre=/usr/local/bin/load-registry-image.sh ExecStartPre=/bin/rm -f %t/%n.ctr-id - ExecStart=podman run --net host --cidfile=%t/%n.ctr-id --log-driver=journald --replace --name=iri-registry -v ${REGISTRY_DIR}:/var/lib/registry:ro,Z -v /etc/iri-registry/certs:/certs:ro -e REGISTRY_HTTP_ADDR=0.0.0.0:22625 -e REGISTRY_HTTP_TLS_CERTIFICATE=/certs/tls.crt -e REGISTRY_HTTP_TLS_KEY=/certs/tls.key -u 0 --entrypoint=/usr/bin/distribution {{ .DockerRegistryImage }} serve /etc/registry/config.yaml + ExecStart=podman run --net host --cidfile=%t/%n.ctr-id --log-driver=journald --replace --name=iri-registry \ + -v ${REGISTRY_DIR}:/var/lib/registry:ro,Z -v /etc/iri-registry/certs:/certs:ro \ + {{ if .IriHtpasswd }}-v /etc/iri-registry/auth:/etc/registry/auth:ro,Z \ + {{ end }}-e REGISTRY_HTTP_ADDR=0.0.0.0:22625 \ + -e REGISTRY_HTTP_TLS_CERTIFICATE=/certs/tls.crt -e REGISTRY_HTTP_TLS_KEY=/certs/tls.key \ + {{ if .IriHtpasswd }}-e REGISTRY_AUTH_HTPASSWD_REALM=openshift-registry -e REGISTRY_AUTH_HTPASSWD_PATH=/etc/registry/auth/htpasswd \ + {{ end }}-u 0 --entrypoint=/usr/bin/distribution {{ .DockerRegistryImage }} serve /etc/registry/config.yaml ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id diff --git a/test/e2e-iri/iri_test.go b/test/e2e-iri/iri_test.go index b9d85570bd..7f3af9a49a 100644 --- a/test/e2e-iri/iri_test.go +++ b/test/e2e-iri/iri_test.go @@ -4,6 +4,7 @@ import ( "context" "crypto/tls" "crypto/x509" + "encoding/base64" "fmt" "net/http" "os" @@ -20,7 +21,9 @@ import ( configv1 "github.com/openshift/api/config/v1" mcfgv1 "github.com/openshift/api/machineconfiguration/v1" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" + iri "github.com/openshift/machine-config-operator/pkg/controller/internalreleaseimage" "github.com/openshift/machine-config-operator/test/framework" + "github.com/openshift/machine-config-operator/test/helpers" ) func TestIRIResource_Available(t *testing.T) { @@ -179,6 +182,229 @@ func TestIRIController_ShouldPreventDeletionWhenInUse(t *testing.T) { require.NotNil(t, iri, "IRI should not be nil") } +// getBaseDomain retrieves the cluster's base domain from the ControllerConfig. +func getBaseDomain(t *testing.T, cs *framework.ClientSet) string { + ctx := context.Background() + cconfig, err := cs.ControllerConfigs().Get(ctx, ctrlcommon.ControllerConfigName, v1.GetOptions{}) + require.NoError(t, err) + require.NotNil(t, cconfig.Spec.DNS) + require.NotEmpty(t, cconfig.Spec.DNS.Spec.BaseDomain) + return cconfig.Spec.DNS.Spec.BaseDomain +} + +// getIRIRegistryClient returns an HTTP client configured with the cluster's +// root CA for making requests to the IRI registry. +func getIRIRegistryClient(t *testing.T, cs *framework.ClientSet) *http.Client { + ctx := context.Background() + cm, err := cs.ConfigMaps("openshift-machine-config-operator").Get(ctx, "machine-config-server-ca", v1.GetOptions{}) + require.NoError(t, err) + rootCA := []byte(cm.Data["ca-bundle.crt"]) + roots := x509.NewCertPool() + require.True(t, roots.AppendCertsFromPEM(rootCA)) + + return &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + RootCAs: roots, + }, + }, + Timeout: 30 * time.Second, + } +} + +// getAPIIntIP returns the api-int IP address from the infrastructure resource. +func getAPIIntIP(t *testing.T, cs *framework.ClientSet) string { + ctx := context.Background() + infra, err := cs.Infrastructures().Get(ctx, "cluster", v1.GetOptions{}) + require.NoError(t, err) + require.NotEmpty(t, infra.Status.PlatformStatus.BareMetal.APIServerInternalIPs) + return infra.Status.PlatformStatus.BareMetal.APIServerInternalIPs[0] +} + +func TestIRIAuth_UnauthenticatedRequestReturns401(t *testing.T) { + skipIfOpenShiftCI(t) + skipIfNoBaremetal(t) + + cs := framework.NewClientSet("") + + // Verify the auth secret exists (auth is enabled). + ctx := context.Background() + _, err := cs.Secrets(ctrlcommon.MCONamespace).Get(ctx, ctrlcommon.InternalReleaseImageAuthSecretName, v1.GetOptions{}) + if k8serrors.IsNotFound(err) { + t.Skip("IRI auth secret not found, authentication is not enabled") + } + require.NoError(t, err) + + client := getIRIRegistryClient(t, cs) + apiIntIP := getAPIIntIP(t, cs) + url := fmt.Sprintf("https://%s:%d/v2/", apiIntIP, 22625) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + require.NoError(t, err) + + resp, err := client.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusUnauthorized, resp.StatusCode, + "unauthenticated request should return 401 when auth is enabled") +} + +func TestIRIAuth_AuthenticatedRequestSucceeds(t *testing.T) { + skipIfOpenShiftCI(t) + skipIfNoBaremetal(t) + + cs := framework.NewClientSet("") + ctx := context.Background() + + // Get the auth secret to read credentials. + authSecret, err := cs.Secrets(ctrlcommon.MCONamespace).Get(ctx, ctrlcommon.InternalReleaseImageAuthSecretName, v1.GetOptions{}) + if k8serrors.IsNotFound(err) { + t.Skip("IRI auth secret not found, authentication is not enabled") + } + require.NoError(t, err) + + password := string(authSecret.Data["password"]) + require.NotEmpty(t, password) + + // Extract the current username from the pull secret. + baseDomain := getBaseDomain(t, cs) + pullSecret, err := cs.Secrets(ctrlcommon.OpenshiftConfigNamespace).Get(ctx, ctrlcommon.GlobalPullSecretName, v1.GetOptions{}) + require.NoError(t, err) + username, _ := iri.ExtractIRICredentialsFromPullSecret(pullSecret.Data[corev1.DockerConfigJsonKey], baseDomain) + if username == "" { + username = iri.IRIBaseUsername + } + + client := getIRIRegistryClient(t, cs) + apiIntIP := getAPIIntIP(t, cs) + url := fmt.Sprintf("https://%s:%d/v2/", apiIntIP, 22625) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + require.NoError(t, err) + req.Header.Set("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(username+":"+password))) + + resp, err := client.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusOK, resp.StatusCode, + "authenticated request should succeed") + require.Equal(t, "registry/2.0", resp.Header.Get("Docker-Distribution-Api-Version")) +} + +func TestIRIAuth_CredentialRotation(t *testing.T) { + skipIfOpenShiftCI(t) + skipIfNoBaremetal(t) + + cs := framework.NewClientSet("") + ctx := context.Background() + + // Get the auth secret. + authSecret, err := cs.Secrets(ctrlcommon.MCONamespace).Get(ctx, ctrlcommon.InternalReleaseImageAuthSecretName, v1.GetOptions{}) + if k8serrors.IsNotFound(err) { + t.Skip("IRI auth secret not found, authentication is not enabled") + } + require.NoError(t, err) + + originalPassword := string(authSecret.Data["password"]) + require.NotEmpty(t, originalPassword) + + baseDomain := getBaseDomain(t, cs) + + // Record the original pull secret credentials. + pullSecret, err := cs.Secrets(ctrlcommon.OpenshiftConfigNamespace).Get(ctx, ctrlcommon.GlobalPullSecretName, v1.GetOptions{}) + require.NoError(t, err) + originalUsername, originalPullSecretPassword := iri.ExtractIRICredentialsFromPullSecret(pullSecret.Data[corev1.DockerConfigJsonKey], baseDomain) + require.NotEmpty(t, originalPullSecretPassword, "pull secret should have IRI credentials before rotation") + + // Record the current rendered MC names for master and worker pools so we + // can detect when the MCP rollout completes. + masterMCP, err := cs.MachineConfigPools().Get(ctx, "master", v1.GetOptions{}) + require.NoError(t, err) + workerMCP, err := cs.MachineConfigPools().Get(ctx, "worker", v1.GetOptions{}) + require.NoError(t, err) + + t.Logf("Before rotation: username=%s, master rendered=%s, worker rendered=%s", + originalUsername, masterMCP.Status.Configuration.Name, workerMCP.Status.Configuration.Name) + + // Trigger rotation by updating the password in the auth secret. + newPassword := fmt.Sprintf("rotated-%d", time.Now().UnixNano()) + authSecret.Data["password"] = []byte(newPassword) + _, err = cs.Secrets(ctrlcommon.MCONamespace).Update(ctx, authSecret, v1.UpdateOptions{}) + require.NoError(t, err) + t.Logf("Updated auth secret password to trigger rotation") + + // Phase 1: Wait for dual htpasswd to be written to the auth secret. + t.Logf("Waiting for phase 1: dual htpasswd deployment...") + err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { + secret, err := cs.Secrets(ctrlcommon.MCONamespace).Get(ctx, ctrlcommon.InternalReleaseImageAuthSecretName, v1.GetOptions{}) + if err != nil { + return false, err + } + htpasswd := string(secret.Data["htpasswd"]) + // Dual htpasswd has the new password under a new username. + newUsername := iri.NextIRIUsername(originalUsername) + return iri.HtpasswdHasValidEntry(htpasswd, newUsername, newPassword), nil + }) + require.NoError(t, err, "timed out waiting for dual htpasswd (phase 1)") + t.Logf("Phase 1 complete: dual htpasswd written") + + // Phase 2: Wait for MCP rollout to complete and pull secret to be updated. + // The controller waits for all MCPs to be fully updated before writing the + // pull secret, so we poll the pull secret for the new credentials. + t.Logf("Waiting for phase 2: pull secret update...") + expectedUsername := iri.NextIRIUsername(originalUsername) + err = wait.PollUntilContextTimeout(ctx, 10*time.Second, 30*time.Minute, true, func(ctx context.Context) (bool, error) { + ps, err := cs.Secrets(ctrlcommon.OpenshiftConfigNamespace).Get(ctx, ctrlcommon.GlobalPullSecretName, v1.GetOptions{}) + if err != nil { + return false, err + } + u, p := iri.ExtractIRICredentialsFromPullSecret(ps.Data[corev1.DockerConfigJsonKey], baseDomain) + return u == expectedUsername && p == newPassword, nil + }) + require.NoError(t, err, "timed out waiting for pull secret update (phase 2)") + t.Logf("Phase 2 complete: pull secret updated with username=%s", expectedUsername) + + // Wait for both pools to finish rolling out the updated pull secret. + t.Logf("Waiting for MCP rollout after pull secret update...") + helpers.WaitForPoolCompleteAny(t, cs, "master") + helpers.WaitForPoolCompleteAny(t, cs, "worker") + + // Phase 3: Wait for dual htpasswd to be cleaned up to a single entry. + t.Logf("Waiting for phase 3: htpasswd cleanup...") + err = wait.PollUntilContextTimeout(ctx, 10*time.Second, 30*time.Minute, true, func(ctx context.Context) (bool, error) { + secret, err := cs.Secrets(ctrlcommon.MCONamespace).Get(ctx, ctrlcommon.InternalReleaseImageAuthSecretName, v1.GetOptions{}) + if err != nil { + return false, err + } + htpasswd := string(secret.Data["htpasswd"]) + // After cleanup, the htpasswd should only have the new entry. + hasNew := iri.HtpasswdHasValidEntry(htpasswd, expectedUsername, newPassword) + hasOld := iri.HtpasswdHasValidEntry(htpasswd, originalUsername, originalPullSecretPassword) + return hasNew && !hasOld, nil + }) + require.NoError(t, err, "timed out waiting for htpasswd cleanup (phase 3)") + t.Logf("Phase 3 complete: htpasswd cleaned up to single entry") + + // Verify the registry accepts the new credentials. + client := getIRIRegistryClient(t, cs) + apiIntIP := getAPIIntIP(t, cs) + url := fmt.Sprintf("https://%s:%d/v2/", apiIntIP, 22625) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + require.NoError(t, err) + req.Header.Set("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(expectedUsername+":"+newPassword))) + + resp, err := client.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusOK, resp.StatusCode, + "registry should accept the new rotated credentials") + t.Logf("Credential rotation completed successfully") +} + func TestIRIController_ShouldRestoreMachineConfigsWhenModified(t *testing.T) { cases := []struct { name string diff --git a/vendor/golang.org/x/crypto/bcrypt/base64.go b/vendor/golang.org/x/crypto/bcrypt/base64.go new file mode 100644 index 0000000000..fc31160908 --- /dev/null +++ b/vendor/golang.org/x/crypto/bcrypt/base64.go @@ -0,0 +1,35 @@ +// Copyright 2011 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package bcrypt + +import "encoding/base64" + +const alphabet = "./ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789" + +var bcEncoding = base64.NewEncoding(alphabet) + +func base64Encode(src []byte) []byte { + n := bcEncoding.EncodedLen(len(src)) + dst := make([]byte, n) + bcEncoding.Encode(dst, src) + for dst[n-1] == '=' { + n-- + } + return dst[:n] +} + +func base64Decode(src []byte) ([]byte, error) { + numOfEquals := 4 - (len(src) % 4) + for i := 0; i < numOfEquals; i++ { + src = append(src, '=') + } + + dst := make([]byte, bcEncoding.DecodedLen(len(src))) + n, err := bcEncoding.Decode(dst, src) + if err != nil { + return nil, err + } + return dst[:n], nil +} diff --git a/vendor/golang.org/x/crypto/bcrypt/bcrypt.go b/vendor/golang.org/x/crypto/bcrypt/bcrypt.go new file mode 100644 index 0000000000..3e7f8df871 --- /dev/null +++ b/vendor/golang.org/x/crypto/bcrypt/bcrypt.go @@ -0,0 +1,304 @@ +// Copyright 2011 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package bcrypt implements Provos and Mazières's bcrypt adaptive hashing +// algorithm. See http://www.usenix.org/event/usenix99/provos/provos.pdf +package bcrypt + +// The code is a port of Provos and Mazières's C implementation. +import ( + "crypto/rand" + "crypto/subtle" + "errors" + "fmt" + "io" + "strconv" + + "golang.org/x/crypto/blowfish" +) + +const ( + MinCost int = 4 // the minimum allowable cost as passed in to GenerateFromPassword + MaxCost int = 31 // the maximum allowable cost as passed in to GenerateFromPassword + DefaultCost int = 10 // the cost that will actually be set if a cost below MinCost is passed into GenerateFromPassword +) + +// The error returned from CompareHashAndPassword when a password and hash do +// not match. +var ErrMismatchedHashAndPassword = errors.New("crypto/bcrypt: hashedPassword is not the hash of the given password") + +// The error returned from CompareHashAndPassword when a hash is too short to +// be a bcrypt hash. +var ErrHashTooShort = errors.New("crypto/bcrypt: hashedSecret too short to be a bcrypted password") + +// The error returned from CompareHashAndPassword when a hash was created with +// a bcrypt algorithm newer than this implementation. +type HashVersionTooNewError byte + +func (hv HashVersionTooNewError) Error() string { + return fmt.Sprintf("crypto/bcrypt: bcrypt algorithm version '%c' requested is newer than current version '%c'", byte(hv), majorVersion) +} + +// The error returned from CompareHashAndPassword when a hash starts with something other than '$' +type InvalidHashPrefixError byte + +func (ih InvalidHashPrefixError) Error() string { + return fmt.Sprintf("crypto/bcrypt: bcrypt hashes must start with '$', but hashedSecret started with '%c'", byte(ih)) +} + +type InvalidCostError int + +func (ic InvalidCostError) Error() string { + return fmt.Sprintf("crypto/bcrypt: cost %d is outside allowed inclusive range %d..%d", int(ic), MinCost, MaxCost) +} + +const ( + majorVersion = '2' + minorVersion = 'a' + maxSaltSize = 16 + maxCryptedHashSize = 23 + encodedSaltSize = 22 + encodedHashSize = 31 + minHashSize = 59 +) + +// magicCipherData is an IV for the 64 Blowfish encryption calls in +// bcrypt(). It's the string "OrpheanBeholderScryDoubt" in big-endian bytes. +var magicCipherData = []byte{ + 0x4f, 0x72, 0x70, 0x68, + 0x65, 0x61, 0x6e, 0x42, + 0x65, 0x68, 0x6f, 0x6c, + 0x64, 0x65, 0x72, 0x53, + 0x63, 0x72, 0x79, 0x44, + 0x6f, 0x75, 0x62, 0x74, +} + +type hashed struct { + hash []byte + salt []byte + cost int // allowed range is MinCost to MaxCost + major byte + minor byte +} + +// ErrPasswordTooLong is returned when the password passed to +// GenerateFromPassword is too long (i.e. > 72 bytes). +var ErrPasswordTooLong = errors.New("bcrypt: password length exceeds 72 bytes") + +// GenerateFromPassword returns the bcrypt hash of the password at the given +// cost. If the cost given is less than MinCost, the cost will be set to +// DefaultCost, instead. Use CompareHashAndPassword, as defined in this package, +// to compare the returned hashed password with its cleartext version. +// GenerateFromPassword does not accept passwords longer than 72 bytes, which +// is the longest password bcrypt will operate on. +func GenerateFromPassword(password []byte, cost int) ([]byte, error) { + if len(password) > 72 { + return nil, ErrPasswordTooLong + } + p, err := newFromPassword(password, cost) + if err != nil { + return nil, err + } + return p.Hash(), nil +} + +// CompareHashAndPassword compares a bcrypt hashed password with its possible +// plaintext equivalent. Returns nil on success, or an error on failure. +func CompareHashAndPassword(hashedPassword, password []byte) error { + p, err := newFromHash(hashedPassword) + if err != nil { + return err + } + + otherHash, err := bcrypt(password, p.cost, p.salt) + if err != nil { + return err + } + + otherP := &hashed{otherHash, p.salt, p.cost, p.major, p.minor} + if subtle.ConstantTimeCompare(p.Hash(), otherP.Hash()) == 1 { + return nil + } + + return ErrMismatchedHashAndPassword +} + +// Cost returns the hashing cost used to create the given hashed +// password. When, in the future, the hashing cost of a password system needs +// to be increased in order to adjust for greater computational power, this +// function allows one to establish which passwords need to be updated. +func Cost(hashedPassword []byte) (int, error) { + p, err := newFromHash(hashedPassword) + if err != nil { + return 0, err + } + return p.cost, nil +} + +func newFromPassword(password []byte, cost int) (*hashed, error) { + if cost < MinCost { + cost = DefaultCost + } + p := new(hashed) + p.major = majorVersion + p.minor = minorVersion + + err := checkCost(cost) + if err != nil { + return nil, err + } + p.cost = cost + + unencodedSalt := make([]byte, maxSaltSize) + _, err = io.ReadFull(rand.Reader, unencodedSalt) + if err != nil { + return nil, err + } + + p.salt = base64Encode(unencodedSalt) + hash, err := bcrypt(password, p.cost, p.salt) + if err != nil { + return nil, err + } + p.hash = hash + return p, err +} + +func newFromHash(hashedSecret []byte) (*hashed, error) { + if len(hashedSecret) < minHashSize { + return nil, ErrHashTooShort + } + p := new(hashed) + n, err := p.decodeVersion(hashedSecret) + if err != nil { + return nil, err + } + hashedSecret = hashedSecret[n:] + n, err = p.decodeCost(hashedSecret) + if err != nil { + return nil, err + } + hashedSecret = hashedSecret[n:] + + // The "+2" is here because we'll have to append at most 2 '=' to the salt + // when base64 decoding it in expensiveBlowfishSetup(). + p.salt = make([]byte, encodedSaltSize, encodedSaltSize+2) + copy(p.salt, hashedSecret[:encodedSaltSize]) + + hashedSecret = hashedSecret[encodedSaltSize:] + p.hash = make([]byte, len(hashedSecret)) + copy(p.hash, hashedSecret) + + return p, nil +} + +func bcrypt(password []byte, cost int, salt []byte) ([]byte, error) { + cipherData := make([]byte, len(magicCipherData)) + copy(cipherData, magicCipherData) + + c, err := expensiveBlowfishSetup(password, uint32(cost), salt) + if err != nil { + return nil, err + } + + for i := 0; i < 24; i += 8 { + for j := 0; j < 64; j++ { + c.Encrypt(cipherData[i:i+8], cipherData[i:i+8]) + } + } + + // Bug compatibility with C bcrypt implementations. We only encode 23 of + // the 24 bytes encrypted. + hsh := base64Encode(cipherData[:maxCryptedHashSize]) + return hsh, nil +} + +func expensiveBlowfishSetup(key []byte, cost uint32, salt []byte) (*blowfish.Cipher, error) { + csalt, err := base64Decode(salt) + if err != nil { + return nil, err + } + + // Bug compatibility with C bcrypt implementations. They use the trailing + // NULL in the key string during expansion. + // We copy the key to prevent changing the underlying array. + ckey := append(key[:len(key):len(key)], 0) + + c, err := blowfish.NewSaltedCipher(ckey, csalt) + if err != nil { + return nil, err + } + + var i, rounds uint64 + rounds = 1 << cost + for i = 0; i < rounds; i++ { + blowfish.ExpandKey(ckey, c) + blowfish.ExpandKey(csalt, c) + } + + return c, nil +} + +func (p *hashed) Hash() []byte { + arr := make([]byte, 60) + arr[0] = '$' + arr[1] = p.major + n := 2 + if p.minor != 0 { + arr[2] = p.minor + n = 3 + } + arr[n] = '$' + n++ + copy(arr[n:], []byte(fmt.Sprintf("%02d", p.cost))) + n += 2 + arr[n] = '$' + n++ + copy(arr[n:], p.salt) + n += encodedSaltSize + copy(arr[n:], p.hash) + n += encodedHashSize + return arr[:n] +} + +func (p *hashed) decodeVersion(sbytes []byte) (int, error) { + if sbytes[0] != '$' { + return -1, InvalidHashPrefixError(sbytes[0]) + } + if sbytes[1] > majorVersion { + return -1, HashVersionTooNewError(sbytes[1]) + } + p.major = sbytes[1] + n := 3 + if sbytes[2] != '$' { + p.minor = sbytes[2] + n++ + } + return n, nil +} + +// sbytes should begin where decodeVersion left off. +func (p *hashed) decodeCost(sbytes []byte) (int, error) { + cost, err := strconv.Atoi(string(sbytes[0:2])) + if err != nil { + return -1, err + } + err = checkCost(cost) + if err != nil { + return -1, err + } + p.cost = cost + return 3, nil +} + +func (p *hashed) String() string { + return fmt.Sprintf("&{hash: %#v, salt: %#v, cost: %d, major: %c, minor: %c}", string(p.hash), p.salt, p.cost, p.major, p.minor) +} + +func checkCost(cost int) error { + if cost < MinCost || cost > MaxCost { + return InvalidCostError(cost) + } + return nil +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 08ef873959..bad62046c1 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -2002,6 +2002,7 @@ go4.org/errorutil # golang.org/x/crypto v0.43.0 ## explicit; go 1.24.0 golang.org/x/crypto/argon2 +golang.org/x/crypto/bcrypt golang.org/x/crypto/blake2b golang.org/x/crypto/blowfish golang.org/x/crypto/cast5