Просмотр исходного кода

Fix firewall config parsing panics on empty group, null groups, and non-string groups

JackDoan 2 недель назад
Родитель
Сommit
f62c54a4f2
3 измененных файлов с 30 добавлено и 2 удалено
  1. 3 0
      CHANGELOG.md
  2. 7 2
      firewall.go
  3. 20 0
      firewall_test.go

+ 3 - 0
CHANGELOG.md

@@ -13,6 +13,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   down with it. A hostinfo returned by StartHandshake had no RemoteList until the handshake loop's first
   pass attached one, and `create-tunnel -address` set the remote before that. The list is now attached when
   the handshake starts, so a pending hostinfo never exists without one.
+- A firewall rule with `group: []`, `groups:` with no value, or a `groups` entry that isn't a string, such as
+  `groups: [1, 2]`, no longer crashes Nebula on startup or reload. An empty `group` array is rejected, a null
+  `groups` means no groups, and non-string entries are read as strings.
 
 ## [1.11.1] - 2026-08-21
 

+ 7 - 2
firewall.go

@@ -972,6 +972,9 @@ func convertRule(l *slog.Logger, p any, table string, i int) (rule, error) {
 		if len(v) > 1 {
 			return r, errors.New("group should contain a single value, an array with more than one entry was provided")
 		}
+		if len(v) == 0 {
+			return r, errors.New("group should contain a single value, an empty array was provided")
+		}
 
 		l.Warn("group was an array with a single value, converting to simple value",
 			"table", table,
@@ -982,13 +985,15 @@ func convertRule(l *slog.Logger, p any, table string, i int) (rule, error) {
 
 	singleGroup := toString("group", m)
 
-	if rg, ok := m["groups"]; ok {
+	// A null value, `groups:` with nothing after it, is the same as no groups
+	if rg, ok := m["groups"]; ok && rg != nil {
 		switch reflect.TypeOf(rg).Kind() {
 		case reflect.Slice:
+			// Every entry is read as a string, as the other fields are, so `groups: [1, 2]` is the groups "1" and "2"
 			v := reflect.ValueOf(rg)
 			r.Groups = make([]string, v.Len())
 			for i := 0; i < v.Len(); i++ {
-				r.Groups[i] = v.Index(i).Interface().(string)
+				r.Groups[i] = fmt.Sprintf("%v", v.Index(i).Interface())
 			}
 		case reflect.String:
 			r.Groups = []string{rg.(string)}

+ 20 - 0
firewall_test.go

@@ -1439,6 +1439,26 @@ func TestAddFirewallRulesFromConfig(t *testing.T) {
 	require.NoError(t, AddFirewallRulesFromConfig(l, true, conf, mf))
 	assert.Equal(t, addRuleCall{incoming: true, proto: firewall.ProtoAny, startPort: 1, endPort: 1, groups: []string{"a", "b"}, ip: "", localIp: ""}, mf.lastCall)
 
+	// Test group and groups values that used to crash: an empty group array is rejected, groups entries that
+	// aren't strings are read as strings, and a null groups is no groups
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "group": []any{}}}}
+	require.EqualError(t, AddFirewallRulesFromConfig(l, true, conf, mf), "firewall.inbound rule #0; group should contain a single value, an empty array was provided")
+
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "groups": []any{1, 2}}}}
+	require.NoError(t, AddFirewallRulesFromConfig(l, true, conf, mf))
+	assert.Equal(t, []string{"1", "2"}, mf.lastCall.groups)
+
+	conf = config.NewC(test.NewLogger())
+	mf = &mockFirewall{}
+	conf.Settings["firewall"] = map[string]any{"inbound": []any{map[string]any{"port": "1", "proto": "any", "groups": nil, "host": "a"}}}
+	require.NoError(t, AddFirewallRulesFromConfig(l, true, conf, mf))
+	assert.Empty(t, mf.lastCall.groups)
+	assert.Equal(t, "a", mf.lastCall.host)
+
 	// Test Add error
 	conf = config.NewC(test.NewLogger())
 	mf = &mockFirewall{}