Skip to content

Commit b625331

Browse files
feat: dedicated least-privilege replication ACL user (#34)
Replicas authenticated to their primary as the full-access `default` user (masterauth only). Seed a dedicated `replicator` ACL user instead — granting just `+psync +replconf +ping` with no key access — and point replicas at it via `masteruser`. A leaked replication credential can now do nothing but replicate, not read or write your data. It mirrors the Sentinel user (S1) and the upstream community operator's dedicated replication user. Seeded in users.acl for every replicating topology (Replication, Cluster, Sentinel); Standalone has no replica link and gets neither the user nor masteruser. The operator's own control connections keep using the default user. Two safety issues a cross-model (Grok) review surfaced are fixed here: - Password rotation now re-keys the managed non-default users (replicator, and the Sentinel user) alongside default, additive-then-cutover. Without this, masteruser=replicator would WRONGPASS the moment a live rotation dropped the old password — a regression this change would otherwise introduce, and a pre-existing gap for the Sentinel user. - `replicator` and `sentinel-user` are reserved: the ValkeyACL webhook rejects them and the reconciler never DELUSERs them, so a user-defined ACL can't wipe the credential the operator depends on. Verified live on a dedicated k3d cluster (a Replication cluster's replicas link as replicator, master_link_status:up, data replicates, no NOPERM) and with a Docker harness for the rotation sequence (after a full add-new + cutover rotation, a forced replica re-handshake re-authenticates as replicator under the new password with no WRONGPASS). Unit tests cover the render/seed/masteruser scoping, the reserved-name webhook rejections, and the managed-user set. Full envtest + lint clean.
1 parent 7730830 commit b625331

9 files changed

Lines changed: 239 additions & 13 deletions

File tree

CHANGELOG.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,17 @@ All notable changes to this project are documented here. The format is based on
44
[Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and the project aims to
55
follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
66

7+
## [Unreleased]
8+
9+
### Changed
10+
- Replicas now authenticate to their primary as a dedicated, least-privilege ACL
11+
user (`replicator`, granting only `+psync +replconf +ping` and no key access)
12+
via `masteruser`, instead of the full-access default user, so a leaked
13+
replication credential can neither read nor write data. It is seeded for every
14+
replicating topology (all but Standalone). Password rotation re-keys it (and the
15+
Sentinel user) in place, so an in-place rotation keeps the replica links alive.
16+
The `replicator` and `sentinel-user` names are reserved and rejected by ValkeyACL.
17+
718
## [0.6.0]
819

920
### Changed

internal/controller/failover.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,23 @@ func (c *replClient) aclAddDefaultPassword(ctx context.Context, password string)
136136
return c.rdb.Do(ctx, "ACL", "SETUSER", "default", ">"+password).Err()
137137
}
138138

139+
// aclSetManagedUser re-keys an operator-managed non-default user (the replication
140+
// or Sentinel identity) during rotation, fully re-specifying its rules so the
141+
// call is correct even if the user was not yet present live (e.g. an upgrade that
142+
// added it). additive=true keeps the old password (ACL accepts old+new during the
143+
// transition); additive=false resets to only the new one at cutover.
144+
func (c *replClient) aclSetManagedUser(ctx context.Context, name, rules, password string, additive bool) error {
145+
args := []any{"ACL", "SETUSER", name, "on"}
146+
if !additive {
147+
args = append(args, "resetpass")
148+
}
149+
args = append(args, ">"+password)
150+
for tok := range strings.FieldsSeq(rules) {
151+
args = append(args, tok)
152+
}
153+
return c.rdb.Do(ctx, args...).Err()
154+
}
155+
139156
// aclSave persists the in-memory ACL to the configured aclfile (users.acl on
140157
// the data PVC). Without it the live `ACL SETUSER` is lost on the next pod
141158
// restart, which reloads the old password from the on-disk aclfile.

internal/controller/password_rotation.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,11 +158,21 @@ func (r *ValkeyClusterReconciler) applyPasswordToPods(ctx context.Context, vc *c
158158
return nil
159159
}
160160

