From ff94f9991bd32076c871ef0ad8bc1b763458e480 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Thu, 19 Jun 2025 10:19:41 +1000 Subject: [PATCH] *: switch to safer securejoin.Reopen filepath-securejoin v0.3 gave us a much safer re-open primitive, we should use it to avoid any theoretical attacks. Rather than using it direcly, add a small pathrs wrapper to make libpathrs migrations in the future easier... Signed-off-by: Aleksa Sarai --- internal/pathrs/procfs_securejoin.go | 30 +++++++++++++++++++++ libcontainer/exeseal/cloned_binary_linux.go | 3 ++- libcontainer/standard_init_linux.go | 14 +++++----- 3 files changed, 39 insertions(+), 8 deletions(-) create mode 100644 internal/pathrs/procfs_securejoin.go diff --git a/internal/pathrs/procfs_securejoin.go b/internal/pathrs/procfs_securejoin.go new file mode 100644 index 000000000..77335e7ef --- /dev/null +++ b/internal/pathrs/procfs_securejoin.go @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: Apache-2.0 +/* + * Copyright (C) 2025 Aleksa Sarai + * Copyright (C) 2025 SUSE LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package pathrs + +import ( + "os" + + securejoin "github.com/cyphar/filepath-securejoin" +) + +// Reopen is a wrapper around securejoin.Reopen. +func Reopen(file *os.File, flags int) (*os.File, error) { + return securejoin.Reopen(file, flags) +} diff --git a/libcontainer/exeseal/cloned_binary_linux.go b/libcontainer/exeseal/cloned_binary_linux.go index cfe03da46..1c5c60872 100644 --- a/libcontainer/exeseal/cloned_binary_linux.go +++ b/libcontainer/exeseal/cloned_binary_linux.go @@ -10,6 +10,7 @@ import ( "github.com/sirupsen/logrus" "golang.org/x/sys/unix" + "github.com/opencontainers/runc/internal/pathrs" "github.com/opencontainers/runc/libcontainer/system" ) @@ -71,7 +72,7 @@ func sealFile(f **os.File) error { // When sealing an O_TMPFILE-style descriptor we need to // re-open the path as O_PATH to clear the existing write // handle we have. - opath, err := os.OpenFile(fmt.Sprintf("/proc/self/fd/%d", (*f).Fd()), unix.O_PATH|unix.O_CLOEXEC, 0) + opath, err := pathrs.Reopen(*f, unix.O_PATH|unix.O_CLOEXEC) if err != nil { return fmt.Errorf("reopen tmpfile: %w", err) } diff --git a/libcontainer/standard_init_linux.go b/libcontainer/standard_init_linux.go index 15a4fcb57..f8023f391 100644 --- a/libcontainer/standard_init_linux.go +++ b/libcontainer/standard_init_linux.go @@ -12,6 +12,7 @@ import ( "golang.org/x/sys/unix" "github.com/opencontainers/runc/internal/linux" + "github.com/opencontainers/runc/internal/pathrs" "github.com/opencontainers/runc/libcontainer/apparmor" "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/keys" @@ -259,19 +260,17 @@ func (l *linuxStandardInit) Init() error { return fmt.Errorf("close log pipe: %w", err) } - fifoPath, closer := utils.ProcThreadSelfFd(l.fifoFile.Fd()) - defer closer() - // Wait for the FIFO to be opened on the other side before exec-ing the // user process. We open it through /proc/self/fd/$fd, because the fd that // was given to us was an O_PATH fd to the fifo itself. Linux allows us to // re-open an O_PATH fd through /proc. - fd, err := linux.Open(fifoPath, unix.O_WRONLY|unix.O_CLOEXEC, 0) + fifoFile, err := pathrs.Reopen(l.fifoFile, unix.O_WRONLY|unix.O_CLOEXEC) if err != nil { - return err + return fmt.Errorf("reopen exec fifo: %w", err) } - if _, err := unix.Write(fd, []byte("0")); err != nil { - return &os.PathError{Op: "write exec fifo", Path: fifoPath, Err: err} + defer fifoFile.Close() + if _, err := fifoFile.Write([]byte("0")); err != nil { + return &os.PathError{Op: "write exec fifo", Path: fifoFile.Name(), Err: err} } // Close the O_PATH fifofd fd before exec because the kernel resets @@ -280,6 +279,7 @@ func (l *linuxStandardInit) Init() error { // N.B. the core issue itself (passing dirfds to the host filesystem) has // since been resolved. // https://github.com/torvalds/linux/blob/v4.9/fs/exec.c#L1290-L1318 + _ = fifoFile.Close() _ = l.fifoFile.Close() if s := l.config.SpecState; s != nil {