Skip to content

Commit

Permalink
PodMounter: Add group read access to credentials if Pod is not root (#…
Browse files Browse the repository at this point in the history
…388)

*Issue:* #108

#### Description

- **Problem**: If `fsGroup` security context is applied on Mountpoint
Pod and it's not run as default root user (`0`), then currently
Mountpoint Pod fails to read credentials that are provided by CSI Driver
Node Pod.
- **Solution**: Adding group read permissions to credentials
folder/files if parent folder has non-zero gid.
- **Implementation**: 
  - Added logic to check gid of parent folder in Mount() method
- Added new `CredentialDirPerm` and `CredentialFilePerm` fields to
credential's `ProvideContext`
  - Use these fields when creating credential files/directory
- **Manual Testing**: Successfully validated this fix in personal ROSA
cluster.


By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
  • Loading branch information
yerzhan7 authored Feb 28, 2025
1 parent 7d8c988 commit 24a0ffc
Show file tree
Hide file tree
Showing 12 changed files with 226 additions and 35 deletions.
2 changes: 1 addition & 1 deletion pkg/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func NewDriver(endpoint string, mpVersion string, nodeID string) (*Driver, error
klog.Fatalf("Failed to start Pod watcher: %v\n", err)
}

mounterImpl, err = mounter.NewPodMounter(podWatcher, credProvider, mount.New(""), nil, kubernetesVersion)
mounterImpl, err = mounter.NewPodMounter(podWatcher, credProvider, mount.New(""), nil, nil, kubernetesVersion)
if err != nil {
klog.Fatalln(err)
}
Expand Down
18 changes: 10 additions & 8 deletions pkg/driver/node/credentialprovider/awsprofile/aws_profile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@ import (

"github.com/awslabs/aws-s3-csi-driver/pkg/driver/node/credentialprovider/awsprofile"
"github.com/awslabs/aws-s3-csi-driver/pkg/driver/node/credentialprovider/awsprofile/awsprofiletest"
"github.com/awslabs/aws-s3-csi-driver/pkg/util"
"github.com/awslabs/aws-s3-csi-driver/pkg/util/testutil/assert"
)

const testAccessKeyId = "test-access-key-id"
const testSecretAccessKey = "test-secret-access-key"
const testSessionToken = "test-session-token"
const testFilePerm = fs.FileMode(0600)
const testFilePerm = util.FileModeUserReadWrite

func TestCreatingAWSProfile(t *testing.T) {
defaultSettings := awsprofile.Settings{
Expand All @@ -32,7 +33,7 @@ func TestCreatingAWSProfile(t *testing.T) {
}
profile, err := awsprofile.Create(defaultSettings, creds)
assert.NoError(t, err)
assertCredentialsFromAWSProfile(t, defaultSettings.Basepath, profile, testAccessKeyId, testSecretAccessKey, testSessionToken)
assertCredentialsFromAWSProfile(t, defaultSettings, profile, testAccessKeyId, testSecretAccessKey, testSessionToken)
})

t.Run("create config and credentials files with empty session token", func(t *testing.T) {
Expand All @@ -42,7 +43,7 @@ func TestCreatingAWSProfile(t *testing.T) {
}
profile, err := awsprofile.Create(defaultSettings, creds)
assert.NoError(t, err)
assertCredentialsFromAWSProfile(t, defaultSettings.Basepath, profile, testAccessKeyId, testSecretAccessKey, "")
assertCredentialsFromAWSProfile(t, defaultSettings, profile, testAccessKeyId, testSecretAccessKey, "")
})

