containers
/
automation-tests
mirror of https://github.com/containers/automation-tests.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

pkg/seccomp: simplify IsSupported 

Current implementation of seccomp.IsSupported (rooted in runc) is not
very good.

First, it parses the whole /proc/self/status, adding each key: value
pair into the map (lots of allocations and future work for garbage
collector), when using a single key from that map.

Second, the presence of "Seccomp" key in /proc/self/status merely means
that kernel option CONFIG_SECCOMP is set, but there is a need to _also_
check for CONFIG_SECCOMP_FILTER (the code for which exists but never
executed in case /proc/self/status has Seccomp key).

Replace all this with a single call to prctl; see the long comment in
the code for details.

NOTE historically, parsing /proc/self/status was added after a concern
was raised in https://github.com/opencontainers/runc/pull/471 that
prctl(PR_GET_SECCOMP, ...) can result in the calling process being
killed with SIGKILL. This is a valid concern, so the new code here
does not use PR_GET_SECCOMP at all.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This commit is contained in:
Kir Kolyshkin 2021-03-23 17:22:04 -07:00
parent 97681e1ae0
commit 2f8a504f7c
2 changed files with 27 additions and 247 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

89
common/pkg/seccomp/supported.go
View File

@ -3,72 +3,37 @@
package seccomp
import (
"bufio"
"errors"
"os"
"strings"
perrors "github.com/pkg/errors"
"golang.org/x/sys/unix"
)
const statusFilePath = "/proc/self/status"
// IsSupported returns true if the system has been configured to support
// seccomp.
// seccomp (including the check for CONFIG_SECCOMP_FILTER kernel option).
func IsSupported() bool {
// Since Linux 3.8, the Seccomp field of the /proc/[pid]/status file
// provides a method of obtaining the same information, without the risk
// that the process is killed; see proc(5).
status, err := parseStatusFile(statusFilePath)
if err == nil {
_, ok := status["Seccomp"]
return ok
}
// Excerpts from prctl(2), section ERRORS:
//
// EACCES
// option is PR_SET_SECCOMP and arg2 is SECCOMP_MODE_FILTER, but
// the process does not have the CAP_SYS_ADMIN capability or has
// not set the no_new_privs attribute <...>.
// <...>
// EFAULT
// option is PR_SET_SECCOMP, arg2 is SECCOMP_MODE_FILTER, the
// system was built with CONFIG_SECCOMP_FILTER, and arg3 is an
// invalid address.
// <...>
// EINVAL
// option is PR_SET_SECCOMP or PR_GET_SECCOMP, and the kernel
// was not configured with CONFIG_SECCOMP.
//
// EINVAL
// option is PR_SET_SECCOMP, arg2 is SECCOMP_MODE_FILTER,
// and the kernel was not configured with CONFIG_SECCOMP_FILTER.
// <end of quote>
//
// Meaning, in case these kernel options are set (this is what we check
// for here), we will get some other error (most probably EACCES or
// EFAULT). IOW, EINVAL means "seccomp not supported", any other error
// means it is supported.
// PR_GET_SECCOMP (since Linux 2.6.23)
// Return (as the function result) the secure computing mode of the calling
// thread. If the caller is not in secure computing mode, this operation
// returns 0; if the caller is in strict secure computing mode, then the
// prctl() call will cause a SIGKILL signal to be sent to the process. If
// the caller is in filter mode, and this system call is allowed by the
// seccomp filters, it returns 2; otherwise, the process is killed with a
// SIGKILL signal. This operation is available only if the kernel is
// configured with CONFIG_SECCOMP enabled.
if err := unix.Prctl(unix.PR_GET_SECCOMP, 0, 0, 0, 0); !errors.Is(err, unix.EINVAL) {
// Make sure the kernel has CONFIG_SECCOMP_FILTER.
if err := unix.Prctl(unix.PR_SET_SECCOMP, unix.SECCOMP_MODE_FILTER, 0, 0, 0); !errors.Is(err, unix.EINVAL) {
return true
}
}
return false
}
// parseStatusFile reads the provided `file` into a map of strings.
func parseStatusFile(file string) (map[string]string, error) {
f, err := os.Open(file)
if err != nil {
return nil, perrors.Wrapf(err, "open status file %s", file)
}
defer f.Close()
status := make(map[string]string)
scanner := bufio.NewScanner(f)
for scanner.Scan() {
text := scanner.Text()
parts := strings.SplitN(text, ":", 2)
if len(parts) <= 1 {
continue
}
status[strings.TrimSpace(parts[0])] = strings.TrimSpace(parts[1])
}
if err := scanner.Err(); err != nil {
return nil, perrors.Wrapf(err, "scan status file %s", file)
}
return status, nil
return unix.Prctl(unix.PR_SET_SECCOMP, unix.SECCOMP_MODE_FILTER, 0, 0, 0) != unix.EINVAL
}

185
common/pkg/seccomp/supported_test.go
View File

