Skip to content

Commit bf772aa

Browse files
committed
Scope Consul agent read to attested peer
1 parent 0a48280 commit bf772aa

4 files changed

Lines changed: 115 additions & 36 deletions

File tree

consul-postgres-ha/admission-broker/main.go

Lines changed: 55 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,9 @@ type Workload struct {
9595
}
9696

9797
type ConsulPermissions struct {
98-
KeyPrefixes []string `json:"key_prefixes,omitempty"`
99-
SessionWrite bool `json:"session_write,omitempty"`
98+
KeyPrefixes []string `json:"key_prefixes,omitempty"`
99+
SessionWrite bool `json:"session_write,omitempty"`
100+
AgentReadSelf bool `json:"agent_read_self,omitempty"`
100101
}
101102

102103
func ParsePolicy(b []byte) (*Policy, error) {
@@ -243,7 +244,8 @@ func (s *Server) handleAttest(w http.ResponseWriter, r *http.Request) {
243244
}
244245

245246
binding := firstNonEmpty(req.Binding, req.CertPubKey)
246-
if err := validateBinding(req.Identity, binding); err != nil {
247+
bindingStatement, err := parseBindingStatement(req.Identity, binding)
248+
if err != nil {
247249
writeError(w, http.StatusBadRequest, "BAD_REQUEST", err.Error())
248250
return
249251
}
@@ -291,6 +293,7 @@ func (s *Server) handleAttest(w http.ResponseWriter, r *http.Request) {
291293
Description: fmt.Sprintf("attested %s %s", workload.WorkloadID, verified.ComposeHash),
292294
ConsulService: workload.ConsulService,
293295
ConsulPermissions: workload.ConsulPermissions,
296+
PeerID: bindingStatement.PeerID,
294297
TTL: s.tokenTTL,
295298
})
296299
if err != nil {
@@ -339,24 +342,36 @@ func rawJSONAsString(raw json.RawMessage) string {
339342
return string(raw)
340343
}
341344

342-
func validateBinding(identity, bindingHex string) error {
345+
type BindingStatement struct {
346+
Version int `json:"version"`
347+
Identity string `json:"identity"`
348+
Cluster string `json:"cluster,omitempty"`
349+
PeerID string `json:"peer_id,omitempty"`
350+
AppID string `json:"app_id,omitempty"`
351+
InstanceID string `json:"instance_id,omitempty"`
352+
ComposeHash string `json:"compose_hash,omitempty"`
353+
IssuedAt string `json:"issued_at,omitempty"`
354+
}
355+
356+
func parseBindingStatement(identity, bindingHex string) (*BindingStatement, error) {
343357
if bindingHex == "" {
344-
return errors.New("binding is required")
358+
return nil, errors.New("binding is required")
345359
}
346360
binding, err := hex.DecodeString(strings.TrimPrefix(strings.ToLower(strings.TrimSpace(bindingHex)), "0x"))
347361
if err != nil {
348-
return fmt.Errorf("binding must be hex: %w", err)
362+
return nil, fmt.Errorf("binding must be hex: %w", err)
349363
}
350-
var statement struct {
351-
Identity string `json:"identity"`
364+
if len(binding) == 0 {
365+
return nil, errors.New("binding must not be empty")
352366
}
367+
var statement BindingStatement
353368
if err := json.Unmarshal(binding, &statement); err != nil {
354-
return fmt.Errorf("binding must be canonical JSON statement bytes encoded as hex: %w", err)
369+
return nil, fmt.Errorf("binding must be canonical JSON statement bytes encoded as hex: %w", err)
355370
}
356371
if statement.Identity != identity {
357-
return fmt.Errorf("binding identity %q does not match request identity %q", statement.Identity, identity)
372+
return nil, fmt.Errorf("binding identity %q does not match request identity %q", statement.Identity, identity)
358373
}
359-
return nil
374+
return &statement, nil
360375
}
361376

362377
type NonceStore struct {
@@ -503,6 +518,7 @@ type TokenRequest struct {
503518
Description string
504519
ConsulService string
505520
ConsulPermissions ConsulPermissions
521+
PeerID string
506522
TTL time.Duration
507523
}
508524

@@ -527,8 +543,12 @@ func (i *ConsulIssuer) IssueToken(ctx context.Context, req TokenRequest) (string
527543
{"ServiceName": req.ConsulService},
528544
},
529545
}
530-
if rules := consulACLRules(req.ConsulPermissions); rules != "" {
531-
policyName, err := i.EnsurePolicy(ctx, "attested-dcs-"+shortSHA256(rules, 16), "DCS permissions for attested workloads", rules)
546+
rules, err := consulACLRules(req.ConsulPermissions, req.PeerID)
547+
if err != nil {
548+
return "", err
549+
}
550+
if rules != "" {
551+
policyName, err := i.EnsurePolicy(ctx, "attested-workload-"+shortSHA256(rules, 16), "Additional permissions for attested workloads", rules)
532552
if err != nil {
533553
return "", err
534554
}
@@ -663,8 +683,15 @@ func (e *consulHTTPError) Error() string {
663683
return fmt.Sprintf("consul returned HTTP %d: %s", e.StatusCode, e.Body)
664684
}
665685

666-
func consulACLRules(p ConsulPermissions) string {
686+
func consulACLRules(p ConsulPermissions, peerID string) (string, error) {
667687
var b strings.Builder
688+
if p.AgentReadSelf {
689+
peerID = strings.TrimSpace(peerID)
690+
if err := validateConsulAgentName(peerID); err != nil {
691+
return "", fmt.Errorf("agent_read_self requires attested peer_id: %w", err)
692+
}
693+
fmt.Fprintf(&b, "agent %q {\n policy = \"read\"\n}\n", peerID)
694+
}
668695
for _, prefix := range p.KeyPrefixes {
669696
prefix = strings.Trim(prefix, "/")
670697
if prefix == "" {
@@ -675,7 +702,20 @@ func consulACLRules(p ConsulPermissions) string {
675702
if p.SessionWrite {
676703
b.WriteString("session_prefix \"\" {\n policy = \"write\"\n}\n")
677704
}
678-
return b.String()
705+
return b.String(), nil
706+
}
707+
708+
func validateConsulAgentName(name string) error {
709+
if name == "" {
710+
return errors.New("peer_id is empty")
711+
}
712+
for _, r := range name {
713+
if r >= 'a' && r <= 'z' || r >= 'A' && r <= 'Z' || r >= '0' && r <= '9' || r == '-' || r == '_' || r == '.' {
714+
continue
715+
}
716+
return fmt.Errorf("peer_id %q contains unsupported character %q", name, r)
717+
}
718+
return nil
679719
}
680720

681721
func shortSHA256(s string, hexChars int) string {

consul-postgres-ha/admission-broker/main_test.go

Lines changed: 46 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ func TestAttestFlow(t *testing.T) {
133133
}
134134
_ = challengeResp.Body.Close()
135135

136-
statement := `{"identity":"spiffe://demo/webdemo"}`
136+
statement := `{"identity":"spiffe://demo/webdemo","peer_id":"worker-1"}`
137137
expectedReportData, err = reportDataHex(hex.EncodeToString([]byte(statement)), challenge.Nonce)
138138
if err != nil {
139139
t.Fatal(err)
@@ -166,6 +166,9 @@ func TestAttestFlow(t *testing.T) {
166166
if issuer.req.ConsulService != "webdemo" {
167167
t.Fatalf("wrong consul service: %s", issuer.req.ConsulService)
168168
}
169+
if issuer.req.PeerID != "worker-1" {
170+
t.Fatalf("wrong peer id: %s", issuer.req.PeerID)
171+
}
169172
}
170173

171174
func TestAttestCarriesConsulPermissionsFromPolicy(t *testing.T) {
@@ -189,7 +192,7 @@ func TestAttestCarriesConsulPermissionsFromPolicy(t *testing.T) {
189192
"workload_id":"demo/worker/0/postgres",
190193
"identity":"spiffe://demo/postgres",
191194
"consul_service":"demo",
192-
"consul_permissions":{"key_prefixes":["/service/demo/"],"session_write":true},
195+
"consul_permissions":{"key_prefixes":["/service/demo/"],"session_write":true,"agent_read_self":true},
193196
"allowed_compose_hashes":["`+testHash+`"]
194197
}]}`),
195198
nonces: NewNonceStore(time.Minute),
@@ -214,7 +217,7 @@ func TestAttestCarriesConsulPermissionsFromPolicy(t *testing.T) {
214217
}
215218
_ = challengeResp.Body.Close()
216219

217-
statement := `{"identity":"spiffe://demo/postgres"}`
220+
statement := `{"identity":"spiffe://demo/postgres","peer_id":"worker-1"}`
218221
expectedReportData, err = reportDataHex(hex.EncodeToString([]byte(statement)), challenge.Nonce)
219222
if err != nil {
220223
t.Fatal(err)
@@ -242,25 +245,42 @@ func TestAttestCarriesConsulPermissionsFromPolicy(t *testing.T) {
242245
if !issuer.req.ConsulPermissions.SessionWrite {
243246
t.Fatal("session_write was not carried to issuer")
244247
}
248+
if !issuer.req.ConsulPermissions.AgentReadSelf {
249+
t.Fatal("agent_read_self was not carried to issuer")
250+
}
251+
if issuer.req.PeerID != "worker-1" {
252+
t.Fatalf("wrong peer id: %s", issuer.req.PeerID)
253+
}
245254
}
246255

247256
func TestConsulACLRules(t *testing.T) {
248-
got := consulACLRules(ConsulPermissions{
249-
KeyPrefixes: []string{"service/demo"},
250-
SessionWrite: true,
251-
})
252-
want := "key_prefix \"service/demo\" {\n policy = \"write\"\n}\nsession_prefix \"\" {\n policy = \"write\"\n}\n"
257+
got := mustConsulACLRules(t, ConsulPermissions{
258+
KeyPrefixes: []string{"service/demo"},
259+
SessionWrite: true,
260+
AgentReadSelf: true,
261+
}, "worker-1")
262+
want := "agent \"worker-1\" {\n policy = \"read\"\n}\nkey_prefix \"service/demo\" {\n policy = \"write\"\n}\nsession_prefix \"\" {\n policy = \"write\"\n}\n"
253263
if got != want {
254264
t.Fatalf("rules mismatch\nwant:\n%s\ngot:\n%s", want, got)
255265
}
256266
}
257267

268+
func TestConsulACLRulesRequiresPeerIDForAgentReadSelf(t *testing.T) {
269+
_, err := consulACLRules(ConsulPermissions{AgentReadSelf: true}, "")
270+
if err == nil {
271+
t.Fatal("expected peer_id requirement")
272+
}
273+
}
274+
258275
func TestConsulIssuerCreatesPolicyForCustomPermissions(t *testing.T) {
259276
var sawPolicyCreate bool
260277
var tokenPolicies []map[string]string
278+
permissions := ConsulPermissions{KeyPrefixes: []string{"service/demo"}, SessionWrite: true, AgentReadSelf: true}
279+
rules := mustConsulACLRules(t, permissions, "worker-1")
280+
policyName := "attested-workload-" + shortSHA256(rules, 16)
261281
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
262282
switch {
263-
case r.Method == http.MethodGet && r.URL.Path == "/v1/acl/policy/name/attested-dcs-"+shortSHA256(consulACLRules(ConsulPermissions{KeyPrefixes: []string{"service/demo"}, SessionWrite: true}), 16):
283+
case r.Method == http.MethodGet && r.URL.Path == "/v1/acl/policy/name/"+policyName:
264284
http.NotFound(w, r)
265285
case r.Method == http.MethodPut && r.URL.Path == "/v1/acl/policy":
266286
var body struct {
@@ -273,6 +293,9 @@ func TestConsulIssuerCreatesPolicyForCustomPermissions(t *testing.T) {
273293
if body.Name == "" || body.Rules == "" {
274294
t.Fatalf("bad policy body: %+v", body)
275295
}
296+
if body.Name != policyName || body.Rules != rules {
297+
t.Fatalf("wrong policy body: %+v", body)
298+
}
276299
sawPolicyCreate = true
277300
writeJSON(w, http.StatusOK, map[string]any{"Name": body.Name, "Rules": body.Rules})
278301
case r.Method == http.MethodPut && r.URL.Path == "/v1/acl/token":
@@ -296,13 +319,11 @@ func TestConsulIssuerCreatesPolicyForCustomPermissions(t *testing.T) {
296319

297320
issuer := &ConsulIssuer{BaseURL: ts.URL, ManagementToken: "mgmt", HTTP: ts.Client()}
298321
token, err := issuer.IssueToken(context.Background(), TokenRequest{
299-
Description: "test",
300-
ConsulService: "demo",
301-
ConsulPermissions: ConsulPermissions{
302-
KeyPrefixes: []string{"service/demo"},
303-
SessionWrite: true,
304-
},
305-
TTL: time.Hour,
322+
Description: "test",
323+
ConsulService: "demo",
324+
ConsulPermissions: permissions,
325+
PeerID: "worker-1",
326+
TTL: time.Hour,
306327
})
307328
if err != nil {
308329
t.Fatal(err)
@@ -382,3 +403,12 @@ func mustPolicy(t *testing.T, raw string) *Policy {
382403
}
383404
return p
384405
}
406+
407+
func mustConsulACLRules(t *testing.T, permissions ConsulPermissions, peerID string) string {
408+
t.Helper()
409+
rules, err := consulACLRules(permissions, peerID)
410+
if err != nil {
411+
t.Fatal(err)
412+
}
413+
return rules
414+
}

consul-postgres-ha/cluster-example/cluster.tf

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -294,17 +294,19 @@ locals {
294294
identity = "spiffe://${var.cluster_name}/webdemo"
295295
consul_service = "webdemo"
296296
consul_permissions = {
297-
key_prefixes = []
298-
session_write = false
297+
key_prefixes = []
298+
session_write = false
299+
agent_read_self = true
299300
}
300301
},
301302
{
302303
name = "postgres"
303304
identity = "spiffe://${var.cluster_name}/postgres"
304305
consul_service = var.cluster_name
305306
consul_permissions = {
306-
key_prefixes = ["service/${var.cluster_name}"]
307-
session_write = true
307+
key_prefixes = ["service/${var.cluster_name}"]
308+
session_write = true
309+
agent_read_self = true
308310
}
309311
},
310312
]

consul-postgres-ha/design/attestation-admission.md

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,8 @@ A generated manifest can be small and explicit:
211211
"consul_service": "demo",
212212
"consul_permissions": {
213213
"key_prefixes": ["service/demo"],
214-
"session_write": true
214+
"session_write": true,
215+
"agent_read_self": true
215216
},
216217
"allowed_compose_hashes": [
217218
"9157fe4a3b6da46de49f8e2ce0943dbd43cfb6001eefae6b1bcc2c9d0749c5a4"
@@ -221,6 +222,12 @@ A generated manifest can be small and explicit:
221222
}
222223
```
223224

225+
`agent_read_self` is intentionally not a broad Consul `agent_prefix`
226+
grant. The admission broker derives the Consul node name from the
227+
attested binding statement's `peer_id` and emits only `agent
228+
"<peer_id>" { policy = "read" }`, so Envoy bootstrap can read its
229+
local agent without gaining visibility into other agents.
230+
224231
The manifest should be generated from the same Terraform graph that
225232
declares the CVMs. The Phala Terraform provider can call the same
226233
preflight path used before deployment and expose the resulting

0 commit comments

Comments
 (0)