Skip to content

Fix resolv.conf content with run --net=private #3424

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 10, 2021
Merged

Fix resolv.conf content with run --net=private #3424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 41 additions & 9 deletions run_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func (b *Builder) Run(command []string, options RunOptions) error {
}

if !(contains(volumes, "/etc/resolv.conf") || (len(b.CommonBuildOpts.DNSServers) == 1 && strings.ToLower(b.CommonBuildOpts.DNSServers[0]) == "none")) {
resolvFile, err := b.addNetworkConfig(path, "/etc/resolv.conf", rootIDPair, b.CommonBuildOpts.DNSServers, b.CommonBuildOpts.DNSSearch, b.CommonBuildOpts.DNSOptions, namespaceOptions)
resolvFile, err := b.addResolvConf(path, rootIDPair, b.CommonBuildOpts.DNSServers, b.CommonBuildOpts.DNSSearch, b.CommonBuildOpts.DNSOptions, namespaceOptions)
if err != nil {
return err
}
Expand Down Expand Up @@ -557,19 +557,52 @@ func (b *Builder) setupMounts(mountPoint string, spec *specs.Spec, bundlePath st
return runTargets, nil
}

// addNetworkConfig copies files from host and sets them up to bind mount into container
func (b *Builder) addNetworkConfig(rdir, hostPath string, chownOpts *idtools.IDPair, dnsServers, dnsSearch, dnsOptions []string, namespaceOptions define.NamespaceOptions) (string, error) {
stat, err := os.Stat(hostPath)
// addResolvConf copies files from host and sets them up to bind mount into container
func (b *Builder) addResolvConf(rdir string, chownOpts *idtools.IDPair, dnsServers, dnsSearch, dnsOptions []string, namespaceOptions define.NamespaceOptions) (string, error) {
resolvConf := "/etc/resolv.conf"

stat, err := os.Stat(resolvConf)
if err != nil {
return "", err
}
contents, err := ioutil.ReadFile(hostPath)
if err != nil {
contents, err := ioutil.ReadFile(resolvConf)
// resolv.conf doesn't have to exists
if err != nil && !os.IsNotExist(err) {
return "", err
}

netns := false
ns := namespaceOptions.Find(string(spec.NetworkNamespace))
if ns != nil && !ns.Host {
netns = true
}

nameservers := resolvconf.GetNameservers(contents, types.IPv4)
// check if systemd-resolved is used, assume it is used when 127.0.0.53 is the only nameserver
if len(nameservers) == 1 && nameservers[0] == "127.0.0.53" && netns {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a test for when only this nameserver is up? I don't see one below, but it's late.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IDK, but can the user change the IP for the system nameserver? It might be nice to have this value stored in a config file that could be tweaked. I'm also a little twitchy about having a particular IP specified and having someone spoof it, but I can't see the harm atm in that case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No this is correct like this. It now does the same as podman. The problem with systemd-resolved is that it only sets 127.0.0.53 as nameserver in /etc/resolv.conf, this is a hard coded value AFAICT and there is no reason why someone should change this. The actual nameservers are in /run/systemd/resolve/resolv.conf. We have to read the actual nameservers in this case and filter the 127... addresses out because there are not reachable in a new netns. If there is no netns we can use the system one because 127.0.0.53 is reachable.
Fedora uses systemd-resolved so the CI should test this.

// read the actual resolv.conf file for systemd-resolved
resolvedContents, err := ioutil.ReadFile("/run/systemd/resolve/resolv.conf")
if err != nil {
if !os.IsNotExist(err) {
return "", errors.Wrapf(err, "detected that systemd-resolved is in use, but could not locate real resolv.conf")
}
} else {
contents = resolvedContents
}
}

// Ensure that the container's /etc/resolv.conf is compatible with its
// network configuration.
if netns {
// FIXME handle IPv6
resolve, err := resolvconf.FilterResolvDNS(contents, true)
if err != nil {
return "", errors.Wrapf(err, "error parsing host resolv.conf")
}
contents = resolve.Content
}
search := resolvconf.GetSearchDomains(contents)
nameservers := resolvconf.GetNameservers(contents, types.IP)
nameservers = resolvconf.GetNameservers(contents, types.IP)
options := resolvconf.GetOptions(contents)

defaultContainerConfig, err := config.Default()
Expand All @@ -582,7 +615,6 @@ func (b *Builder) addNetworkConfig(rdir, hostPath string, chownOpts *idtools.IDP
}

if b.Isolation == IsolationOCIRootless {
ns := namespaceOptions.Find(string(specs.NetworkNamespace))
if ns != nil && !ns.Host && ns.Path == "" {
// if we are using slirp4netns, also add the built-in DNS server.
logrus.Debugf("adding slirp4netns 10.0.2.3 built-in DNS server")
Expand All @@ -607,7 +639,7 @@ func (b *Builder) addNetworkConfig(rdir, hostPath string, chownOpts *idtools.IDP
options = dnsOptions
}

cfile := filepath.Join(rdir, filepath.Base(hostPath))
cfile := filepath.Join(rdir, filepath.Base(resolvConf))
if _, err = resolvconf.Build(cfile, nameservers, search, options); err != nil {
return "", errors.Wrapf(err, "error building resolv.conf for container %s", b.ContainerID)
}
Expand Down
7 changes: 7 additions & 0 deletions tests/helpers.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,13 @@ function check_options_flag_err() {
[[ $output = *"No options ($flag) can be specified after"* ]]
}

#################
# is_rootless # Check if we run as normal user
#################
function is_rootless() {
[ "$(id -u)" -ne 0 ]
}

####################
# skip_if_chroot #
####################
Expand Down
74 changes: 48 additions & 26 deletions tests/run.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,39 +558,61 @@ function configure_and_check_user() {
}

@test "run check /etc/resolv.conf" {
skip_if_no_runtime
skip_if_no_runtime

${OCI} --version
_prefetch debian
${OCI} --version
_prefetch alpine

run_buildah from --quiet --pull=false --signature-policy ${TESTSDIR}/policy.json alpine
cid=$output
run_buildah run --isolation=chroot --network=container $cid cat /etc/resolv.conf
expect_output --substring "nameserver"
m=$(buildah mount $cid)
run cat $m/etc/resolv.conf
[ "$status" -eq 0 ]
expect_output --substring ""
# Make sure to read the correct /etc/resolv.conf file in case of systemd-resolved.
resolve_file=$(readlink -f /etc/resolv.conf)
if [[ "$resolve_file" == "/run/systemd/resolve/stub-resolv.conf" ]]; then
resolve_file="/run/systemd/resolve/resolv.conf"
fi

run grep nameserver $resolve_file
# filter out 127... nameservers
run grep -v "nameserver 127." <<< "$output"
nameservers="$output"
# in case of rootless add extra slirp4netns nameserver
if is_rootless; then
nameservers="nameserver 10.0.2.3
$output"
fi
run_buildah from --quiet --pull=false --signature-policy ${TESTSDIR}/policy.json alpine
cid=$output
run_buildah run --network=private $cid grep nameserver /etc/resolv.conf
# check that no 127... nameserver is in resolv.conf
assert "$output" !~ "^nameserver 127." "Container contains local nameserver"
assert "$nameservers" "Container nameservers match correct host nameservers"
if ! is_rootless; then
run_buildah mount $cid
assert "$output" != ""
assert "$(< $output/etc/resolv.conf)" = "" "resolv.conf is empty"
fi
run_buildah rm -a

run_buildah from --quiet --pull=false --signature-policy ${TESTSDIR}/policy.json alpine
cid=$output
run_buildah run --isolation=chroot --network=host $cid cat /etc/resolv.conf
expect_output --substring "nameserver"
m=$(buildah mount $cid)
run cat $m/etc/resolv.conf
[ "$status" -eq 0 ]
expect_output --substring ""
run grep nameserver /etc/resolv.conf
nameservers="$output"
run_buildah from --quiet --pull=false --signature-policy ${TESTSDIR}/policy.json alpine
cid=$output
run_buildah run --isolation=chroot --network=host $cid grep nameserver /etc/resolv.conf
assert "$nameservers" "Container nameservers match the host nameservers"
if ! is_rootless; then
run_buildah mount $cid
assert "$output" != ""
assert "$(< $output/etc/resolv.conf)" = "" "resolv.conf is empty"
fi
run_buildah rm -a

run_buildah from --quiet --pull=false --signature-policy ${TESTSDIR}/policy.json alpine
cid=$output
run_buildah run --isolation=chroot --network=none $cid sh -c 'echo "nameserver 110.110.0.110" >> /etc/resolv.conf; cat /etc/resolv.conf'
run_buildah from --quiet --pull=false --signature-policy ${TESTSDIR}/policy.json alpine
cid=$output
run_buildah run --isolation=chroot --network=none $cid sh -c 'echo "nameserver 110.110.0.110" >> /etc/resolv.conf; cat /etc/resolv.conf'
expect_output "nameserver 110.110.0.110"
m=$(buildah mount $cid)
run cat $m/etc/resolv.conf
[ "$status" -eq 0 ]
expect_output --substring "nameserver 110.110.0.110"
if ! is_rootless; then
run_buildah mount $cid
assert "$output" != ""
assert "$(< $output/etc/resolv.conf)" =~ "^nameserver 110.110.0.110" "Nameserver is set in the image resolv.conf file"
fi
run_buildah rm -a
}

Expand Down