Skip to content

Commit c1506f7

Browse files
authored
feat(configgen): yaml.v3 typed-struct config marshal (closes #126) (#130)
Replaces internal/configgen/generator.go's text/template-based agent config generator with typed-struct marshalling through gopkg.in/yaml.v3. Defense-in-depth follow-up to GHSA-7hp6-g3pq-3pc3 — closes the entire YAML-injection class at the output side regardless of upstream validators. The public GeneratorInput / FirewallRule / LighthouseInfo / AdvancedUnsafeRoute types are unchanged; only the implementation moves from string-interpolation to yaml.v3 Encoder. - internal/configgen/marshal.go (new): typed nebulaConfig struct mirroring the previous template's shape. Inline PEM blocks use a literalString MarshalYAML for the `|` literal-block scalar form. - internal/configgen/generator.go: configTemplate, indentLines, and the template-based Generate are removed. Only public input types remain. - internal/configgen/generator_test.go: three multi-address tests converted from byte-equality assertions on flow-style + quoted IPs to parse-then-compare via yaml.Unmarshal, per your issue body. Added TestGenerate_LighthouseAndRelay (lighthouse+relay combo) and an assertion in TestGenerate_Lighthouse that `static_host_map: {}` is emitted explicitly (locks down the empty contract against future omitempty drift). - internal/configgen/advanced_test.go: TestGenerate_TunDevice_StructuralBreakCharsAreQuoted (new) is the defense-in-depth acceptance criterion: a TunDevice value containing \r, \n, ENQ (0x05), ", :, #, or trailing whitespace round-trips through yaml.Unmarshal exactly, and no extra top-level keys appear in the output (which would indicate YAML injection). Mobile bundle path (internal/mobilebundle/builder.go) uses the same GeneratorInput — no API change needed there. make ci green: vet, build, race tests, golangci-lint v2.12.0 ("0 issues."). All existing configgen tests pass plus the new LighthouseAndRelay test plus the 7-case defense-in-depth subtest.
1 parent b8a8ac0 commit c1506f7

4 files changed

Lines changed: 368 additions & 189 deletions

File tree

internal/configgen/advanced_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package configgen
33
import (
44
"strings"
55
"testing"
6+
7+
"gopkg.in/yaml.v3"
68
)
79

810
func TestGenerate_DefaultsWhenNoAdvanced(t *testing.T) {
@@ -100,3 +102,66 @@ func TestGenerate_PunchyOverride(t *testing.T) {
100102
t.Errorf("expected punch: false override; got:\n%s", out)
101103
}
102104
}
105+
106+
// TestGenerate_TunDevice_StructuralBreakCharsAreQuoted is the GHSA-7hp6
107+
// defense-in-depth assertion (issue #126): even if the upstream TunDevice
108+
// validator were bypassed, yaml.v3 marshaling MUST emit structural-break
109+
// characters as safely-quoted scalars so they cannot escape into YAML
110+
// structure. Each pathological TunDevice value below round-trips through
111+
// yaml.Unmarshal exactly — no new top-level keys, no value corruption.
112+
func TestGenerate_TunDevice_StructuralBreakCharsAreQuoted(t *testing.T) {
113+
cases := []string{
114+
"nebula0\rinjected: true", // CR — splits lines in plain scalars
115+
"nebula0\ninjected: true", // LF — same
116+
"nebula0\x05ctrl", // ENQ — arbitrary control character
117+
`nebula0"quote`, // double-quote — terminates quoted scalars
118+
"nebula0:colon", // colon — key separator in plain scalars
119+
"nebula0#hash", // hash — comment indicator
120+
"nebula0 trailing-space ", // leading/trailing whitespace stripping
121+
}
122+
123+
for _, dev := range cases {
124+
t.Run(dev, func(t *testing.T) {
125+
out, err := Generate(GeneratorInput{
126+
HostName: "h",
127+
NebulaIPs: []string{"10.0.0.1"},
128+
CACertPath: "/etc/nebula/ca.crt",
129+
CertPath: "/etc/nebula/host.crt",
130+
KeyPath: "/etc/nebula/host.key",
131+
TunDevice: dev,
132+
})
133+
if err != nil {
134+
t.Fatalf("Generate(%q): %v", dev, err)
135+
}
136+
137+
var parsed struct {
138+
Tun struct {
139+
Dev string `yaml:"dev"`
140+
} `yaml:"tun"`
141+
}
142+
if err := yaml.Unmarshal(out, &parsed); err != nil {
143+
t.Fatalf("invalid YAML for %q: %v\n%s", dev, err, string(out))
144+
}
145+
if parsed.Tun.Dev != dev {
146+
t.Errorf("round-trip mismatch for %q:\n got: %q\n output:\n%s", dev, parsed.Tun.Dev, string(out))
147+
}
148+
149+
// Also assert no injection happened at the top level — only
150+
// the expected keys exist after unmarshal.
151+
var top map[string]any
152+
if err := yaml.Unmarshal(out, &top); err != nil {
153+
t.Fatalf("invalid YAML (top-level) for %q: %v", dev, err)
154+
}
155+
allowed := map[string]bool{
156+
"pki": true, "static_host_map": true, "lighthouse": true,
157+
"listen": true, "punchy": true, "tun": true, "relay": true,
158+
"logging": true, "firewall": true,
159+
}
160+
for k := range top {
161+
if !allowed[k] {
162+
t.Errorf("unexpected top-level key %q for TunDevice=%q — possible YAML injection", k, dev)
163+
}
164+
}
165+
})
166+
}
167+
}

internal/configgen/generator.go

Lines changed: 6 additions & 159 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,5 @@
11
package configgen
22

3-
import (
4-
"bytes"
5-
"fmt"
6-
"strings"
7-
"text/template"
8-
)
9-
103
// LighthouseInfo describes a lighthouse node for config generation.
114
type LighthouseInfo struct {
125
NebulaIPs []string
@@ -20,7 +13,7 @@ type FirewallRule struct {
2013
Group string // "any", "admin", etc.
2114
}
2215

23-
// AdvancedUnsafeRoute mirrors models.UnsafeRoute for the template.
16+
// AdvancedUnsafeRoute mirrors models.UnsafeRoute for the generator input.
2417
type AdvancedUnsafeRoute struct {
2518
Route string
2619
Via string
@@ -48,158 +41,12 @@ type GeneratorInput struct {
4841
TunDevice string
4942
UnsafeRoutes []AdvancedUnsafeRoute
5043

51-
// Optional: if set, override the path-based pki section with inline PEM blocks.
52-
// Used for Mobile Nebula clients which import a self-contained YAML config.
53-
// When CACertPEM is non-empty, CertPEM and KeyPEM must also be non-empty;
54-
// the template uses literal-block scalars for all three. When empty, the
55-
// template falls back to CACertPath/CertPath/KeyPath (default behavior).
44+
// Optional inline PEM blocks. When CACertPEM is non-empty, CertPEM and
45+
// KeyPEM must also be non-empty; all three are emitted as literal-block
46+
// scalars in the pki section. When empty, the path-based fields above
47+
// are used instead. Mobile Nebula clients use the inline form since
48+
// they import a self-contained YAML config.
5649
CACertPEM string
5750
CertPEM string
5851
KeyPEM string
5952
}
60-
61-
const configTemplate = `pki:
62-
{{- if .CACertPEM }}
63-
ca: |
64-
{{ .CACertPEM | indent4 }}
65-
cert: |
66-
{{ .CertPEM | indent4 }}
67-
key: |
68-
{{ .KeyPEM | indent4 }}
69-
{{- else }}
70-
ca: {{ .CACertPath }}
71-
cert: {{ .CertPath }}
72-
key: {{ .KeyPath }}
73-
{{- end }}
74-
75-
{{- if .IsLighthouse }}
76-
77-
static_host_map: {}
78-
79-
lighthouse:
80-
am_lighthouse: true
81-
{{- if gt .ListenPort 0 }}
82-
# Lighthouse listens on port {{ .ListenPort }}
83-
{{- end }}
84-
85-
listen:
86-
host: {{ if .ListenHost }}{{ .ListenHost }}{{ else }}0.0.0.0{{ end }}
87-
port: {{ if gt .ListenPort 0 }}{{ .ListenPort }}{{ else }}4242{{ end }}
88-
89-
{{- else }}
90-
91-
static_host_map:
92-
{{- range $lh := .Lighthouses }}
93-
{{- range $lh.NebulaIPs }}
94-
"{{ . }}": ["{{ $lh.PublicAddr }}"]
95-
{{- end }}
96-
{{- end }}
97-
98-
lighthouse:
99-
am_lighthouse: false
100-
hosts:
101-
{{- range $lh := .Lighthouses }}
102-
{{- range $lh.NebulaIPs }}
103-
- "{{ . }}"
104-
{{- end }}
105-
{{- end }}
106-
107-
listen:
108-
host: {{ if .ListenHost }}{{ .ListenHost }}{{ else }}0.0.0.0{{ end }}
109-
port: {{ if gt .ListenPort 0 }}{{ .ListenPort }}{{ else }}0{{ end }}
110-
111-
{{- end }}
112-
113-
punchy:
114-
punch: {{ if .PunchyOverride }}{{ .PunchyOverride }}{{ else }}true{{ end }}
115-
{{- if or .MTU .TunDevice .UnsafeRoutes }}
116-
117-
tun:
118-
{{- if .TunDevice }}
119-
dev: {{ .TunDevice }}
120-
{{- end }}
121-
{{- if .MTU }}
122-
mtu: {{ .MTU }}
123-
{{- end }}
124-
{{- if .UnsafeRoutes }}
125-
unsafe_routes:
126-
{{- range .UnsafeRoutes }}
127-
- route: {{ .Route }}
128-
via: {{ .Via }}
129-
{{- end }}
130-
{{- end }}
131-
{{- end }}
132-
133-
{{- if .IsRelay }}
134-
135-
relay:
136-
am_relay: true
137-
{{- else if .Relays }}
138-
139-
relay:
140-
relays:
141-
{{- range .Relays }}
142-
- "{{ . }}"
143-
{{- end }}
144-
{{- end }}
145-
146-
logging:
147-
level: info
148-
format: text
149-
150-
firewall:
151-
outbound:
152-
{{- range .FirewallOutbound }}
153-
- port: {{ .Port }}
154-
proto: {{ .Proto }}
155-
{{- if ne .Group "any" }}
156-
group: {{ .Group }}
157-
{{- else }}
158-
host: any
159-
{{- end }}
160-
{{- end }}
161-
162-
inbound:
163-
{{- range .FirewallInbound }}
164-
- port: {{ .Port }}
165-
proto: {{ .Proto }}
166-
{{- if ne .Group "any" }}
167-
group: {{ .Group }}
168-
{{- else }}
169-
host: any
170-
{{- end }}
171-
{{- end }}
172-
`
173-
174-
// indentLines indents each line of the given string by n spaces.
175-
// Strips trailing newline to avoid empty indented lines at the end.
176-
func indentLines(s string, n int) string {
177-
pad := strings.Repeat(" ", n)
178-
s = strings.TrimRight(s, "\n")
179-
lines := strings.Split(s, "\n")
180-
for i, line := range lines {
181-
if line != "" {
182-
lines[i] = pad + line
183-
}
184-
}
185-
return strings.Join(lines, "\n")
186-
}
187-
188-
// Generate produces a Nebula config.yml from the given input.
189-
func Generate(input GeneratorInput) ([]byte, error) {
190-
funcs := template.FuncMap{
191-
"indent4": func(s string) string { return indentLines(s, 4) },
192-
}
193-
194-
tmpl, err := template.New("nebula-config").Funcs(funcs).Parse(configTemplate)
195-
if err != nil {
196-
return nil, fmt.Errorf("parse template: %w", err)
197-
}
198-
199-
var buf bytes.Buffer
200-
if err := tmpl.Execute(&buf, input); err != nil {
201-
return nil, fmt.Errorf("execute template: %w", err)
202-
}
203-
204-
return buf.Bytes(), nil
205-
}

0 commit comments

Comments
 (0)