Skip to content

Commit 0188ef2

Browse files
authored
acmeserver: warn when policy rules unset (#7469)
1 parent c0af7b6 commit 0188ef2

File tree

2 files changed

+111
-0
lines changed

2 files changed

+111
-0
lines changed

modules/caddypki/acmeserver/acmeserver.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,8 @@ func (ash *Handler) Provision(ctx caddy.Context) error {
140140
}
141141
}
142142

143+
ash.warnIfPolicyAllowsAll()
144+
143145
// get a reference to the configured CA
144146
appModule, err := ctx.App("pki")
145147
if err != nil {
@@ -214,6 +216,21 @@ func (ash *Handler) Provision(ctx caddy.Context) error {
214216
return nil
215217
}
216218

219+
func (ash *Handler) warnIfPolicyAllowsAll() {
220+
allow := ash.Policy.normalizeAllowRules()
221+
deny := ash.Policy.normalizeDenyRules()
222+
if allow != nil || deny != nil {
223+
return
224+
}
225+
226+
allowWildcardNames := ash.Policy != nil && ash.Policy.AllowWildcardNames
227+
ash.logger.Warn(
228+
"acme_server policy has no allow/deny rules; order identifiers are unrestricted (allow-all)",
229+
zap.String("ca", ash.CA),
230+
zap.Bool("allow_wildcard_names", allowWildcardNames),
231+
)
232+
}
233+
217234
func (ash Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyhttp.Handler) error {
218235
if strings.HasPrefix(r.URL.Path, ash.PathPrefix) {
219236
acmeCtx := acme.NewContext(
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
package acmeserver
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
"go.uber.org/zap"
8+
"go.uber.org/zap/zaptest/observer"
9+
)
10+
11+
func TestHandler_warnIfPolicyAllowsAll(t *testing.T) {
12+
tests := []struct {
13+
name string
14+
policy *Policy
15+
wantWarns int
16+
wantAllowWildcard bool
17+
}{
18+
{
19+
name: "warns when policy is nil",
20+
policy: nil,
21+
wantWarns: 1,
22+
wantAllowWildcard: false,
23+
},
24+
{
25+
name: "warns when allow/deny rules are empty",
26+
policy: &Policy{},
27+
wantWarns: 1,
28+
wantAllowWildcard: false,
29+
},
30+
{
31+
name: "warns when only allow_wildcard_names is true",
32+
policy: &Policy{
33+
AllowWildcardNames: true,
34+
},
35+
wantWarns: 1,
36+
wantAllowWildcard: true,
37+
},
38+
{
39+
name: "does not warn when allow rules are configured",
40+
policy: &Policy{
41+
Allow: &RuleSet{
42+
Domains: []string{"example.com"},
43+
},
44+
},
45+
wantWarns: 0,
46+
wantAllowWildcard: false,
47+
},
48+
{
49+
name: "does not warn when deny rules are configured",
50+
policy: &Policy{
51+
Deny: &RuleSet{
52+
Domains: []string{"bad.example.com"},
53+
},
54+
},
55+
wantWarns: 0,
56+
wantAllowWildcard: false,
57+
},
58+
}
59+
60+
for _, tt := range tests {
61+
t.Run(tt.name, func(t *testing.T) {
62+
core, logs := observer.New(zap.WarnLevel)
63+
ash := &Handler{
64+
CA: "local",
65+
Policy: tt.policy,
66+
logger: zap.New(core),
67+
}
68+
69+
ash.warnIfPolicyAllowsAll()
70+
if logs.Len() != tt.wantWarns {
71+
t.Fatalf("expected %d warning logs, got %d", tt.wantWarns, logs.Len())
72+
}
73+
74+
if tt.wantWarns == 0 {
75+
return
76+
}
77+
78+
entry := logs.All()[0]
79+
if entry.Level != zap.WarnLevel {
80+
t.Fatalf("expected warn level, got %v", entry.Level)
81+
}
82+
if !strings.Contains(entry.Message, "policy has no allow/deny rules") {
83+
t.Fatalf("unexpected log message: %q", entry.Message)
84+
}
85+
ctx := entry.ContextMap()
86+
if ctx["ca"] != "local" {
87+
t.Fatalf("expected ca=local, got %v", ctx["ca"])
88+
}
89+
if ctx["allow_wildcard_names"] != tt.wantAllowWildcard {
90+
t.Fatalf("expected allow_wildcard_names=%v, got %v", tt.wantAllowWildcard, ctx["allow_wildcard_names"])
91+
}
92+
})
93+
}
94+
}

0 commit comments

Comments
 (0)