-
Notifications
You must be signed in to change notification settings - Fork 384
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Add a /etc/containers/auth.json
#1746
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ import ( | |
"encoding/json" | ||
"errors" | ||
"fmt" | ||
"io" | ||
"os" | ||
"os/exec" | ||
"path/filepath" | ||
|
@@ -32,6 +33,8 @@ type dockerConfigFile struct { | |
CredHelpers map[string]string `json:"credHelpers,omitempty"` | ||
} | ||
|
||
// systemPath is the global auth path preferred for systemd services. | ||
var systemPath = authPath{path: filepath.FromSlash("/etc/containers/auth.json"), legacyFormat: false, requireUserOnly: true} | ||
var ( | ||
defaultPerUIDPathFormat = filepath.FromSlash("/run/containers/%d/auth.json") | ||
xdgConfigHomePath = filepath.FromSlash("containers/auth.json") | ||
|
@@ -53,6 +56,8 @@ var ( | |
type authPath struct { | ||
path string | ||
legacyFormat bool | ||
// requireUserOnly will cause the file to be ignored if it is readable by group or other | ||
requireUserOnly bool | ||
} | ||
|
||
// newAuthPathDefault constructs an authPath in non-legacy format. | ||
|
@@ -215,7 +220,21 @@ func GetAllCredentials(sys *types.SystemContext) (map[string]types.DockerAuthCon | |
// The homeDir parameter should always be homedir.Get(), and is only intended to be overridden | ||
// by tests. | ||
func getAuthFilePaths(sys *types.SystemContext, homeDir string) []authPath { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With growing complexity, it seems more important to have unit tests for this. But that’s the very last step. |
||
runningInSystemd := os.Getenv("INVOCATION_ID") != "" | ||
runningAsRoot := os.Getuid() == 0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At a first glance it seems that this should use |
||
runningSystemdPrivileged := runningInSystemd && runningAsRoot | ||
|
||
paths := []authPath{} | ||
|
||
haveExplicitConfig := sys != nil && (sys.AuthFilePath != "" || sys.LegacyFormatAuthFilePath != "") | ||
|
||
// If we're in systemd, prefer the global auth path first. | ||
insertedGlobalPath := false | ||
if !haveExplicitConfig && runningSystemdPrivileged { | ||
paths = append(paths, systemPath) | ||
insertedGlobalPath = true | ||
} | ||
|
||
pathToAuth, err := getPathToAuth(sys) | ||
if err == nil { | ||
paths = append(paths, pathToAuth) | ||
|
@@ -225,7 +244,7 @@ func getAuthFilePaths(sys *types.SystemContext, homeDir string) []authPath { | |
// Logging the error as a warning instead and moving on to pulling the image | ||
logrus.Warnf("%v: Trying to pull image in the event that it is a public image.", err) | ||
} | ||
if sys == nil || (sys.AuthFilePath == "" && sys.LegacyFormatAuthFilePath == "") { | ||
if !haveExplicitConfig { | ||
xdgCfgHome := os.Getenv("XDG_CONFIG_HOME") | ||
if xdgCfgHome == "" { | ||
xdgCfgHome = filepath.Join(homeDir, ".config") | ||
|
@@ -241,6 +260,12 @@ func getAuthFilePaths(sys *types.SystemContext, homeDir string) []authPath { | |
paths = append(paths, | ||
authPath{path: filepath.Join(homeDir, dockerLegacyHomePath), legacyFormat: true}, | ||
) | ||
// If we didn't already insert the global path, do it at the end if we're running as root. | ||
// This will ensure the same semantics for code executed as systemd units and run | ||
// from an interactive shell (as root) as long as there's no user-root owned configs. | ||
if !insertedGlobalPath && runningAsRoot { | ||
paths = append(paths, systemPath) | ||
} | ||
} | ||
return paths | ||
} | ||
|
@@ -552,14 +577,29 @@ func getPathToAuthWithOS(sys *types.SystemContext, goOS string) (authPath, error | |
func (path authPath) parse() (dockerConfigFile, error) { | ||
var auths dockerConfigFile | ||
|
||
raw, err := os.ReadFile(path.path) | ||
f, err := os.Open(path.path) | ||
if err != nil { | ||
if os.IsNotExist(err) { | ||
auths.AuthConfigs = map[string]dockerAuthConfig{} | ||
return auths, nil | ||
} | ||
return dockerConfigFile{}, err | ||
} | ||
defer f.Close() | ||
if path.requireUserOnly { | ||
st, err := f.Stat() | ||
if err != nil { | ||
return dockerConfigFile{}, fmt.Errorf("stat %s: %w", path.path, err) | ||
} | ||
perms := st.Mode().Perm() | ||
if (perms & 044) > 0 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
return dockerConfigFile{}, fmt.Errorf("refusing to process %s with group or world read permissions", path.path) | ||
} | ||
} | ||
raw, err := io.ReadAll(f) | ||
if err != nil { | ||
return dockerConfigFile{}, fmt.Errorf("reading %s: %w", path.path, err) | ||
} | ||
|
||
if path.legacyFormat { | ||
if err = json.Unmarshal(raw, &auths.AuthConfigs); err != nil { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the new paths to this function means that writers would never write to
systemPath
without a specific override; is that intended?I think that’s actually a good idea, so that
root
’s interactive login doesn’t write to a noninteractively-used file, but it should be documented.