Ignore bad ndots in host resolv.conf

Rather than error out if the host's resolv.conf has a bad ndots option,
just ignore it. Still validate ndots supplied via '--dns-option' and
treat failure as an error.

Signed-off-by: Rob Murray <rob.murray@docker.com>

libnetwork/internal/resolvconf/resolvconf.go

@@ -293,21 +293,17 @@ func (rc *ResolvConf) TransformForIntNS(
 		rc.md.UsedDefaultNS = true
 	}
 
-	// Validate the ndots option from host config or overrides, if present.
-	// TODO(robmry) - pre-existing behaviour, but ...
-	//   Validating ndots from an override is good, but not-liking something in the
-	//   host's resolv.conf isn't a reason to fail - just remove? (And it'll be
-	//   replaced by the value in reqdOptions, if given.)
-	if ndots, exists := rc.Option("ndots"); exists {
-		if n, err := strconv.Atoi(ndots); err != nil || n < 0 {
-			return nil, errdefs.InvalidParameter(
-				fmt.Errorf("invalid number for ndots option: %v", ndots))
-		}
-	}
-	// For each option required by the nameserver, add it if not already
-	// present (if the option already has a value don't change it).
+	// For each option required by the nameserver, add it if not already present. If
+	// the option is already present, don't override it. Apart from ndots - if the
+	// ndots value is invalid and an ndots option is required, replace the existing
+	// value.
 	for _, opt := range reqdOptions {
 		optName, _, _ := strings.Cut(opt, ":")
+		if optName == "ndots" {
+			rc.options = removeInvalidNDots(rc.options)
+			// No need to update rc.md.NDotsFrom, if there is no ndots option remaining,
+			// it'll be set to "internal" when the required value is added.
+		}
 		if _, exists := rc.Option(optName); !exists {
 			rc.AddOption(opt)
 		}
@@ -496,3 +492,22 @@ func defaultNSAddrs(ipv6 bool) []netip.Addr {
 	}
 	return addrs
 }
+
+// removeInvalidNDots filters ill-formed "ndots" settings from options.
+// The backing array of the options slice is reused.
+func removeInvalidNDots(options []string) []string {
+	n := 0
+	for _, opt := range options {
+		k, v, _ := strings.Cut(opt, ":")
+		if k == "ndots" {
+			ndots, err := strconv.Atoi(v)
+			if err != nil || ndots < 0 {
+				continue
+			}
+		}
+		options[n] = opt
+		n++
+	}
+	clear(options[n:]) // Zero out the obsolete elements, for GC.
+	return options[:n]
+}

libnetwork/internal/resolvconf/resolvconf_test.go

@@ -500,6 +500,60 @@ func TestRCTransformForIntNS(t *testing.T) {
 	}
 }
 
+// Check that invalid ndots options in the host's file are ignored, unless
+// starting the internal resolver (which requires an ndots option), in which
+// case invalid ndots should be replaced.
+func TestRCTransformForIntNSInvalidNdots(t *testing.T) {
+	testcases := []struct {
+		name         string
+		options      string
+		reqdOptions  []string
+		expVal       string
+		expOptions   []string
+		expNDotsFrom string
+	}{
+		{
+			name:         "Negative value",
+			options:      "options ndots:-1",
+			expOptions:   []string{"ndots:-1"},
+			expVal:       "-1",
+			expNDotsFrom: "host",
+		},
+		{
+			name:         "Invalid values with reqd ndots",
+			options:      "options ndots:-1 foo:bar ndots ndots:",
+			reqdOptions:  []string{"ndots:2"},
+			expVal:       "2",
+			expNDotsFrom: "internal",
+			expOptions:   []string{"foo:bar", "ndots:2"},
+		},
+		{
+			name:         "Valid value with reqd ndots",
+			options:      "options ndots:1 foo:bar ndots ndots:",
+			reqdOptions:  []string{"ndots:2"},
+			expVal:       "1",
+			expNDotsFrom: "host",
+			expOptions:   []string{"ndots:1", "foo:bar"},
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			content := "nameserver 8.8.8.8\n" + tc.options
+			rc, err := Parse(bytes.NewBuffer([]byte(content)), "/etc/resolv.conf")
+			assert.NilError(t, err)
+			_, err = rc.TransformForIntNS(false, netip.MustParseAddr("127.0.0.11"), tc.reqdOptions)
+			assert.NilError(t, err)
+
+			val, found := rc.Option("ndots")
+			assert.Check(t, is.Equal(found, true))
+			assert.Check(t, is.Equal(val, tc.expVal))
+			assert.Check(t, is.Equal(rc.md.NDotsFrom, tc.expNDotsFrom))
+			assert.Check(t, is.DeepEqual(rc.options, tc.expOptions))
+		})
+	}
+}
+
 func TestRCRead(t *testing.T) {
 	d := t.TempDir()
 	path := filepath.Join(d, "resolv.conf")

libnetwork/sandbox_dns_unix_test.go

@@ -77,11 +77,19 @@ func TestDNSOptions(t *testing.T) {
 	err = sb2.setupDNS()
 	assert.NilError(t, err)
 	err = sb2.rebuildDNS()
-	assert.Error(t, err, "invalid number for ndots option: foobar")
+	assert.NilError(t, err)
+	currRC, err = resolvconf.GetSpecific(sb2.config.resolvConfPath)
+	assert.NilError(t, err)
+	dnsOptionsList = resolvconf.GetOptions(currRC.Content)
+	assert.Check(t, is.DeepEqual([]string{"ndots:0"}, dnsOptionsList))
 
 	sb2.config.dnsOptionsList = []string{"ndots:-1"}
 	err = sb2.setupDNS()
 	assert.NilError(t, err)
 	err = sb2.rebuildDNS()
-	assert.Error(t, err, "invalid number for ndots option: -1")
+	assert.NilError(t, err)
+	currRC, err = resolvconf.GetSpecific(sb2.config.resolvConfPath)
+	assert.NilError(t, err)
+	dnsOptionsList = resolvconf.GetOptions(currRC.Content)
+	assert.Check(t, is.DeepEqual([]string{"ndots:0"}, dnsOptionsList))
 }