Skip to content

Commit

Permalink
Do not make read-only mounts recursively read-only by default (also u…
Browse files Browse the repository at this point in the history
…pdates Docker client module to v25) (Mirantis#311)

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
  • Loading branch information
AkihiroSuda authored Mar 14, 2024
1 parent 9a8a9fe commit c2e3805
Show file tree
Hide file tree
Showing 137 changed files with 3,407 additions and 1,307 deletions.
30 changes: 27 additions & 3 deletions core/container_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ import (
"path/filepath"

"github.com/Mirantis/cri-dockerd/libdocker"
"github.com/docker/docker/api/types"
dockerbackend "github.com/docker/docker/api/types/backend"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/strslice"
"github.com/opencontainers/selinux/go-selinux/label"
v1 "k8s.io/cri-api/pkg/apis/runtime/v1"
)

Expand Down Expand Up @@ -64,8 +65,9 @@ func (ds *dockerService) CreateContainer(
image = iSpec.Image
}
containerName := makeContainerName(sandboxConfig, config)
mounts := config.GetMounts()
terminationMessagePath, _ := config.Annotations["io.kubernetes.container.terminationMessagePath"]
createConfig := types.ContainerCreateConfig{
createConfig := dockerbackend.ContainerCreateConfig{
Name: containerName,
Config: &container.Config{
Entrypoint: strslice.StrSlice(config.Command),
Expand All @@ -85,13 +87,35 @@ func (ds *dockerService) CreateContainer(
},
},
HostConfig: &container.HostConfig{
Binds: libdocker.GenerateMountBindings(config.GetMounts(), terminationMessagePath),
Mounts: libdocker.GenerateMountBindings(mounts, terminationMessagePath),
RestartPolicy: container.RestartPolicy{
Name: "no",
},
},
}

// Only request relabeling if the pod provides an SELinux context. If the pod
// does not provide an SELinux context relabeling will label the volume with
// the container's randomly allocated MCS label. This would restrict access
// to the volume to the container which mounts it first.
if selinuxOpts := config.GetLinux().GetSecurityContext().GetSelinuxOptions(); selinuxOpts != nil {
mountLabel, err := selinuxMountLabel(selinuxOpts)
if err != nil {
return nil, fmt.Errorf("unable to generate SELinux mount label: %v", err)
}
if mountLabel != "" {
// Equates to "Z" in the old bind API
const shared = false
for _, m := range mounts {
if m.SelinuxRelabel {
if err := label.Relabel(m.HostPath, mountLabel, shared); err != nil {
return nil, fmt.Errorf("unable to relabel %q with %q: %v", m.HostPath, mountLabel, err)
}
}
}
}
}

hc := createConfig.HostConfig
err = ds.updateCreateConfig(
&createConfig,
Expand Down
8 changes: 5 additions & 3 deletions core/helpers_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ package core

import (
"fmt"

"github.com/blang/semver"
dockertypes "github.com/docker/docker/api/types"
dockerbackend "github.com/docker/docker/api/types/backend"
dockercontainer "github.com/docker/docker/api/types/container"
"github.com/sirupsen/logrus"
runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
Expand All @@ -34,7 +36,7 @@ func DefaultMemorySwap() int64 {
}

func (ds *dockerService) updateCreateConfig(
createConfig *dockertypes.ContainerCreateConfig,
createConfig *dockerbackend.ContainerCreateConfig,
config *runtimeapi.ContainerConfig,
sandboxConfig *runtimeapi.PodSandboxConfig,
podSandboxID string, securityOptSep rune, apiVersion *semver.Version) error {
Expand Down Expand Up @@ -97,12 +99,12 @@ func getNetworkNamespace(c *dockertypes.ContainerJSON) (string, error) {

type containerCleanupInfo struct{}

// applyPlatformSpecificDockerConfig applies platform-specific configurations to a dockertypes.ContainerCreateConfig struct.
// applyPlatformSpecificDockerConfig applies platform-specific configurations to a dockerbackend.ContainerCreateConfig struct.
// The containerCleanupInfo struct it returns will be passed as is to performPlatformSpecificContainerCleanup
// after either the container creation has failed or the container has been removed.
func (ds *dockerService) applyPlatformSpecificDockerConfig(
*runtimeapi.CreateContainerRequest,
*dockertypes.ContainerCreateConfig,
*dockerbackend.ContainerCreateConfig,
) (*containerCleanupInfo, error) {
return nil, nil
}
Expand Down
22 changes: 12 additions & 10 deletions core/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (
"github.com/Mirantis/cri-dockerd/libdocker"

dockertypes "github.com/docker/docker/api/types"
dockermount "github.com/docker/docker/api/types/mount"
dockerregistry "github.com/docker/docker/api/types/registry"
dockernat "github.com/docker/go-connections/nat"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -140,7 +142,7 @@ func TestParsingCreationConflictError(t *testing.T) {

func TestEnsureSandboxImageExists(t *testing.T) {
sandboxImage := "gcr.io/test/image"
authConfig := dockertypes.AuthConfig{Username: "user", Password: "pass"}
authConfig := dockerregistry.AuthConfig{Username: "user", Password: "pass"}
for desc, test := range map[string]struct {
injectImage bool
imgNeedsAuth bool
Expand Down Expand Up @@ -326,15 +328,15 @@ func TestGenerateMountBindings(t *testing.T) {
Propagation: runtimeapi.MountPropagation_PROPAGATION_BIDIRECTIONAL,
},
}
expectedResult := []string{
"/mnt/1:/var/lib/mysql/1",
"/mnt/2:/var/lib/mysql/2:ro",
"/mnt/3:/var/lib/mysql/3:Z",
"/mnt/4:/var/lib/mysql/4",
"/mnt/5:/var/lib/mysql/5:rslave",
"/mnt/6:/var/lib/mysql/6:rshared",
"/mnt/7:/var/lib/mysql/7",
"/mnt/8:/var/lib/mysql/8:ro,Z,rshared",
expectedResult := []dockermount.Mount{
{Type: dockermount.TypeBind, Source: "/mnt/1", Target: "/var/lib/mysql/1", BindOptions: &dockermount.BindOptions{CreateMountpoint: true}},
{Type: dockermount.TypeBind, Source: "/mnt/2", Target: "/var/lib/mysql/2", ReadOnly: true, BindOptions: &dockermount.BindOptions{CreateMountpoint: true, ReadOnlyNonRecursive: true}},
{Type: dockermount.TypeBind, Source: "/mnt/3", Target: "/var/lib/mysql/3", BindOptions: &dockermount.BindOptions{CreateMountpoint: true}}, // Relabeling is not handled here
{Type: dockermount.TypeBind, Source: "/mnt/4", Target: "/var/lib/mysql/4", BindOptions: &dockermount.BindOptions{CreateMountpoint: true}},
{Type: dockermount.TypeBind, Source: "/mnt/5", Target: "/var/lib/mysql/5", BindOptions: &dockermount.BindOptions{CreateMountpoint: true, Propagation: dockermount.PropagationRSlave}},
{Type: dockermount.TypeBind, Source: "/mnt/6", Target: "/var/lib/mysql/6", BindOptions: &dockermount.BindOptions{CreateMountpoint: true, Propagation: dockermount.PropagationRShared}},
{Type: dockermount.TypeBind, Source: "/mnt/7", Target: "/var/lib/mysql/7", BindOptions: &dockermount.BindOptions{CreateMountpoint: true}},
{Type: dockermount.TypeBind, Source: "/mnt/8", Target: "/var/lib/mysql/8", ReadOnly: true, BindOptions: &dockermount.BindOptions{CreateMountpoint: true, ReadOnlyNonRecursive: true, Propagation: dockermount.PropagationRShared}}, // Relabeling is not handled here
}
result := libdocker.GenerateMountBindings(mounts, "")

Expand Down
7 changes: 4 additions & 3 deletions core/helpers_unsupported.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"github.com/blang/semver"
dockertypes "github.com/docker/docker/api/types"
dockerbackend "github.com/docker/docker/api/types/backend"
"github.com/sirupsen/logrus"
runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
)
Expand All @@ -47,7 +48,7 @@ func (ds *dockerService) getSandBoxSecurityOpts(separator rune) []string {
}

func (ds *dockerService) updateCreateConfig(
createConfig *dockertypes.ContainerCreateConfig,
createConfig *dockerbackend.ContainerCreateConfig,
config *runtimeapi.ContainerConfig,
sandboxConfig *runtimeapi.PodSandboxConfig,
podSandboxID string, securityOptSep rune, apiVersion *semver.Version) error {
Expand All @@ -66,12 +67,12 @@ func getNetworkNamespace(c *dockertypes.ContainerJSON) (string, error) {

type containerCleanupInfo struct{}

// applyPlatformSpecificDockerConfig applies platform-specific configurations to a dockertypes.ContainerCreateConfig struct.
// applyPlatformSpecificDockerConfig applies platform-specific configurations to a dockerbackend.ContainerCreateConfig struct.
// The containerCleanupInfo struct it returns will be passed as is to performPlatformSpecificContainerCleanup
// after either the container creation has failed or the container has been removed.
func (ds *dockerService) applyPlatformSpecificDockerConfig(
*runtimeapi.CreateContainerRequest,
*dockertypes.ContainerCreateConfig,
*dockerbackend.ContainerCreateConfig,
) (*containerCleanupInfo, error) {
return nil, nil
}
Expand Down
12 changes: 7 additions & 5 deletions core/helpers_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@ import (
"crypto/rand"
"encoding/hex"
"fmt"
"golang.org/x/sys/windows/registry"
"os"
"regexp"
"runtime"

"golang.org/x/sys/windows/registry"

"github.com/blang/semver"
dockertypes "github.com/docker/docker/api/types"
dockerbackend "github.com/docker/docker/api/types/backend"
dockercontainer "github.com/docker/docker/api/types/container"
dockerfilters "github.com/docker/docker/api/types/filters"
"github.com/sirupsen/logrus"
Expand All @@ -42,7 +44,7 @@ func DefaultMemorySwap() int64 {
}

func (ds *dockerService) updateCreateConfig(
createConfig *dockertypes.ContainerCreateConfig,
createConfig *dockerbackend.ContainerCreateConfig,
config *runtimeapi.ContainerConfig,
sandboxConfig *runtimeapi.PodSandboxConfig,
podSandboxID string, securityOptSep rune, apiVersion *semver.Version) error {
Expand Down Expand Up @@ -147,12 +149,12 @@ type containerCleanupInfo struct {
gMSARegistryValueName string
}

// applyPlatformSpecificDockerConfig applies platform-specific configurations to a dockertypes.ContainerCreateConfig struct.
// applyPlatformSpecificDockerConfig applies platform-specific configurations to a dockerbackend.ContainerCreateConfig struct.
// The containerCleanupInfo struct it returns will be passed as is to performPlatformSpecificContainerCleanup
// after either the container creation has failed or the container has been removed.
func (ds *dockerService) applyPlatformSpecificDockerConfig(
request *runtimeapi.CreateContainerRequest,
createConfig *dockertypes.ContainerCreateConfig,
createConfig *dockerbackend.ContainerCreateConfig,
) (*containerCleanupInfo, error) {
cleanupInfo := &containerCleanupInfo{}

Expand All @@ -173,7 +175,7 @@ func (ds *dockerService) applyPlatformSpecificDockerConfig(
// https://github.com/moby/moby/pull/38777
func applyGMSAConfig(
config *runtimeapi.ContainerConfig,
createConfig *dockertypes.ContainerCreateConfig,
createConfig *dockerbackend.ContainerCreateConfig,
cleanupInfo *containerCleanupInfo,
) error {
var credSpec string
Expand Down
3 changes: 2 additions & 1 deletion core/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

dockertypes "github.com/docker/docker/api/types"
dockerfilters "github.com/docker/docker/api/types/filters"
dockerregistry "github.com/docker/docker/api/types/registry"
"github.com/docker/docker/pkg/jsonmessage"

"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -137,7 +138,7 @@ func (ds *dockerService) PullImage(
) (*runtimeapi.PullImageResponse, error) {
image := r.GetImage()
auth := r.GetAuth()
authConfig := dockertypes.AuthConfig{}
authConfig := dockerregistry.AuthConfig{}

if auth != nil {
authConfig.Username = auth.Username
Expand Down
23 changes: 13 additions & 10 deletions core/sandbox_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,20 @@ package core

import (
"fmt"
"os"
"strings"
"time"

"github.com/Mirantis/cri-dockerd/libdocker"
"github.com/Mirantis/cri-dockerd/utils"
"github.com/Mirantis/cri-dockerd/utils/errors"
"k8s.io/kubernetes/pkg/credentialprovider"
"os"
"strings"
"time"

"github.com/Mirantis/cri-dockerd/config"
dockertypes "github.com/docker/docker/api/types"
dockerbackend "github.com/docker/docker/api/types/backend"
dockercontainer "github.com/docker/docker/api/types/container"
dockerregistry "github.com/docker/docker/api/types/registry"
"github.com/sirupsen/logrus"
runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
)
Expand Down Expand Up @@ -169,7 +172,7 @@ func (ds *dockerService) getPodSandboxDetails(
func (ds *dockerService) applySandboxLinuxOptions(
hc *dockercontainer.HostConfig,
lc *runtimeapi.LinuxPodSandboxConfig,
createConfig *dockertypes.ContainerCreateConfig,
createConfig *dockerbackend.ContainerCreateConfig,
image string,
separator rune,
) error {
Expand Down Expand Up @@ -207,11 +210,11 @@ func (ds *dockerService) applySandboxResources(
return nil
}

// makeSandboxDockerConfig returns dockertypes.ContainerCreateConfig based on runtimeapi.PodSandboxConfig.
// makeSandboxDockerConfig returns dockerbackend.ContainerCreateConfig based on runtimeapi.PodSandboxConfig.
func (ds *dockerService) makeSandboxDockerConfig(
c *runtimeapi.PodSandboxConfig,
image string,
) (*dockertypes.ContainerCreateConfig, error) {
) (*dockerbackend.ContainerCreateConfig, error) {
// Merge annotations and labels because docker supports only labels.
labels := makeLabels(c.GetLabels(), c.GetAnnotations())
// Apply a label to distinguish sandboxes from regular containers.
Expand All @@ -222,7 +225,7 @@ func (ds *dockerService) makeSandboxDockerConfig(
hc := &dockercontainer.HostConfig{
IpcMode: dockercontainer.IpcMode("shareable"),
}
createConfig := &dockertypes.ContainerCreateConfig{
createConfig := &dockerbackend.ContainerCreateConfig{
Name: makeSandboxName(c),
Config: &dockercontainer.Config{
Hostname: c.Hostname,
Expand Down Expand Up @@ -374,7 +377,7 @@ func rewriteFile(filePath, stringToWrite string) error {

func recoverFromCreationConflictIfNeeded(
client libdocker.DockerClientInterface,
createConfig dockertypes.ContainerCreateConfig,
createConfig dockerbackend.ContainerCreateConfig,
err error,
) (*dockercontainer.CreateResponse, error) {
matches := conflictRE.FindStringSubmatch(err.Error())
Expand Down Expand Up @@ -421,7 +424,7 @@ func ensureSandboxImageExists(client libdocker.DockerClientInterface, image stri
if !withCredentials {
logrus.Infof("Pulling the image without credentials. Image: %v", image)

err := client.PullImage(image, dockertypes.AuthConfig{}, dockertypes.ImagePullOptions{})
err := client.PullImage(image, dockerregistry.AuthConfig{}, dockertypes.ImagePullOptions{})
if err != nil {
return fmt.Errorf("failed pulling image %q: %v", image, err)
}
Expand All @@ -431,7 +434,7 @@ func ensureSandboxImageExists(client libdocker.DockerClientInterface, image stri

var pullErrs []error
for _, currentCreds := range creds {
authConfig := dockertypes.AuthConfig(currentCreds)
authConfig := dockerregistry.AuthConfig(currentCreds)
err := client.PullImage(image, authConfig, dockertypes.ImagePullOptions{})
// If there was no error, return success
if err == nil {
Expand Down
24 changes: 24 additions & 0 deletions core/selinux_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package core
import (
"fmt"

"github.com/opencontainers/selinux/go-selinux/label"
runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
)

Expand Down Expand Up @@ -88,3 +89,26 @@ func modifySecurityOption(config []string, name, value string) []string {

return config
}

func selinuxMountLabel(selinuxOpts *runtimeapi.SELinuxOption) (string, error) {
var opts []string
if selinuxOpts != nil {
if selinuxOpts.User != "" {
opts = append(opts, "user:"+selinuxOpts.User)
}
if selinuxOpts.Role != "" {
opts = append(opts, "role:"+selinuxOpts.Role)
}
if selinuxOpts.Type != "" {
opts = append(opts, "type:"+selinuxOpts.Type)
}
if selinuxOpts.Level != "" {
opts = append(opts, "level:"+selinuxOpts.Level)
}
}
if len(opts) == 0 {
opts = []string{"disable"}
}
_, s, err := label.InitLabels(opts)
return s, err
}
15 changes: 15 additions & 0 deletions core/selinux_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ package core
import (
"reflect"
"testing"

"github.com/opencontainers/selinux/go-selinux"
"github.com/stretchr/testify/assert"
)

func TestModifySecurityOptions(t *testing.T) {
Expand Down Expand Up @@ -57,3 +60,15 @@ func TestModifySecurityOptions(t *testing.T) {
}
}
}

func TestSELinuxMountLabel(t *testing.T) {
selinuxOpts := inputSELinuxOptions()
s, err := selinuxMountLabel(selinuxOpts)
assert.NoError(t, err)
t.Logf("mount label for %+v=%q", selinuxOpts, s)
if selinux.GetEnabled() {
assert.NotEmpty(t, s)
} else {
assert.Empty(t, s)
}
}
Loading

0 comments on commit c2e3805

Please sign in to comment.