Skip to content

Commit eed2ca6

Browse files
authored
CLID-389: v2: validate access to destination registry (#1201)
Check that we can authenticate against the destination registry with the given credentials.
1 parent f2e63d2 commit eed2ca6

File tree

2 files changed

+140
-0
lines changed

2 files changed

+140
-0
lines changed

v2/internal/pkg/cli/executor.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"log"
1010
"net"
1111
"net/http"
12+
"net/url"
1213
"os"
1314
"path"
1415
"path/filepath"
@@ -24,6 +25,9 @@ import (
2425
"golang.org/x/term"
2526
"k8s.io/kubectl/pkg/util/templates"
2627

28+
"github.com/containers/image/v5/docker"
29+
"github.com/containers/image/v5/docker/reference"
30+
imgconfig "github.com/containers/image/v5/pkg/docker/config"
2731
"github.com/distribution/distribution/v3/configuration"
2832
"github.com/distribution/distribution/v3/registry"
2933
_ "github.com/distribution/distribution/v3/registry/storage/driver/filesystem"
@@ -242,6 +246,14 @@ func NewMirrorCmd(log clog.PluggableLoggerInterface) *cobra.Command {
242246
defer ex.logFile.Close()
243247
cmd.SetOutput(ex.logFile)
244248

249+
// NOTE: this is not in the `ex.Validate` function because it breaks unit tests
250+
if strings.Contains(args[0], dockerProtocol) {
251+
registry := strings.TrimPrefix(args[0], dockerProtocol)
252+
if err := ex.checkRegistryAccess(cmd.Context(), registry); err != nil {
253+
return fmt.Errorf("checking registry %q access: %w", registry, err)
254+
}
255+
}
256+
245257
// prepare internal storage
246258
if err := ex.setupLocalStorage(cmd.Context()); err != nil {
247259
return err
@@ -399,6 +411,64 @@ func (o ExecutorSchema) Validate(dest []string) error {
399411

400412
}
401413

414+
func (o *ExecutorSchema) checkRegistryAccess(ctx context.Context, registry string) error {
415+
o.Log.Debug("Checking registry access to %q...", registry)
416+
417+
sctx, err := o.Opts.DestImage.NewSystemContext()
418+
if err != nil {
419+
return fmt.Errorf("failed to get system context: %w", err)
420+
}
421+
422+
key, reg, err := parseCredentialsKey(registry)
423+
if err != nil {
424+
return err
425+
}
426+
427+
authConfig, err := imgconfig.GetCredentials(sctx, key)
428+
if err != nil {
429+
return fmt.Errorf("failed to find credentials: %w", err)
430+
}
431+
432+
ctx, cancel := context.WithTimeout(ctx, 2*time.Minute)
433+
defer cancel()
434+
if err := docker.CheckAuth(ctx, sctx, authConfig.Username, authConfig.Password, reg); err != nil {
435+
return fmt.Errorf("failed to authenticate: %w", err)
436+
}
437+
438+
o.Log.Info("Verified we can authenticate against registry %q", registry)
439+
return nil
440+
}
441+
442+
// See containers/common/pkg/auth.parseCredentialsKey
443+
func parseCredentialsKey(arg string) (key string, registry string, err error) {
444+
if strings.HasPrefix(arg, "http://") || strings.HasPrefix(arg, "https://") {
445+
url, err := url.Parse(arg)
446+
if err != nil {
447+
return "", "", fmt.Errorf("failed to parse: %w", err)
448+
}
449+
key = url.Host
450+
} else {
451+
key = arg
452+
}
453+
split := strings.Split(key, "/")
454+
registry = split[0]
455+
if registry == key {
456+
return key, registry, nil
457+
}
458+
ref, err := reference.ParseNormalizedNamed(key)
459+
if err != nil {
460+
return "", "", fmt.Errorf("parse reference %q: %w", key, err)
461+
}
462+
if !reference.IsNameOnly(ref) {
463+
return "", "", fmt.Errorf("reference %q contains tag or digest", ref.String())
464+
}
465+
refRegistry := reference.Domain(ref)
466+
if refRegistry != registry {
467+
return "", "", fmt.Errorf("key %q registry mismatch %q vs %q", key, registry, refRegistry)
468+
}
469+
return key, registry, nil
470+
}
471+
402472
// Complete - do the final setup of modules
403473
func (o *ExecutorSchema) Complete(args []string) error {
404474
if envOverride, ok := os.LookupEnv("CONTAINERS_REGISTRIES_CONF"); ok {

v2/internal/pkg/cli/executor_test.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -945,6 +945,76 @@ func TestExcludeImages(t *testing.T) {
945945
}
946946
}
947947

948+
func TestExecutorCheckRegistryAccess(t *testing.T) {
949+
const validRegistry = "localhost:5000"
950+
testFolder := t.TempDir()
951+
regCfg, err := setupRegForTest(testFolder)
952+
assert.NoError(t, err, "failed to parse local registry config")
953+
reg, err := registry.NewRegistry(context.Background(), regCfg)
954+
assert.NoError(t, err, "failed to create local registry service")
955+
global := &mirror.GlobalOptions{
956+
SecurePolicy: false,
957+
Force: true,
958+
WorkingDir: filepath.Join(testFolder, "tests"),
959+
}
960+
fsShared, sharedOpts := mirror.SharedImageFlags()
961+
_, deprecatedTLSVerifyOpt := mirror.DeprecatedTLSVerifyFlags()
962+
fsDest, destOpts := mirror.ImageDestFlags(global, sharedOpts, deprecatedTLSVerifyOpt, "dest-", "dcreds")
963+
opts := &mirror.CopyOptions{
964+
Global: global,
965+
DestImage: destOpts,
966+
}
967+
ex := &ExecutorSchema{
968+
Log: clog.New("debug"),
969+
LocalStorageService: *reg,
970+
Opts: opts,
971+
}
972+
973+
go ex.startLocalRegistry()
974+
// Make sure registry has started up
975+
time.Sleep(5 * time.Second)
976+
t.Cleanup(func() {
977+
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
978+
defer cancel()
979+
ex.stopLocalRegistry(ctx)
980+
})
981+
982+
t.Run("checkRegistryAccess should fail when", func(t *testing.T) {
983+
t.Run("cannot resolve registry hostname", func(t *testing.T) {
984+
const invalidRegistry = "invalid-registry-url.io"
985+
err := ex.checkRegistryAccess(context.TODO(), invalidRegistry)
986+
assert.ErrorContains(t, err, "no such host")
987+
})
988+
t.Run("registry is not accessible", func(t *testing.T) {
989+
const invalidRegistry = "localhost:9999"
990+
err := ex.checkRegistryAccess(context.TODO(), invalidRegistry)
991+
assert.ErrorContains(t, err, "connect: connection refused")
992+
})
993+
t.Run("http registry but tls-verify=true", func(t *testing.T) {
994+
err := ex.checkRegistryAccess(context.TODO(), validRegistry)
995+
assert.ErrorContains(t, err, "http: server gave HTTP response to HTTPS client")
996+
})
997+
t.Run("invalid creds", func(t *testing.T) {
998+
err := fsShared.Set("authfile", "/tmp/invalid-creds.json")
999+
assert.NoError(t, err, "should set flag")
1000+
t.Cleanup(func() { _ = fsShared.Set("authfile", "") })
1001+
err = ex.checkRegistryAccess(context.TODO(), "quay.io/redhat")
1002+
assert.ErrorContains(t, err, "unable to retrieve auth token: invalid username/password: unauthorized")
1003+
})
1004+
})
1005+
1006+
t.Run("checkRegistryAccess should succeed", func(t *testing.T) {
1007+
t.Run("against local http registry when tls-verify=false ", func(t *testing.T) {
1008+
err := fsDest.Set("dest-tls-verify", "false")
1009+
assert.NoError(t, err, "should set flag")
1010+
t.Cleanup(func() { _ = fsDest.Set("dest-tls-verify", "") })
1011+
err = ex.checkRegistryAccess(context.TODO(), validRegistry)
1012+
assert.NoError(t, err)
1013+
})
1014+
// TODO: add HTTPS and cert tests
1015+
})
1016+
}
1017+
9481018
// setup mocks
9491019

9501020
type Mirror struct {

0 commit comments

Comments
 (0)