Skip to content

Commit 4f19c57

Browse files
Merge pull request #2291 from h2zh/drop-privs-dirs-fix
Handle directories permission denied error in drop privs mode
2 parents c3b0e19 + 6f52b71 commit 4f19c57

8 files changed

Lines changed: 171 additions & 30 deletions

File tree

cmd/generate_password.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"github.com/tg123/go-htpasswd"
3434
"golang.org/x/term"
3535

36+
"github.com/pelicanplatform/pelican/config"
3637
"github.com/pelicanplatform/pelican/param"
3738
"github.com/pelicanplatform/pelican/web_ui"
3839
)
@@ -122,6 +123,15 @@ func passwordMain(cmd *cobra.Command, args []string) error {
122123
}
123124
file.Close()
124125

126+
// chown the file to the pelican user
127+
user, err := config.GetPelicanUser()
128+
if err != nil {
129+
return errors.Wrap(err, "no OS user found for pelican")
130+
}
131+
if err := os.Chown(outPasswordPath, user.Uid, user.Gid); err != nil {
132+
return errors.Wrapf(err, "failed to set ownership on %s", outPasswordPath)
133+
}
134+
125135
_, err = htpasswd.New(outPasswordPath, []htpasswd.PasswdParser{htpasswd.AcceptBcrypt}, nil)
126136
if err != nil {
127137
return err

config/config.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1368,7 +1368,9 @@ func InitServer(ctx context.Context, currentServers server_structs.ServerType) e
13681368
// Set up the directories for the server to run as a non-root user;
13691369
// for the most part, we need to recursively chown and chmod the directory
13701370
// so either root or pelican can access it.
1371-
pelicanLocations := []string{}
1371+
pelicanLocations := []string{
1372+
param.Server_DbLocation.GetString(),
1373+
}
13721374
if currentServers.IsEnabled(server_structs.RegistryType) {
13731375
pelicanLocations = append(pelicanLocations, param.Registry_DbLocation.GetString())
13741376
}
@@ -1390,8 +1392,17 @@ func InitServer(ctx context.Context, currentServers server_structs.ServerType) e
13901392
return errors.Wrap(err, "failure when setting up the file permissions for pelican")
13911393
}
13921394

