Skip to content

[release-1.27] OCPBUGS-33810: server: use SecureJoin when setting container /etc directory #8234

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 3 commits into from
May 31, 2024
Merged

[release-1.27] OCPBUGS-33810: server: use SecureJoin when setting container /etc directory #8234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8708001
server: use SecureJoin when setting container /etc directory
May 22, 2024
3443118
server: warn about container /etc not being a regular directory
May 22, 2024
58e697b
golangci-lint: bump cyclomatic complexity again
haircommander May 28, 2024
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
2 changes: 1 addition & 1 deletion .golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,6 @@ linters-settings:
# - filepathJoin
# - whyNoLint
gocyclo:
min-complexity: 154
min-complexity: 175
nakedret:
max-func-lines: 15
42 changes: 34 additions & 8 deletions server/container_create_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -828,16 +828,42 @@ func (s *Server) createSandboxContainer(ctx context.Context, ctr ctrfactory.Cont
rootPair = idtools.IDPair{UID: 0, GID: 0}
}

etc := filepath.Join(mountPoint, "/etc")
// create the `/etc` folder only when it doesn't exist
if _, err := os.Stat(etc); err != nil && os.IsNotExist(err) {
if err := idtools.MkdirAllAndChown(etc, 0o755, rootPair); err != nil {
return nil, fmt.Errorf("error creating mtab directory: %w", err)
etcPath := filepath.Join(mountPoint, "/etc")

// Warn users if the container /etc directory path points to a location
// that is not a regular directory. This could indicate that something
// might be afoot.
etc, err := os.Lstat(etcPath)
if err != nil && !os.IsNotExist(err) {
return nil, err
}
if err == nil && !etc.IsDir() {
log.Warnf(ctx, "Detected /etc path for container %s is not a directory", ctr.ID())
}

// The /etc directory can be subjected to various attempts on the path (directory)
// traversal attacks. As such, we need to ensure that its path will be relative to
// the base (or root, if you wish) of the container to mitigate a container escape.
etcPath, err = securejoin.SecureJoin(mountPoint, "/etc")
if err != nil {
return nil, fmt.Errorf("failed to resolve container /etc directory path: %w", err)
}

// Create the /etc directory only when it doesn't exist.
if _, err := os.Stat(etcPath); err != nil && os.IsNotExist(err) {
if err := idtools.MkdirAllAndChown(etcPath, 0o755, rootPair); err != nil {
return nil, fmt.Errorf("failed to create container /etc directory: %w", err)
}
}
// add symlink /etc/mtab to /proc/mounts allow looking for mountfiles there in the container
// compatible with Docker
if err := os.Symlink("/proc/mounts", filepath.Join(etc, "mtab")); err != nil && !os.IsExist(err) {

// Add a symbolic link from /proc/mounts to /etc/mtab to keep compatibility with legacy
// Linux distributions and Docker.
//
// We cannot use SecureJoin here, as the /etc/mtab can already be symlinked from somewhere
// else in some cases, and doing so would resolve an existing mtab path to the symbolic
// link target location, for example, the /etc/proc/self/mounts, which breaks container
// creation.
if err := os.Symlink("/proc/mounts", filepath.Join(etcPath, "mtab")); err != nil && !os.IsExist(err) {
return nil, err
}

Expand Down