161+
// Operator-managed non-default users (replication / Sentinel) carry the same
162+
// password and must be re-keyed alongside default, or the replica links
163+
// (masteruser=replicator) and Sentinel break the moment the old password drops.
164+
managed := managedNonDefaultUsers(vc)
165+
161166
// Pass 1: every node accepts old AND new; replicas adopt the new masterauth.
162167
if err := onEachPod("add-new", func(c *replClient) error {
163168
if err := c.aclAddDefaultPassword(ctx, newPw); err != nil {
164169
return err
165170
}
171+
for _, u := range managed {
172+
if err := c.aclSetManagedUser(ctx, u.name, u.rules, newPw, true); err != nil {
173+
return err
174+
}
175+
}
166176
return c.configSet(ctx, "masterauth", newPw)
167177
}); err != nil {
168178
return err
@@ -173,6 +183,11 @@ func (r *ValkeyClusterReconciler) applyPasswordToPods(ctx context.Context, vc *c
173183
if err := c.aclSetDefaultPassword(ctx, newPw); err != nil {
174184
return err
175185
}
186+
for _, u := range managed {
187+
if err := c.aclSetManagedUser(ctx, u.name, u.rules, newPw, false); err != nil {
188+
return err
189+
}
190+
}
176191
if err := c.configSet(ctx, "requirepass", newPw); err != nil {
177192
return err
178193
}

internal/controller/resources.go

Lines changed: 72 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -405,11 +405,7 @@ func renderValkeyConf(vc *cachev1beta1.ValkeyCluster, password string) string {
405405
fmt.Fprintf(&b, "bind 0.0.0.0\n")
406406
fmt.Fprintf(&b, "protected-mode no\n")
407407
fmt.Fprintf(&b, "dir %s\n", dataMountPath)
408-
if password != "" {
409-
quotedPassword := valkeyConfigArg(password)
410-
fmt.Fprintf(&b, "requirepass %s\n", quotedPassword)
411-
fmt.Fprintf(&b, "masterauth %s\n", quotedPassword)
412-
}
408+
renderAuthConfig(&b, vc, password)
413409

414410
// Persist ACL state on the data PVC so users survive pod restarts.
415411
// `ACL SAVE` (called by ValkeyACLReconciler) writes here; Valkey loads
@@ -664,9 +660,25 @@ func renderInitScript(vc *cachev1beta1.ValkeyCluster) string {
664660
sentinelReseed = fmt.Sprintf(" echo \"user %s on #$PW_HASH &* %s\" >> %s/users.acl.new\n",
665661
sentinelACLUser, sentinelACLCommands, dataMountPath)
666662
}
663+
// Every replicating topology (all but Standalone) also seeds the dedicated
664+
// replication user a replica authenticates as (masteruser) — minimal commands,
665+
// no key glob, so it is far less exposed than the default user.
666+
replSeed, replReseed := "", ""
667+
if topologyReplicates(vc) {
668+
replSeed = fmt.Sprintf(" echo \"user %s on #$PW_HASH %s\" >> %s/users.acl\n",
669+
replicationACLUser, replicationACLCommands, dataMountPath)
670+
replReseed = fmt.Sprintf(" echo \"user %s on #$PW_HASH %s\" >> %s/users.acl.new\n",
671+
replicationACLUser, replicationACLCommands, dataMountPath)
672+
}
673+
managedSeed := sentinelSeed + replSeed
674+
managedReseed := sentinelReseed + replReseed
675+
// Extra operator-managed user names for the reseed grep (the template already
676+
// lists `default`). Listing a user that isn't in the file is harmless.
677+
managedUsersRe := fmt.Sprintf("%s|%s", sentinelACLUser, replicationACLUser)
667678
// users.acl seeding. Valkey applies `requirepass` first, then loads the
668679
// aclfile, and the aclfile is authoritative — so the operator-managed users
669-
// (default, plus the Sentinel user) carry the password as a SHA-256 hash here.
680+
// (default, plus the Sentinel and replication users) carry the password as a
681+
// SHA-256 hash here.
670682
// When the password changes (e.g. a rotated auth.existingSecret, which
671683
// re-renders requirepass and rolls the pods) the seeded hash no longer matches,
672684
// so we rewrite ONLY the operator-managed user lines and preserve any other
@@ -700,7 +712,7 @@ valkey_config_arg() {
700712
END { printf "\"\n" }
701713
'
702714
}
703-
`, configMountPath, dataMountPath, sentinelSeed, sentinelReseed, sentinelACLUser)
715+
`, configMountPath, dataMountPath, managedSeed, managedReseed, managedUsersRe)
704716

705717
// Multi-region: every pod (including pod-0) replicates from an external
706718
// primary. Local primary/replica entrypoint logic is bypassed.
@@ -1406,6 +1418,59 @@ func tlsEnabled(vc *cachev1beta1.ValkeyCluster) bool {
14061418
return vc.Spec.TLS != nil && vc.Spec.TLS.Enabled
14071419
}
14081420

1421+
// topologyReplicates reports whether the topology has replicas that connect to a
1422+
// primary (so a dedicated replication user is worth seeding). Standalone is the
1423+
// only topology with no replication link.
1424+
func topologyReplicates(vc *cachev1beta1.ValkeyCluster) bool {
1425+
return vc.Spec.Topology != cachev1beta1.TopologyStandalone
1426+
}
1427+
1428+
// isReservedACLUser reports whether an ACL user is operator-managed (the default
1429+
// user plus the dedicated Sentinel and replication identities) and so must never
1430+
// be dropped by the ValkeyACL reconciler. The webhook rejects the same names.
1431+
func isReservedACLUser(name string) bool {
1432+
return name == "default" || name == sentinelACLUser || name == replicationACLUser
1433+
}
1434+
1435+
// managedACLUser is an operator-seeded non-default ACL user plus the ACL rules
1436+
// (channels + commands) that define it.
1437+
type managedACLUser struct {
1438+
name string
1439+
rules string
1440+
}
1441+
1442+
// managedNonDefaultUsers returns the operator-managed ACL users (besides default)
1443+
// this topology seeds, so password rotation can re-key them along with default.
1444+
// Their password always matches the cluster auth password; leaving them stale
1445+
// after a rotation would break the replica links (masteruser) and Sentinel.
1446+
func managedNonDefaultUsers(vc *cachev1beta1.ValkeyCluster) []managedACLUser {
1447+
var users []managedACLUser
1448+
if topologyReplicates(vc) {
1449+
users = append(users, managedACLUser{replicationACLUser, replicationACLCommands})
1450+
}
1451+
if vc.Spec.Topology == cachev1beta1.TopologySentinel {
1452+
users = append(users, managedACLUser{sentinelACLUser, "&* " + sentinelACLCommands})
1453+
}
1454+
return users
1455+
}
1456+
1457+
// renderAuthConfig writes the auth directives into valkey.conf. Replicas
1458+
// authenticate to their primary as the dedicated, least-privilege replication
1459+
// user (seeded in users.acl by renderInitScript) rather than the full-access
1460+
// default user; only replicating topologies need it, and the operator's own
1461+
// control connections keep using the default user.
1462+
func renderAuthConfig(b *strings.Builder, vc *cachev1beta1.ValkeyCluster, password string) {
1463+
if password == "" {
1464+
return
1465+
}
1466+
quotedPassword := valkeyConfigArg(password)
1467+
fmt.Fprintf(b, "requirepass %s\n", quotedPassword)
1468+
fmt.Fprintf(b, "masterauth %s\n", quotedPassword)
1469+
if topologyReplicates(vc) {
1470+
fmt.Fprintf(b, "masteruser %s\n", replicationACLUser)
1471+
}
1472+
}
1473+
14091474
// sourceCAMergeEnabled reports whether this cluster pulls from an external TLS
14101475
// primary signed by a separate CA (S4). When true the operator merges that CA
14111476
// with the local cluster CA into one trust bundle, because Valkey reads a single

internal/controller/resources_test.go

Lines changed: 91 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1572,21 +1572,109 @@ func TestRenderInitScriptReseedsDefaultUserOnPasswordChange(t *testing.T) {
15721572
std.Spec.Auth = &cachev1beta1.AuthSpec{Enabled: true}
15731573
s := renderInitScript(std)
15741574
for _, want := range []string{
1575-
`grep -q "^user default on #$PW_HASH "`, // re-seed unless the line already carries the new hash
1576-
`grep -vE "^user (default|sentinel-user) "`, // preserve other ACL users while replacing operator-managed ones
1577-
dataMountPath + "/users.acl.new", // rewrite via a temp file
1575+
`grep -q "^user default on #$PW_HASH "`, // re-seed unless the line already carries the new hash
1576+
`grep -vE "^user (default|sentinel-user|replicator) "`, // preserve other ACL users while replacing operator-managed ones
1577+
dataMountPath + "/users.acl.new", // rewrite via a temp file
15781578
} {
15791579
if !strings.Contains(s, want) {
15801580
t.Errorf("init script missing re-seed-on-change logic %q\n%s", want, s)
15811581
}
15821582
}
1583+
// A replicating topology (minimalCR is Replication) re-seeds the replication
1584+
// user on a password change too.
1585+
if !strings.Contains(s, `echo "user replicator on #$PW_HASH `+replicationACLCommands+`" >> `+dataMountPath+"/users.acl.new") {
1586+
t.Errorf("init must re-seed the replication ACL user on a password change\n%s", s)
1587+
}
15831588
// Sentinel must re-seed its dedicated ACL user too; it also carries the password.
15841589
sen := renderInitScript(sentinelCR())
15851590
if !strings.Contains(sen, `echo "user sentinel-user on #$PW_HASH &* `+sentinelACLCommands+`" >> `+dataMountPath+"/users.acl.new") {
15861591
t.Errorf("sentinel init must re-seed the sentinel ACL user on a password change\n%s", sen)
15871592
}
15881593
}
15891594

1595+
// The dedicated replication user is seeded and wired as masteruser for every
1596+
// replicating topology (not Standalone), least-privilege vs the default user.
1597+
func TestRenderReplicationUser(t *testing.T) {
1598+
mk := func(topo cachev1beta1.Topology) *cachev1beta1.ValkeyCluster {
1599+
vc := minimalCR()
1600+
vc.Spec.Topology = topo
1601+
vc.Spec.Auth = &cachev1beta1.AuthSpec{Enabled: true}
1602+
if topo == cachev1beta1.TopologyCluster {
1603+
vc.Spec.Shards = ptr.To[int32](3)
1604+
}
1605+
if topo == cachev1beta1.TopologySentinel {
1606+
vc.Spec.Sentinel = &cachev1beta1.SentinelSpec{Replicas: 3}
1607+
}
1608+
return vc
1609+
}
1610+
seedLine := `echo "user replicator on #$PW_HASH ` + replicationACLCommands + `"`
1611+
1612+
for _, topo := range []cachev1beta1.Topology{
1613+
cachev1beta1.TopologyReplication, cachev1beta1.TopologyCluster, cachev1beta1.TopologySentinel,
1614+
} {
1615+
vc := mk(topo)
1616+
conf := renderValkeyConf(vc, "secret")
1617+
if !strings.Contains(conf, "masteruser replicator") {
1618+
t.Errorf("%s: valkey.conf must set masteruser replicator\n%s", topo, conf)
1619+
}
1620+
if s := renderInitScript(vc); !strings.Contains(s, seedLine) {
1621+
t.Errorf("%s: init must seed the replication user\n%s", topo, s)
1622+
}
1623+
}
1624+
1625+
// Standalone has no replicas: no replication user, no masteruser.
1626+
std := mk(cachev1beta1.TopologyStandalone)
1627+
conf := renderValkeyConf(std, "secret")
1628+
if strings.Contains(conf, "masteruser") {
1629+
t.Errorf("Standalone must not set masteruser\n%s", conf)
1630+
}
1631+
if s := renderInitScript(std); strings.Contains(s, "user replicator") {
1632+
t.Errorf("Standalone must not seed the replication user\n%s", s)
1633+
}
1634+
// The replication user carries NO key glob (no data access).
1635+
if strings.Contains(seedLine, "~*") {
1636+
t.Error("replication user must not be granted key access")
1637+
}
1638+
}
1639+
1640+
// managedNonDefaultUsers drives which users password rotation re-keys, and
1641+
// isReservedACLUser protects them from the ValkeyACL reconciler.
1642+
func TestManagedNonDefaultUsersAndReserved(t *testing.T) {
1643+
rep := minimalCR() // Replication
1644+
if got := managedNonDefaultUsers(rep); len(got) != 1 ||
1645+
got[0].name != replicationACLUser || got[0].rules != replicationACLCommands {
1646+
t.Fatalf("Replication managed users = %+v", got)
1647+
}
1648+
1649+
sen := minimalCR()
1650+
sen.Spec.Topology = cachev1beta1.TopologySentinel
1651+
rules := map[string]string{}
1652+
for _, u := range managedNonDefaultUsers(sen) {
1653+
rules[u.name] = u.rules
1654+
}
1655+
if _, ok := rules[replicationACLUser]; !ok {
1656+
t.Error("Sentinel must still re-key the replication user")
1657+
}
1658+
if r := rules[sentinelACLUser]; !strings.HasPrefix(r, "&* ") {
1659+
t.Errorf("sentinel-user rules must keep the channel glob: %q", r)
1660+
}
1661+
1662+
std := minimalCR()
1663+
std.Spec.Topology = cachev1beta1.TopologyStandalone
1664+
if got := managedNonDefaultUsers(std); len(got) != 0 {
1665+
t.Fatalf("Standalone must have no managed non-default users, got %+v", got)
1666+
}
1667+
1668+
for _, n := range []string{"default", replicationACLUser, sentinelACLUser} {
1669+
if !isReservedACLUser(n) {
1670+
t.Errorf("%q must be reserved", n)
1671+
}
1672+
}
1673+
if isReservedACLUser("alice") {
1674+
t.Error("a user-defined name must not be reserved")
1675+
}
1676+
}
1677+
15901678
func TestRenderInitScriptIsValidShell(t *testing.T) {
15911679
std := minimalCR()
15921680
std.Spec.Auth = &cachev1beta1.AuthSpec{Enabled: true}

internal/controller/sentinel.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,16 @@ const (
4040
// user can never read or write your data.
4141
sentinelACLCommands = "+multi +slaveof +ping +exec +subscribe " +
4242
"+config|rewrite +role +publish +info +client|setname +client|kill +script|kill"
43+
// replicationACLUser is the dedicated ACL user a REPLICA authenticates as when
44+
// it connects to its primary (masteruser/masterauth), instead of the
45+
// full-access default user. If the replication credential ever leaks it can do
46+
// nothing but replicate: no key access, no arbitrary commands.
47+
replicationACLUser = "replicator"
48+
// replicationACLCommands is the minimal set a replica needs on its primary:
49+
// PSYNC to start/continue the replication stream, REPLCONF for the handshake
50+
// and ACKs, and PING for keepalive. The stream itself is not ACL-checked per
51+
// key, so no key glob is granted.
52+
replicationACLCommands = "+psync +replconf +ping"
4353
)
4454

4555
// reconcileSentinel brings up Replication primitives plus a separate

internal/controller/valkeyacl_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ func (r *ValkeyACLReconciler) applyACLFanout(
201201
}
202202
}
203203
for _, prev := range acl.Status.AppliedUsers {
204-
if _, keep := desiredNames[prev]; keep || prev == "default" {
204+
if _, keep := desiredNames[prev]; keep || isReservedACLUser(prev) {
205205
continue
206206
}
207207
if err := c.rdb.Do(ctx, "ACL", "DELUSER", prev).Err(); err != nil {

internal/webhook/v1beta1/validators_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,22 @@ func TestValkeyACLValidator(t *testing.T) {
330330
objs: []client.Object{cluster},
331331
wantErr: true,
332332
},
333+
{
334+
name: "reserved replication user -> error",
335+
mutate: func(a *cachev1beta1.ValkeyACL) {
336+
a.Spec.Users = []cachev1beta1.ValkeyACLUser{{Name: "replicator"}}
337+
},
338+
objs: []client.Object{cluster},
339+
wantErr: true,
340+
},
341+
{
342+
name: "reserved sentinel user -> error",
343+
mutate: func(a *cachev1beta1.ValkeyACL) {
344+
a.Spec.Users = []cachev1beta1.ValkeyACLUser{{Name: "sentinel-user"}}
345+
},
346+
objs: []client.Object{cluster},
347+
wantErr: true,
348+
},
333349
{
334350
name: "user passwordSecret missing -> error",
335351
mutate: func(a *cachev1beta1.ValkeyACL) {

internal/webhook/v1beta1/valkeyacl_webhook.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,12 @@ func (v *ValkeyACLCustomValidator) validate(ctx context.Context, acl *cachev1bet
7575
return nil, fmt.Errorf("duplicate user %q in spec.users", u.Name)
7676
}
7777
seen[u.Name] = struct{}{}
78-
if u.Name == "default" {
79-
return nil, fmt.Errorf("user %q is reserved and cannot be managed via ValkeyACL", u.Name)
78+
// Reserved by the operator: the default user plus the dedicated Sentinel
79+
// (sentinel-user) and replication (replicator) identities it seeds itself.
80+
// Managing these via ValkeyACL would let a reset/DELUSER wipe the credential
81+
// the operator's own connections depend on.
82+
if u.Name == "default" || u.Name == "sentinel-user" || u.Name == "replicator" {
83+
return nil, fmt.Errorf("user %q is reserved by the operator and cannot be managed via ValkeyACL", u.Name)
8084
}
8185
}
8286

0 commit comments

Comments
 (0)