1395+
pelicanLocationsSingleFiles := []string{
1396+
param.Server_UIPasswordFile.GetString(),
1397+
param.IssuerKey.GetString(), // Backward compatibility for legacy key file
1398+
}
1399+
if err = setFilePerms(pelicanLocationsSingleFiles, 0640, puser.Uid, 0); err != nil {
1400+
return errors.Wrap(err, "failure when setting up the file permissions for pelican")
1401+
}
1402+
13931403
pelicanDirs := []string{
13941404
param.Monitoring_DataLocation.GetString(),
1405+
param.IssuerKeysDirectory.GetString(),
13951406
}
13961407
if currentServers.IsEnabled(server_structs.LocalCacheType) {
13971408
pelicanDirs = append(pelicanDirs, param.LocalCache_RunLocation.GetString())

config/init_server_creds.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,7 @@ func loadPEMFiles(dir string) (jwk.Key, error) {
671671
func GeneratePEM(dir string) (key jwk.Key, err error) {
672672
var fname string
673673
var keyFile *os.File
674+
674675
if dir == "" {
675676
issuerKeyLocation := param.IssuerKey.GetString()
676677
if issuerKeyLocation == "" {
@@ -685,8 +686,7 @@ func GeneratePEM(dir string) (key jwk.Key, err error) {
685686
return
686687
}
687688
} else {
688-
// Generate a unique filename using a POSIX mkstemp-like logic
689-
// Create a temp file, store its filename, then immediately delete this temp file
689+
// Generate a unique filename
690690
filenamePattern := fmt.Sprintf("pelican_generated_%d_*.pem",
691691
time.Now().UnixNano())
692692
if err = createDirForKeys(dir); err != nil {
@@ -695,14 +695,23 @@ func GeneratePEM(dir string) (key jwk.Key, err error) {
695695
}
696696
keyFile, err = os.CreateTemp(dir, filenamePattern)
697697
if err != nil {
698-
err = errors.Wrap(err, "failed to remove temp file")
698+
err = errors.Wrap(err, "failed to create private key file")
699699
return
700700
}
701701
fname = keyFile.Name()
702702
log.Debugln("Generating new private key in the IssuerKeys directory at", fname)
703703
}
704704
defer keyFile.Close()
705705

706+
user, err := GetPelicanUser()
707+
if err != nil {
708+
return nil, errors.Wrap(err, "failed to get pelican user for setting ownership")
709+
}
710+
711+
if err = SetOwnershipAndPermissions(fname, 0640, user); err != nil {
712+
return nil, errors.Wrapf(err, "failed to set ownership and permissions for %s", fname)
713+
}
714+
706715
if err = generatePrivateKeyToFile(keyFile, elliptic.P256()); err != nil {
707716
return nil, errors.Wrapf(err, "failed to generate private key in file %s", fname)
708717
}

config/mkdirall.go

Lines changed: 43 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,28 @@ import (
2828
"github.com/pkg/errors"
2929
)
3030

31+
// Apply the given permission bits and ownership to path in a cross-platform way.
32+
// - On Windows it runs “icacls path /grant username:F”
33+
// - On Linux/macOS it does os.Chmod(path, perm) then os.Chown(path, uid, gid)
34+
func SetOwnershipAndPermissions(path string, perm os.FileMode, user User) error {
35+
if runtime.GOOS == "windows" {
36+
cmd := exec.Command("icacls", path, "/grant", user.Username+":F")
37+
if out, err := cmd.CombinedOutput(); err != nil {
38+
return errors.Wrapf(err, "unable to modify ACLs on %q: %s", path, string(out))
39+
}
40+
return nil
41+
}
42+
43+
// Assume macOS or Linux
44+
if err := os.Chmod(path, perm); err != nil {
45+
return errors.Wrapf(err, "unable to chmod %q to %v", path, perm)
46+
}
47+
if err := os.Chown(path, user.Uid, user.Gid); err != nil {
48+
return errors.Wrapf(err, "unable to chown %q to %d:%d", path, user.Uid, user.Gid)
49+
}
50+
return nil
51+
}
52+
3153
// This is the pelican version of `MkdirAll`; ensures that any created directory
3254
// is owned by a given uid/gid. This allows the created directory to be owned by
3355
// the xrootd user.
@@ -76,26 +98,29 @@ func MkdirAll(path string, perm os.FileMode, uid int, gid int) error {
7698
}
7799

78100
// Set ownership on the directory that we just created.
79-
if runtime.GOOS == "windows" {
80-
username, err := GetDaemonUser() // FIXME (brianaydemir): This is not the correct user.
81-
if err != nil {
82-
return err
83-
}
84-
cmd := exec.Command("icacls", path, "/grant", username+":F")
85-
output, err := cmd.CombinedOutput()
86-
if err != nil {
87-
return errors.Wrapf(err, "unable to modify discretionary ACLs on directory %v: %s", path, string(output))
101+
user, err := GetPelicanUser()
102+
if err != nil {
103+
return errors.Wrap(err, "failed to get pelican user")
104+
}
105+
if err = SetOwnershipAndPermissions(path, perm, User{Uid: uid, Gid: gid, Username: user.Username}); err != nil {
106+
return errors.Wrapf(err, "failed to set ownership and permissions for %s", path)
107+
}
108+
return nil
109+
}
110+
111+
// Set the permissions on the targeted file only if it exists. Doesn't care about its parent or siblings
112+
func setFilePerms(paths []string, perm os.FileMode, uid int, gid int) error {
113+
for _, path := range paths {
114+
// Skip the file if it doesn't exist
115+
if _, err := os.Stat(path); os.IsNotExist(err) {
116+
continue
88117
}
89-
} else { // Assume macOS or Linux.
90-
// Any default system umask may prevent previous application of the permissions
91-
// from taking effect. To override this, set the permissions explicitly
92-
// after creation.
93-
if err = os.Chmod(path, perm); err != nil {
94-
return errors.Wrapf(err, "unable to chmod on directory %v to %v", path, perm)
118+
// Set the permissions on the file
119+
if err := os.Chmod(path, perm); err != nil {
120+
return errors.Wrapf(err, "failed to set permissions on file %v", path)
95121
}
96-
97-
if err = os.Chown(path, uid, gid); err != nil {
98-
return errors.Wrapf(err, "unable to chown on directory %v to %v:%v", path, uid, gid)
122+
if err := os.Chown(path, uid, gid); err != nil {
123+
return errors.Wrapf(err, "failed to chown file %v", path)
99124
}
100125
}
101126
return nil

xrootd/authorization.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,41 @@ func writeScitokensConfiguration(modules server_structs.ServerType, cfg *Scitoke
169169
Funcs(template.FuncMap{"StringsJoin": strings.Join, "JSONify": JSONify}).
170170
Parse(scitokensCfgTemplate))
171171

172+
// If Pelican is run by unprivileged user, we use xrdhttp-pelican plugin to make sure the final scitokens.cfg file is owned by xrootd
173+
if param.Server_DropPrivileges.GetBool() {
174+
// Create a temporary file to assemble the scitokens configuration
175+
tempCfgFile, err := os.CreateTemp("", "scitokens-generated-*.cfg.tmp")
176+
if err != nil {
177+
return errors.Wrapf(err, "failed to create a temporary scitokens file %s", tempCfgFile.Name())
178+
} else {
179+
log.Debugln("Created temporary scitokens config file", tempCfgFile.Name())
180+
}
181+
defer func() {
182+
tempCfgFile.Close()
183+
os.Remove(tempCfgFile.Name())
184+
}()
185+
deduplicateBasePaths(cfg)
186+
err = templ.Execute(tempCfgFile, cfg)
187+
if err != nil {
188+
return errors.Wrapf(err, "unable to create scitokens.cfg template")
189+
}
190+
// After writing configuration to the file, the file pointer remains at the end.
191+
// Seek back to the beginning of the file so that the copy operation reads from the start.
192+
if _, err := tempCfgFile.Seek(0, io.SeekStart); err != nil {
193+
return errors.Wrap(err, "failed to seek to beginning of the file")
194+
}
195+
196+
// Use command "7" in xrdhttp-pelican plugin to transplant the scitoken config file to a
197+
// directory owned by xrootd. The directory is specified in `xrootd/launch.go`. This is because
198+
// the xrootd daemon will periodically reload the scitokens.cfg and, in some cases, we may
199+
// want to update it without restarting the server.
200+
if err = FileCopyToXrootdDir(modules.IsEnabled(server_structs.OriginType), 7, tempCfgFile); err != nil {
201+
return errors.Wrap(err, "failed to copy the scitokens config file to the xrootd directory")
202+
}
203+
204+
return nil
205+
}
206+
172207
gid, err := config.GetDaemonGID()
173208
if err != nil {
174209
return err
@@ -366,6 +401,37 @@ func EmitAuthfile(server server_structs.XRootDServer) error {
366401
}
367402
}
368403

404+
// If Pelican is run by unprivileged user, we use xrdhttp-pelican plugin to make sure the final authfile is owned by xrootd
405+
if param.Server_DropPrivileges.GetBool() {
406+
// Create a temporary authfile
407+
tempAuthFile, err := os.CreateTemp("", "temp-authfile-generated-*")
408+
if err != nil {
409+
return errors.Wrapf(err, "failed to create a generated temporary authfile %s", tempAuthFile.Name())
410+
} else {
411+
log.Debugln("Created temporary authfile", tempAuthFile.Name())
412+
}
413+
defer func() {
414+
tempAuthFile.Close()
415+
os.Remove(tempAuthFile.Name())
416+
}()
417+
if _, err := output.WriteTo(tempAuthFile); err != nil {
418+
return errors.Wrapf(err, "failed to write to generated authfile %v", tempAuthFile.Name())
419+
}
420+
// After writing content to the file, the file pointer remains at the end.
421+
// Seek back to the beginning of the file so that the copy operation reads from the start.
422+
if _, err := tempAuthFile.Seek(0, io.SeekStart); err != nil {
423+
return errors.Wrap(err, "failed to seek to beginning of the file")
424+
}
425+
426+
// Transplant the authfile using the xrdhttp-pelican plugin so xrootd can access it.
427+
// Command "6" instructs the plugin to put the auth file into the designated location owned by "xrootd" user,
428+
// which is specified in `xrootd/launch.go`.
429+
if err = FileCopyToXrootdDir(server.GetServerType().IsEnabled(server_structs.OriginType), 6, tempAuthFile); err != nil {
430+
return errors.Wrap(err, "failed to copy the auth file to the xrootd directory")
431+
}
432+
return nil
433+
}
434+
369435
gid, err := config.GetDaemonGID()
370436
if err != nil {
371437
return err

xrootd/launch.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,19 @@ func makeUnprivilegedXrootdLauncher(daemonName string, configPath string, isCach
165165
testFileCinfoLocation := filepath.Join(basePath, "self-test-cache-server.txt.cinfo")
166166
result.ExtraEnv = append(result.ExtraEnv, "XRDHTTP_PELICAN_CACHE_SELF_TEST_FILE="+testFileLocation)
167167
result.ExtraEnv = append(result.ExtraEnv, "XRDHTTP_PELICAN_CACHE_SELF_TEST_FILE_CINFO="+testFileCinfoLocation)
168+
169+
xrootdRun := param.Origin_RunLocation.GetString()
170+
authFileName := "authfile-origin-generated"
171+
scitokensCfgFileName := "scitokens-origin-generated.cfg"
172+
if isCache {
173+
xrootdRun = param.Cache_RunLocation.GetString()
174+
authFileName = "authfile-cache-generated"
175+
scitokensCfgFileName = "scitokens-cache-generated.cfg"
176+
}
177+
authPath := filepath.Join(xrootdRun, authFileName)
178+
configPath := filepath.Join(xrootdRun, scitokensCfgFileName)
179+
result.ExtraEnv = append(result.ExtraEnv, "XRDHTTP_PELICAN_AUTHFILE_GENERATED="+authPath)
180+
result.ExtraEnv = append(result.ExtraEnv, "XRDHTTP_PELICAN_SCITOKENS_GENERATED="+configPath)
168181
}
169182
return
170183
}

xrootd/self_monitor.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -236,18 +236,17 @@ func generateTestFileViaPlugin() (string, error) {
236236
// Transplant the test file to selfTestDir using the xrdhttp-pelican plugin.
237237
// Command "4" instructs the plugin to put the test file into the designated location, which is specified in `xrootd/launch.go`.
238238
// Check `src/XrdHttpPelican.cc` in https://github.com/PelicanPlatform/xrdhttp-pelican for the counterpart in the plugin.
239-
if err = SelfTestFileCopy(4, file); err != nil {
239+
if err = FileCopyToXrootdDir(false, 4, file); err != nil {
240240
return "", errors.Wrap(err, "failed to copy the test file to the self-test directory")
241241
}
242242
// Transplant the cinfo file to selfTestDir using the xrdhttp-pelican plugin.
243243
// Command "5" instructs the plugin to put the cinfo file into the designated location, which is specified in `xrootd/launch.go`.
244-
if err = SelfTestFileCopy(5, cinfoFile); err != nil {
244+
if err = FileCopyToXrootdDir(false, 5, cinfoFile); err != nil {
245245
return "", errors.Wrap(err, "failed to copy the test cinfo file to the self-test directory")
246246
}
247247

248248
// Construct and return the URL of the copied test file in the selfTestDir
249-
cachePort := param.Cache_Port.GetInt()
250-
baseUrlStr := fmt.Sprintf("https://%s:%d", param.Server_Hostname.GetString(), cachePort)
249+
baseUrlStr := fmt.Sprintf("https://%s:%d", param.Server_Hostname.GetString(), param.Cache_Port.GetInt())
251250
baseUrl, err := url.Parse(baseUrlStr)
252251
if err != nil {
253252
return "", errors.Wrap(err, "failed to validate the base url for self-test download")

xrootd/xrootd_config.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -611,10 +611,10 @@ func copyXrootdCertificates(server server_structs.XRootDServer) error {
611611
return nil
612612
}
613613

614-
func SelfTestFileCopy(cmd int, file *os.File) error {
615-
log.Debug("Transplanting a self-test file")
616-
if err := sendChildFD(false, cmd, file); err != nil {
617-
return errors.Wrap(err, "Failed to copy the self-test file via FD")
614+
func FileCopyToXrootdDir(isOrigin bool, cmd int, file *os.File) error {
615+
log.Debugf("Transplanting file %s to a directory owned by the xrootd process", file.Name())
616+
if err := sendChildFD(isOrigin, cmd, file); err != nil {
617+
return errors.Wrapf(err, "Failed to copy the file %s via FD", file.Name())
618618
}
619619
return nil
620620
}
@@ -633,6 +633,14 @@ func dropPrivilegeCopy(server server_structs.XRootDServer) error {
633633
destination = filepath.Join(param.Cache_RunLocation.GetString(), "pelican")
634634
}
635635
destination = filepath.Join(destination, "copied-tls-creds.crt")
636+
637+
// If the file already exists, delete it so that OpenFile will create a new one.
638+
// Because the destination file is read-only (0400), user cannot update it (os.O_TRUNC will hit an permission denied error).
639+
err := os.Remove(destination)
640+
if err != nil && !os.IsNotExist(err) {
641+
return errors.Wrapf(err, "Failure when removing existing certificate key pair file for xrootd")
642+
}
643+
636644
destFile, err := os.OpenFile(destination, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, fs.FileMode(0400))
637645
if err != nil {
638646
return errors.Wrap(err, "Failure when opening certificate key pair file to pass to xrootd")

0 commit comments

Comments
 (0)