Skip to content

Commit

Permalink
Make validateKey a bit more strict
Browse files Browse the repository at this point in the history
For now, reject things that look like tags or digests,
but still accept clearly invalid characters.

Signed-off-by: Miloslav Trmač <[email protected]>
  • Loading branch information
mtrmac committed Sep 13, 2021
1 parent 5798b0e commit 2136228
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 3 deletions.
19 changes: 16 additions & 3 deletions pkg/docker/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -766,11 +766,24 @@ func normalizeRegistry(registry string) string {

// validateKey verifies that the input key does not have a prefix that is not
// allowed and returns an indicator if the key is namespaced.
func validateKey(key string) (isNamespaced bool, err error) {
func validateKey(key string) (bool, error) {
if strings.HasPrefix(key, "http://") || strings.HasPrefix(key, "https://") {
return isNamespaced, errors.Errorf("key %s contains http[s]:// prefix", key)
return false, errors.Errorf("key %s contains http[s]:// prefix", key)
}

// Ideally this should only accept explicitly valid keys, compare
// validateIdentityRemappingPrefix. For now, just reject values that look
// like tagged or digested values.
if strings.ContainsRune(key, '@') {
return false, fmt.Errorf(`key %s contains a '@' character`, key)
}

firstSlash := strings.IndexRune(key, '/')
isNamespaced := firstSlash != -1
// Reject host/repo:tag, but allow localhost:5000 and localhost:5000/foo.
if isNamespaced && strings.ContainsRune(key[firstSlash+1:], ':') {
return false, fmt.Errorf(`key %s contains a ':' character after host[:port]`, key)
}
// check if the provided key contains one or more subpaths.
return strings.ContainsRune(key, '/'), nil
return isNamespaced, nil
}
6 changes: 6 additions & 0 deletions pkg/docker/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,10 @@ func TestValidateKey(t *testing.T) {
// Invalid keys
for _, key := range []string{
"https://my-registry.local",
"host/foo:tag",
"host/foo@digest",
"localhost:5000/repo:tag",
"localhost:5000/repo@digest",
} {
_, err := validateKey(key)
assert.Error(t, err, key)
Expand All @@ -875,6 +879,8 @@ func TestValidateKey(t *testing.T) {
{"my-registry.local", false},
{"my-registry.local/path", true},
{"quay.io/a/b/c/d", true},
{"localhost:5000", false},
{"localhost:5000/repo", true},
} {
isNamespaced, err := validateKey(tc.key)
require.NoError(t, err, tc.key)
Expand Down

0 comments on commit 2136228

Please sign in to comment.