t.Run("ensure config and credentials files are created with correct permissions", func(t *testing.T) {
Expand All @@ -53,7 +54,7 @@ func TestCreatingAWSProfile(t *testing.T) {
}
profile, err := awsprofile.Create(defaultSettings, creds)
assert.NoError(t, err)
assertCredentialsFromAWSProfile(t, defaultSettings.Basepath, profile, testAccessKeyId, testSecretAccessKey, testSessionToken)
assertCredentialsFromAWSProfile(t, defaultSettings, profile, testAccessKeyId, testSecretAccessKey, testSessionToken)

configStat, err := os.Stat(filepath.Join(defaultSettings.Basepath, profile.ConfigFilename))
assert.NoError(t, err)
Expand Down Expand Up @@ -111,7 +112,7 @@ func TestCleaningUpAWSProfile(t *testing.T) {

profile, err := awsprofile.Create(settings, creds)
assert.NoError(t, err)
assertCredentialsFromAWSProfile(t, settings.Basepath, profile, testAccessKeyId, testSecretAccessKey, testSessionToken)
assertCredentialsFromAWSProfile(t, settings, profile, testAccessKeyId, testSecretAccessKey, testSessionToken)

err = awsprofile.Cleanup(settings)
assert.NoError(t, err)
Expand All @@ -129,12 +130,13 @@ func TestCleaningUpAWSProfile(t *testing.T) {
})
}

func assertCredentialsFromAWSProfile(t *testing.T, basepath string, profile awsprofile.Profile, accessKeyID string, secretAccessKey string, sessionToken string) {
func assertCredentialsFromAWSProfile(t *testing.T, settings awsprofile.Settings, profile awsprofile.Profile, accessKeyID string, secretAccessKey string, sessionToken string) {
awsprofiletest.AssertCredentialsFromAWSProfile(
t,
profile.Name,
filepath.Join(basepath, profile.ConfigFilename),
filepath.Join(basepath, profile.CredentialsFilename),
settings.FilePerm,
filepath.Join(settings.Basepath, profile.ConfigFilename),
filepath.Join(settings.Basepath, profile.CredentialsFilename),
accessKeyID,
secretAccessKey,
sessionToken,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package awsprofiletest

import (
"context"
"os"
"testing"

"github.com/aws/aws-sdk-go-v2/aws"
Expand All @@ -11,15 +12,24 @@ import (
"github.com/awslabs/aws-s3-csi-driver/pkg/util/testutil/assert"
)

func AssertCredentialsFromAWSProfile(t *testing.T, profileName, configFile, credentialsFile, accessKeyID, secretAccessKey, sessionToken string) {
func AssertCredentialsFromAWSProfile(t *testing.T, profileName string, filePerm os.FileMode, configFile, credentialsFile, accessKeyID, secretAccessKey, sessionToken string) {
t.Helper()

assertCredentialFilePermissions(t, configFile, filePerm)
assertCredentialFilePermissions(t, credentialsFile, filePerm)

credentials := parseAWSProfile(t, profileName, configFile, credentialsFile)
assert.Equals(t, accessKeyID, credentials.AccessKeyID)
assert.Equals(t, secretAccessKey, credentials.SecretAccessKey)
assert.Equals(t, sessionToken, credentials.SessionToken)
}

func assertCredentialFilePermissions(t *testing.T, file string, filePerm os.FileMode) {
fileInfo, err := os.Stat(file)
assert.NoError(t, err)
assert.Equals(t, filePerm, fileInfo.Mode().Perm())
}

func parseAWSProfile(t *testing.T, profileName, configFile, credentialsFile string) aws.Credentials {
sharedConfig, err := config.LoadSharedConfigProfile(context.Background(), profileName, func(c *config.LoadSharedConfigOptions) {
c.ConfigFiles = []string{configFile}
Expand Down
17 changes: 9 additions & 8 deletions pkg/driver/node/credentialprovider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,6 @@ import (
"github.com/awslabs/aws-s3-csi-driver/pkg/driver/node/envprovider"
)

// CredentialFilePerm is the default permissions to be used for credential files.
// It's only readable and writeable by the owner.
const CredentialFilePerm = fs.FileMode(0600)

// CredentialDirPerm is the default permissions to be used for credential directories.
// It's only readable, listable (execute bit), and writeable by the owner.
const CredentialDirPerm = fs.FileMode(0700)

// An AuthenticationSource represents the source (i.e., driver-level or pod-level) where the credentials was obtained.
type AuthenticationSource = string

Expand Down Expand Up @@ -51,6 +43,9 @@ type Provider struct {
// access some path visible from both the CSI Driver Node Pod and Mountpoint, and setups files in that volume
// using [WritePath] and returns paths to these files in [EnvPath], so Mountpoint can correctly read these files.
type ProvideContext struct {
CredentialDirPerm fs.FileMode
CredentialFilePerm fs.FileMode

// WritePath is basepath to write credentials into.
WritePath string
// EnvPath is basepath to use while creating environment variables to pass Mountpoint.
Expand All @@ -70,6 +65,12 @@ type ProvideContext struct {
BucketRegion string
}

// SetCredentialPerm sets credential permissions for credential file and directory for `ctx`
func (ctx *ProvideContext) SetCredentialPerm(credentialDirPerm, credentialFilePerm fs.FileMode) {
ctx.CredentialDirPerm = credentialDirPerm
ctx.CredentialFilePerm = credentialFilePerm
}

// SetWriteAndEnvPath sets `WritePath` and `EnvPath` for `ctx`.
func (ctx *ProvideContext) SetWriteAndEnvPath(writePath, envPath string) {
ctx.WritePath = writePath
Expand Down
4 changes: 2 additions & 2 deletions pkg/driver/node/credentialprovider/provider_driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (c *Provider) cleanupFromDriver(cleanupCtx CleanupContext) error {
func provideStsWebIdentityCredentialsFromDriver(provideCtx ProvideContext) (envprovider.Environment, error) {
driverServiceAccountTokenFile := os.Getenv(envprovider.EnvWebIdentityTokenFile)
tokenFile := filepath.Join(provideCtx.WritePath, driverLevelServiceAccountTokenName)
err := util.ReplaceFile(tokenFile, driverServiceAccountTokenFile, CredentialFilePerm)
err := util.ReplaceFile(tokenFile, driverServiceAccountTokenFile, provideCtx.CredentialFilePerm)
if err != nil {
return nil, fmt.Errorf("credentialprovider: sts-web-identity: failed to copy driver's service account token: %w", err)
}
Expand All @@ -94,7 +94,7 @@ func provideLongTermCredentialsFromDriver(provideCtx ProvideContext, accessKeyID
awsProfile, err := awsprofile.Create(awsprofile.Settings{
Basepath: provideCtx.WritePath,
Prefix: prefix,
FilePerm: CredentialFilePerm,
FilePerm: provideCtx.CredentialFilePerm,
}, awsprofile.Credentials{
AccessKeyID: accessKeyID,
SecretAccessKey: secretAccessKey,
Expand Down
2 changes: 1 addition & 1 deletion pkg/driver/node/credentialprovider/provider_pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (c *Provider) provideFromPod(ctx context.Context, provideCtx ProvideContext

tokenName := podLevelServiceAccountTokenName(podID, volumeID)

err = renameio.WriteFile(filepath.Join(provideCtx.WritePath, tokenName), []byte(stsToken.Token), CredentialFilePerm)
err = renameio.WriteFile(filepath.Join(provideCtx.WritePath, tokenName), []byte(stsToken.Token), provideCtx.CredentialFilePerm)
if err != nil {
return nil, status.Errorf(codes.Internal, "Failed to write service account token: %v", err)
}
Expand Down
28 changes: 20 additions & 8 deletions pkg/driver/node/credentialprovider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/awslabs/aws-s3-csi-driver/pkg/driver/node/credentialprovider"
"github.com/awslabs/aws-s3-csi-driver/pkg/driver/node/credentialprovider/awsprofile/awsprofiletest"
"github.com/awslabs/aws-s3-csi-driver/pkg/driver/node/envprovider"
"github.com/awslabs/aws-s3-csi-driver/pkg/util"
"github.com/awslabs/aws-s3-csi-driver/pkg/util/testutil"
"github.com/awslabs/aws-s3-csi-driver/pkg/util/testutil/assert"
)
Expand All @@ -25,6 +26,8 @@ const testAccessKeyID = "test-access-key-id"
const testSecretAccessKey = "test-secret-access-key"
const testSessionToken = "test-session-token"

const testCredentialFilePerm = util.FileModeUserReadWrite

const testRoleARN = "arn:aws:iam::111122223333:role/pod-a-role"
const testWebIdentityToken = "test-web-identity-token"

Expand Down Expand Up @@ -61,6 +64,7 @@ func TestProvidingDriverLevelCredentials(t *testing.T) {

writePath := t.TempDir()
provideCtx := credentialprovider.ProvideContext{
CredentialFilePerm: testCredentialFilePerm,
AuthenticationSource: authSource,
WritePath: writePath,
EnvPath: testEnvPath,
Expand All @@ -76,7 +80,7 @@ func TestProvidingDriverLevelCredentials(t *testing.T) {
"AWS_CONFIG_FILE": "/test-env/" + testProfilePrefix + "s3-csi-config",
"AWS_SHARED_CREDENTIALS_FILE": "/test-env/" + testProfilePrefix + "s3-csi-credentials",
}, env)
assertLongTermCredentials(t, writePath)
assertLongTermCredentials(t, provideCtx)
}
})

Expand All @@ -86,6 +90,7 @@ func TestProvidingDriverLevelCredentials(t *testing.T) {

writePath := t.TempDir()
provideCtx := credentialprovider.ProvideContext{
CredentialFilePerm: testCredentialFilePerm,
AuthenticationSource: authSource,
WritePath: writePath,
EnvPath: testEnvPath,
Expand Down Expand Up @@ -133,6 +138,7 @@ func TestProvidingDriverLevelCredentials(t *testing.T) {

writePath := t.TempDir()
provideCtx := credentialprovider.ProvideContext{
CredentialFilePerm: testCredentialFilePerm,
AuthenticationSource: authSource,
WritePath: writePath,
EnvPath: testEnvPath,
Expand All @@ -150,7 +156,7 @@ func TestProvidingDriverLevelCredentials(t *testing.T) {
"AWS_ROLE_ARN": testRoleARN,
"AWS_WEB_IDENTITY_TOKEN_FILE": filepath.Join(testEnvPath, testDriverLevelServiceAccountToken),
}, env)
assertLongTermCredentials(t, writePath)
assertLongTermCredentials(t, provideCtx)
assertWebIdentityTokenFile(t, filepath.Join(writePath, testDriverLevelServiceAccountToken))
}
})
Expand Down Expand Up @@ -205,7 +211,7 @@ func TestProvidingDriverLevelCredentials(t *testing.T) {

// Only set token file without role ARN
tokenPath := filepath.Join(t.TempDir(), "token")
assert.NoError(t, os.WriteFile(tokenPath, []byte(testWebIdentityToken), 0600))
assert.NoError(t, os.WriteFile(tokenPath, []byte(testWebIdentityToken), testCredentialFilePerm))
t.Setenv("AWS_ROLE_ARN", "")
t.Setenv("AWS_WEB_IDENTITY_TOKEN_FILE", tokenPath)

Expand Down Expand Up @@ -245,6 +251,7 @@ func TestProvidingPodLevelCredentials(t *testing.T) {

writePath := t.TempDir()
provideCtx := credentialprovider.ProvideContext{
CredentialFilePerm: testCredentialFilePerm,
AuthenticationSource: credentialprovider.AuthenticationSourcePod,
WritePath: writePath,
EnvPath: testEnvPath,
Expand Down Expand Up @@ -580,6 +587,7 @@ func TestProvidingPodLevelCredentialsForDifferentPods(t *testing.T) {
provider := credentialprovider.New(clientset.CoreV1(), dummyRegionProvider)

baseProvideCtx := credentialprovider.ProvideContext{
CredentialFilePerm: testCredentialFilePerm,
AuthenticationSource: credentialprovider.AuthenticationSourcePod,
WritePath: t.TempDir(),
EnvPath: testEnvPath,
Expand Down Expand Up @@ -647,6 +655,7 @@ func TestProvidingPodLevelCredentialsWithSlashInIDs(t *testing.T) {
provider := credentialprovider.New(clientset.CoreV1(), dummyRegionProvider)

baseProvideCtx := credentialprovider.ProvideContext{
CredentialFilePerm: testCredentialFilePerm,
AuthenticationSource: credentialprovider.AuthenticationSourcePod,
WritePath: t.TempDir(),
EnvPath: testEnvPath,
Expand Down Expand Up @@ -716,6 +725,7 @@ func TestCleanup(t *testing.T) {
provider := credentialprovider.New(nil, dummyRegionProvider)
writePath := t.TempDir()
provideCtx := credentialprovider.ProvideContext{
CredentialFilePerm: testCredentialFilePerm,
AuthenticationSource: credentialprovider.AuthenticationSourceDriver,
WritePath: writePath,
EnvPath: testEnvPath,
Expand All @@ -731,7 +741,7 @@ func TestCleanup(t *testing.T) {
"AWS_CONFIG_FILE": "/test-env/" + testProfilePrefix + "s3-csi-config",
"AWS_SHARED_CREDENTIALS_FILE": "/test-env/" + testProfilePrefix + "s3-csi-credentials",
}, env)
assertLongTermCredentials(t, writePath)
assertLongTermCredentials(t, provideCtx)

// Perform cleanup
err = provider.Cleanup(credentialprovider.CleanupContext{
Expand Down Expand Up @@ -764,6 +774,7 @@ func TestCleanup(t *testing.T) {

writePath := t.TempDir()
provideCtx := credentialprovider.ProvideContext{
CredentialFilePerm: testCredentialFilePerm,
AuthenticationSource: credentialprovider.AuthenticationSourcePod,
WritePath: writePath,
EnvPath: testEnvPath,
Expand Down Expand Up @@ -823,14 +834,15 @@ func setEnvForLongTermCredentials(t *testing.T) {
t.Setenv("AWS_SESSION_TOKEN", testSessionToken)
}

func assertLongTermCredentials(t *testing.T, basepath string) {
func assertLongTermCredentials(t *testing.T, ctx credentialprovider.ProvideContext) {
t.Helper()

awsprofiletest.AssertCredentialsFromAWSProfile(
t,
testProfilePrefix+"s3-csi",
filepath.Join(basepath, testProfilePrefix+"s3-csi-config"),
filepath.Join(basepath, testProfilePrefix+"s3-csi-credentials"),
ctx.CredentialFilePerm,
filepath.Join(ctx.WritePath, testProfilePrefix+"s3-csi-config"),
filepath.Join(ctx.WritePath, testProfilePrefix+"s3-csi-credentials"),
testAccessKeyID,
testSecretAccessKey,
testSessionToken,
Expand All @@ -841,7 +853,7 @@ func setEnvForStsWebIdentityCredentials(t *testing.T) {
t.Helper()

tokenPath := filepath.Join(t.TempDir(), "token")
assert.NoError(t, os.WriteFile(tokenPath, []byte(testWebIdentityToken), 0600))
assert.NoError(t, os.WriteFile(tokenPath, []byte(testWebIdentityToken), testCredentialFilePerm))

t.Setenv("AWS_ROLE_ARN", testRoleARN)
t.Setenv("AWS_WEB_IDENTITY_TOKEN_FILE", tokenPath)
Expand Down
Loading

0 comments on commit 24a0ffc

Please sign in to comment.