@ -1,185 +0,0 @@
// +build seccomp
package seccomp
import (
"io/ioutil"
"os"
"testing"
"github.com/stretchr/testify/require"
)
var statusFile = `
Name: bash
Umask: 0022
State: S (sleeping)
Tgid: 17248
Ngid: 0
Pid: 17248
PPid: 17200
TracerPid: 0
Uid: 1000 1000 1000 1000
Gid: 100 100 100 100
FDSize: 256
Groups: 16 33 100
NStgid: 17248
NSpid: 17248
NSpgid: 17248
NSsid: 17200
VmPeak: 131168 kB
VmSize: 131168 kB
VmLck: 0 kB
VmPin: 0 kB
VmHWM: 13484 kB
VmRSS: 13484 kB
RssAnon: 10264 kB
RssFile: 3220 kB
RssShmem: 0 kB
VmData: 10332 kB
VmStk: 136 kB
VmExe: 992 kB
VmLib: 2104 kB
VmPTE: 76 kB
VmPMD: 12 kB
VmSwap: 0 kB
HugetlbPages: 0 kB # 4.4
Threads: 1
SigQ: 0/3067
SigPnd: 0000000000000000
ShdPnd: 0000000000000000
SigBlk: 0000000000010000
SigIgn: 0000000000384004
SigCgt: 000000004b813efb
CapInh: 0000000000000000
CapPrm: 0000000000000000
CapEff: 0000000000000000
CapBnd: ffffffffffffffff
CapAmb: 0000000000000000
NoNewPrivs: 0
Seccomp: 0
Cpus_allowed: 00000001
Cpus_allowed_list: 0
Mems_allowed: 1
Mems_allowed_list: 0
voluntary_ctxt_switches: 150
nonvoluntary_ctxt_switches: 545
`
func TestParseStatusFile(t *testing.T) {
for _, tc := range []struct {
getFilePath func() (string, func())
shouldErr bool
expected map[string]string
}{
{ // success
getFilePath: func() (string, func()) {
tempFile, err := ioutil.TempFile("", "parse-status-file-")
require.Nil(t, err)
// Valid entry
_, err = tempFile.WriteString("Seccomp: 0\n")
require.Nil(t, err)
// Unparsable entry
_, err = tempFile.WriteString("wrong")
require.Nil(t, err)
return tempFile.Name(), func() {
require.Nil(t, os.RemoveAll(tempFile.Name()))
}
},
shouldErr: false,
expected: map[string]string{"Seccomp": "0"},
},
{ // success whole file
getFilePath: func() (string, func()) {
tempFile, err := ioutil.TempFile("", "parse-status-file-")
require.Nil(t, err)
_, err = tempFile.WriteString(statusFile)
require.Nil(t, err)
return tempFile.Name(), func() {
require.Nil(t, os.RemoveAll(tempFile.Name()))
}
},
shouldErr: false,
expected: map[string]string{
"CapAmb": "0000000000000000",
"CapBnd": "ffffffffffffffff",
"CapEff": "0000000000000000",
"CapInh": "0000000000000000",
"CapPrm": "0000000000000000",
"Cpus_allowed": "00000001",
"Cpus_allowed_list": "0",
"FDSize": "256",
"Gid": "100 100 100 100",
"Groups": "16 33 100",
"HugetlbPages": "0 kB # 4.4",
"Mems_allowed": "1",
"Mems_allowed_list": "0",
"NSpgid": "17248",
"NSpid": "17248",
"NSsid": "17200",
"NStgid": "17248",
"Name": "bash",
"Ngid": "0",
"NoNewPrivs": "0",
"PPid": "17200",
"Pid": "17248",
"RssAnon": "10264 kB",
"RssFile": "3220 kB",
"RssShmem": "0 kB",
"Seccomp": "0",
"ShdPnd": "0000000000000000",
"SigBlk": "0000000000010000",
"SigCgt": "000000004b813efb",
"SigIgn": "0000000000384004",
"SigPnd": "0000000000000000",
"SigQ": "0/3067",
"State": "S (sleeping)",
"Tgid": "17248",
"Threads": "1",
"TracerPid": "0",
"Uid": "1000 1000 1000 1000",
"Umask": "0022",
"VmData": "10332 kB",
"VmExe": "992 kB",
"VmHWM": "13484 kB",
"VmLck": "0 kB",
"VmLib": "2104 kB",
"VmPMD": "12 kB",
"VmPTE": "76 kB",
"VmPeak": "131168 kB",
"VmPin": "0 kB",
"VmRSS": "13484 kB",
"VmSize": "131168 kB",
"VmStk": "136 kB",
"VmSwap": "0 kB",
"nonvoluntary_ctxt_switches": "545",
"voluntary_ctxt_switches": "150",
},
},
{ // error opening file
getFilePath: func() (string, func()) {
tempFile, err := ioutil.TempFile("", "parse-status-file-")
require.Nil(t, err)
require.Nil(t, os.RemoveAll(tempFile.Name()))
return tempFile.Name(), func() {}
},
shouldErr: true,
},
} {
filePath, cleanup := tc.getFilePath()
defer cleanup()
res, err := parseStatusFile(filePath)
if tc.shouldErr {
require.NotNil(t, err)
} else {
require.Equal(t, tc.expected, res)
}
}
}