From 1e3e99f2fe95c8679f8962b8175038bd7d0558f2 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 8 Jul 2019 18:37:40 -0400 Subject: Move the HostConfig portion of Inspect inside libpod When we first began writing Podman, we ran into a major issue when implementing Inspect. Libpod deliberately does not tie its internal data structures to Docker, and stores most information about containers encoded within the OCI spec. However, Podman must present a CLI compatible with Docker, which means it must expose all the information in 'docker inspect' - most of which is not contained in the OCI spec or libpod's Config struct. Our solution at the time was the create artifact. We JSON'd the complete CreateConfig (a parsed form of the CLI arguments to 'podman run') and stored it with the container, restoring it when we needed to run commands that required the extra info. Over the past month, I've been looking more at Inspect, and refactored large portions of it into Libpod - generating them from what we know about the OCI config and libpod's (now much expanded, versus previously) container configuration. This path comes close to completing the process, moving the last part of inspect into libpod and removing the need for the create artifact. This improves libpod's compatability with non-Podman containers. We no longer require an arbitrarily-formatted JSON blob to be present to run inspect. Fixes: #3500 Signed-off-by: Matthew Heon --- pkg/spec/spec.go | 37 +++++++++++++++++++++++++++++++++++++ pkg/util/utils_linux.go | 43 +++++++++++++++++++++++++++++++++++++++++++ pkg/util/utils_unsupported.go | 12 ++++++++++++ pkg/varlinkapi/containers.go | 24 +----------------------- 4 files changed, 93 insertions(+), 23 deletions(-) create mode 100644 pkg/util/utils_unsupported.go (limited to 'pkg') diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go index 6d8d399f4..ca627f3aa 100644 --- a/pkg/spec/spec.go +++ b/pkg/spec/spec.go @@ -418,6 +418,43 @@ func (config *CreateConfig) createConfigToOCISpec(runtime *libpod.Runtime, userM } } + // Add annotations + if configSpec.Annotations == nil { + configSpec.Annotations = make(map[string]string) + } + + if config.CidFile != "" { + configSpec.Annotations[libpod.InspectAnnotationCIDFile] = config.CidFile + } + + if config.Rm { + configSpec.Annotations[libpod.InspectAnnotationAutoremove] = libpod.InspectResponseTrue + } else { + configSpec.Annotations[libpod.InspectAnnotationAutoremove] = libpod.InspectResponseFalse + } + + if len(config.VolumesFrom) > 0 { + configSpec.Annotations[libpod.InspectAnnotationVolumesFrom] = strings.Join(config.VolumesFrom, ",") + } + + if config.Privileged { + configSpec.Annotations[libpod.InspectAnnotationPrivileged] = libpod.InspectResponseTrue + } else { + configSpec.Annotations[libpod.InspectAnnotationPrivileged] = libpod.InspectResponseFalse + } + + if config.PublishAll { + configSpec.Annotations[libpod.InspectAnnotationPublishAll] = libpod.InspectResponseTrue + } else { + configSpec.Annotations[libpod.InspectAnnotationPublishAll] = libpod.InspectResponseFalse + } + + if config.Init { + configSpec.Annotations[libpod.InspectAnnotationInit] = libpod.InspectResponseTrue + } else { + configSpec.Annotations[libpod.InspectAnnotationInit] = libpod.InspectResponseFalse + } + return configSpec, nil } diff --git a/pkg/util/utils_linux.go b/pkg/util/utils_linux.go index 47fa1031f..318bd2b1b 100644 --- a/pkg/util/utils_linux.go +++ b/pkg/util/utils_linux.go @@ -1,7 +1,14 @@ package util import ( + "fmt" + "os" + "path/filepath" + "syscall" + "github.com/containers/psgo" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" ) // GetContainerPidInformationDescriptors returns a string slice of all supported @@ -9,3 +16,39 @@ import ( func GetContainerPidInformationDescriptors() ([]string, error) { return psgo.ListDescriptors(), nil } + +// FindDeviceNodes parses /dev/ into a set of major:minor -> path, where +// [major:minor] is the device's major and minor numbers formatted as, for +// example, 2:0 and path is the path to the device node. +// Symlinks to nodes are ignored. +func FindDeviceNodes() (map[string]string, error) { + nodes := make(map[string]string) + err := filepath.Walk("/dev", func(path string, info os.FileInfo, err error) error { + if err != nil { + logrus.Warnf("Error descending into path %s: %v", path, err) + return filepath.SkipDir + } + + // If we aren't a device node, do nothing. + if info.Mode()&(os.ModeDevice|os.ModeCharDevice) == 0 { + return nil + } + + // We are a device node. Get major/minor. + sysstat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return errors.Errorf("Could not convert stat output for use") + } + major := uint64(sysstat.Rdev / 256) + minor := uint64(sysstat.Rdev % 256) + + nodes[fmt.Sprintf("%d:%d", major, minor)] = path + + return nil + }) + if err != nil { + return nil, err + } + + return nodes, nil +} diff --git a/pkg/util/utils_unsupported.go b/pkg/util/utils_unsupported.go new file mode 100644 index 000000000..62805d7c8 --- /dev/null +++ b/pkg/util/utils_unsupported.go @@ -0,0 +1,12 @@ +// +build darwin windows + +package util + +import ( + "github.com/pkg/errors" +) + +// FindDeviceNodes is not implemented anywhere except Linux. +func FindDeviceNodes() (map[string]string, error) { + return nil, errors.Errorf("not supported on non-Linux OSes") +} diff --git a/pkg/varlinkapi/containers.go b/pkg/varlinkapi/containers.go index 6855a7231..700e02b0c 100644 --- a/pkg/varlinkapi/containers.go +++ b/pkg/varlinkapi/containers.go @@ -19,7 +19,6 @@ import ( "github.com/containers/libpod/libpod/define" "github.com/containers/libpod/libpod/logs" "github.com/containers/libpod/pkg/adapter/shortcuts" - cc "github.com/containers/libpod/pkg/spec" "github.com/containers/storage/pkg/archive" "github.com/pkg/errors" ) @@ -170,16 +169,7 @@ func (i *LibpodAPI) InspectContainer(call iopodman.VarlinkCall, name string) err if err != nil { return call.ReplyContainerNotFound(name, err.Error()) } - inspectInfo, err := ctr.Inspect(true) - if err != nil { - return call.ReplyErrorOccurred(err.Error()) - } - artifact, err := getArtifact(ctr) - if err != nil { - return call.ReplyErrorOccurred(err.Error()) - } - - data, err := shared.GetCtrInspectInfo(ctr.Config(), inspectInfo, artifact) + data, err := ctr.Inspect(true) if err != nil { return call.ReplyErrorOccurred(err.Error()) } @@ -587,18 +577,6 @@ func (i *LibpodAPI) ContainerRestore(call iopodman.VarlinkCall, name string, kee return call.ReplyContainerRestore(ctr.ID()) } -func getArtifact(ctr *libpod.Container) (*cc.CreateConfig, error) { - var createArtifact cc.CreateConfig - artifact, err := ctr.GetArtifact("create-config") - if err != nil { - return nil, err - } - if err := json.Unmarshal(artifact, &createArtifact); err != nil { - return nil, err - } - return &createArtifact, nil -} - // ContainerConfig returns just the container.config struct func (i *LibpodAPI) ContainerConfig(call iopodman.VarlinkCall, name string) error { ctr, err := i.Runtime.LookupContainer(name) -- cgit v1.2.3-54-g00ecf From c91bc31570f1fab616e10d0e2b4a6c8b7fe631c7 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 16 Jul 2019 15:00:41 -0400 Subject: Populate inspect with security-opt settings We can infer no-new-privileges. For now, manually populate seccomp (can't infer what file we sourced from) and SELinux/Apparmor (hard to tell if they're enabled or not). Signed-off-by: Matthew Heon --- libpod/container_inspect.go | 32 +++++++++++++++++++++++++++++++- pkg/spec/spec.go | 19 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) (limited to 'pkg') diff --git a/libpod/container_inspect.go b/libpod/container_inspect.go index 7273a9005..c4d2af66e 100644 --- a/libpod/container_inspect.go +++ b/libpod/container_inspect.go @@ -58,6 +58,24 @@ const ( // If an annotation with this key is found in the OCI spec, it will be // used in the output of Inspect(). InspectAnnotationInit = "io.podman.annotations.init" + // InspectAnnotationLabel is used by Inspect to identify containers with + // special SELinux-related settings. It is used to populate the output + // of the SecurityOpt setting. + // If an annotation with this key is found in the OCI spec, it will be + // used in the output of Inspect(). + InspectAnnotationLabel = "io.podman.annotations.label" + // InspectAnnotationSeccomp is used by Inspect to identify containers + // with special Seccomp-related settings. It is used to populate the + // output of the SecurityOpt setting in Inspect. + // If an annotation with this key is found in the OCI spec, it will be + // used in the output of Inspect(). + InspectAnnotationSeccomp = "io.podman.annotations.seccomp" + // InspectAnnotationApparmor is used by Inspect to identify containers + // with special Apparmor-related settings. It is used to populate the + // output of the SecurityOpt setting. + // If an annotation with this key is found in the OCI spec, it will be + // used in the output of Inspect(). + InspectAnnotationApparmor = "io.podman.annotations.apparmor" // InspectResponseTrue is a boolean True response for an inspect // annotation. @@ -275,7 +293,6 @@ type InspectContainerHostConfig struct { ReadonlyRootfs bool `json:"ReadonlyRootfs"` // SecurityOpt is a list of security-related options that are set in the // container. - // TODO. SecurityOpt []string `json:"SecurityOpt"` // Tmpfs is a list of tmpfs filesystems that will be mounted into the // container. @@ -965,10 +982,14 @@ func (c *Container) generateInspectContainerHostConfig(ctrSpec *spec.Spec, named hostConfig.GroupAdd = append(hostConfig.GroupAdd, group) } + hostConfig.SecurityOpt = []string{} if ctrSpec.Process != nil { if ctrSpec.Process.OOMScoreAdj != nil { hostConfig.OomScoreAdj = *ctrSpec.Process.OOMScoreAdj } + if ctrSpec.Process.NoNewPrivileges { + hostConfig.SecurityOpt = append(hostConfig.SecurityOpt, "no-new-privileges") + } } hostConfig.ReadonlyRootfs = ctrSpec.Root.Readonly @@ -995,6 +1016,15 @@ func (c *Container) generateInspectContainerHostConfig(ctrSpec *spec.Spec, named if ctrSpec.Annotations[InspectAnnotationInit] == InspectResponseTrue { hostConfig.Init = true } + if label, ok := ctrSpec.Annotations[InspectAnnotationLabel]; ok { + hostConfig.SecurityOpt = append(hostConfig.SecurityOpt, fmt.Sprintf("label=%s", label)) + } + if seccomp, ok := ctrSpec.Annotations[InspectAnnotationSeccomp]; ok { + hostConfig.SecurityOpt = append(hostConfig.SecurityOpt, fmt.Sprintf("seccomp=%s", seccomp)) + } + if apparmor, ok := ctrSpec.Annotations[InspectAnnotationApparmor]; ok { + hostConfig.SecurityOpt = append(hostConfig.SecurityOpt, fmt.Sprintf("apparmor=%s", apparmor)) + } } // Resource limits diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go index ca627f3aa..41054633f 100644 --- a/pkg/spec/spec.go +++ b/pkg/spec/spec.go @@ -455,6 +455,25 @@ func (config *CreateConfig) createConfigToOCISpec(runtime *libpod.Runtime, userM configSpec.Annotations[libpod.InspectAnnotationInit] = libpod.InspectResponseFalse } + for _, opt := range config.SecurityOpts { + // Split on both : and = + splitOpt := strings.Split(opt, "=") + if len(splitOpt) == 1 { + splitOpt = strings.Split(opt, ":") + } + if len(splitOpt) < 2 { + continue + } + switch splitOpt[0] { + case "label": + configSpec.Annotations[libpod.InspectAnnotationLabel] = splitOpt[1] + case "seccomp": + configSpec.Annotations[libpod.InspectAnnotationSeccomp] = splitOpt[1] + case "apparmor": + configSpec.Annotations[libpod.InspectAnnotationApparmor] = splitOpt[1] + } + } + return configSpec, nil } -- cgit v1.2.3-54-g00ecf