diff --git a/components/gitpod-protocol/src/protocol.spec.ts b/components/gitpod-protocol/src/protocol.spec.ts index 62a0a6eb52d9bd..11a211a85df44a 100644 --- a/components/gitpod-protocol/src/protocol.spec.ts +++ b/components/gitpod-protocol/src/protocol.spec.ts @@ -6,7 +6,7 @@ import { suite, test } from "@testdeck/mocha"; import * as chai from "chai"; -import { SSHPublicKeyValue } from "."; +import { SSHPublicKeyValue, EnvVar, EnvVarWithValue } from "."; const expect = chai.expect; @@ -94,4 +94,120 @@ class TestSSHPublicKeyValue { ).to.throw("Key is invalid"); } } -module.exports = new TestSSHPublicKeyValue(); // Only to circumvent no usage warning :-/ + +@suite +class TestEnvVar { + @test + public testGetGitpodImageAuth_empty() { + const result = EnvVar.getGitpodImageAuth([]); + expect(result.size).to.equal(0); + } + + @test + public testGetGitpodImageAuth_noRelevantVar() { + const envVars: EnvVarWithValue[] = [{ name: "OTHER_VAR", value: "some_value" }]; + const result = EnvVar.getGitpodImageAuth(envVars); + expect(result.size).to.equal(0); + } + + @test + public testGetGitpodImageAuth_singleEntryNoPort() { + const envVars: EnvVarWithValue[] = [ + { + name: EnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME, + value: "my-registry.foo.net:Zm9vOmJhcg==", + }, + ]; + const result = EnvVar.getGitpodImageAuth(envVars); + expect(result.size).to.equal(1); + expect(result.get("my-registry.foo.net")).to.equal("Zm9vOmJhcg=="); + } + + @test + public testGetGitpodImageAuth_singleEntryWithPort() { + const envVars: EnvVarWithValue[] = [ + { + name: EnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME, + value: "my-registry.foo.net:5000:Zm9vOmJhcg==", + }, + ]; + const result = EnvVar.getGitpodImageAuth(envVars); + expect(result.size).to.equal(1); + expect(result.get("my-registry.foo.net:5000")).to.equal("Zm9vOmJhcg=="); + } + + @test + public testGetGitpodImageAuth_multipleEntries() { + const envVars: EnvVarWithValue[] = [ + { + name: EnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME, + value: "my-registry.foo.net:Zm9vOmJhcg==,my-registry2.bar.com:YWJjOmRlZg==", + }, + ]; + const result = EnvVar.getGitpodImageAuth(envVars); + expect(result.size).to.equal(2); + expect(result.get("my-registry.foo.net")).to.equal("Zm9vOmJhcg=="); + expect(result.get("my-registry2.bar.com")).to.equal("YWJjOmRlZg=="); + } + + @test + public testGetGitpodImageAuth_multipleEntriesWithPortAndMalformed() { + const envVars: EnvVarWithValue[] = [ + { + name: EnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME, + value: "my-registry.foo.net:5000:Zm9vOmJhcg==,my-registry2.bar.com:YWJjOmRlZg==,invalidEntry,another.host:anothercred", + }, + ]; + const result = EnvVar.getGitpodImageAuth(envVars); + expect(result.size).to.equal(3); + expect(result.get("my-registry2.bar.com")).to.equal("YWJjOmRlZg=="); + expect(result.get("another.host")).to.equal("anothercred"); + expect(result.get("my-registry.foo.net:5000")).to.equal("Zm9vOmJhcg=="); + } + + @test + public testGetGitpodImageAuth_emptyValue() { + const envVars: EnvVarWithValue[] = [ + { + name: EnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME, + value: "", + }, + ]; + const result = EnvVar.getGitpodImageAuth(envVars); + expect(result.size).to.equal(0); + } + + @test + public testGetGitpodImageAuth_malformedEntries() { + const envVars: EnvVarWithValue[] = [ + { + name: EnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME, + value: "justhost,hostonly:,:credonly,:::,:,", + }, + ]; + const result = EnvVar.getGitpodImageAuth(envVars); + expect(result.size).to.equal(0); + } + + @test + public testGetGitpodImageAuth_entriesWithSpaces() { + const envVars: EnvVarWithValue[] = [ + { + name: EnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME, + value: " my-registry.foo.net : Zm9vOmJhcg== , my-registry2.bar.com:YWJjOmRlZg== ", + }, + ]; + const result = EnvVar.getGitpodImageAuth(envVars); + expect(result.size).to.equal(2); + expect(result.get("my-registry.foo.net")).to.equal("Zm9vOmJhcg=="); + expect(result.get("my-registry2.bar.com")).to.equal("YWJjOmRlZg=="); + } +} + +// Exporting both test suites +const testSSHPublicKeyValue = new TestSSHPublicKeyValue(); +const testEnvVar = new TestEnvVar(); +module.exports = { + testSSHPublicKeyValue, + testEnvVar, +}; diff --git a/components/gitpod-protocol/src/protocol.ts b/components/gitpod-protocol/src/protocol.ts index 8606fe10f76134..865689d438ecc8 100644 --- a/components/gitpod-protocol/src/protocol.ts +++ b/components/gitpod-protocol/src/protocol.ts @@ -293,11 +293,28 @@ export namespace EnvVar { return res; } + const parse = (parts: string[]): { host: string; auth: string } | undefined => { + if (parts.some((e) => e === "")) { + return undefined; + } + if (parts.length === 2) { + return { host: parts[0], auth: parts[1] }; + } else if (parts.length === 3) { + return { host: `${parts[0]}:${parts[1]}`, auth: parts[2] }; + } + return undefined; + }; + (imageAuth.value || "") .split(",") - .map((e) => e.trim().split(":")) - .filter((e) => e.length == 2) - .forEach((e) => res.set(e[0], e[1])); + .map((e) => e.split(":").map((part) => part.trim())) + .forEach((parts) => { + const parsed = parse(parts); + if (parsed) { + res.set(parsed.host, parsed.auth); + } + }); + return res; } } diff --git a/components/image-builder-bob/pkg/proxy/auth.go b/components/image-builder-bob/pkg/proxy/auth.go index f175acc5ecc326..3bb925d4ee2275 100644 --- a/components/image-builder-bob/pkg/proxy/auth.go +++ b/components/image-builder-bob/pkg/proxy/auth.go @@ -44,30 +44,50 @@ type authConfig struct { type MapAuthorizer map[string]authConfig -func (a MapAuthorizer) Authorize(host string) (user, pass string, err error) { +func (a MapAuthorizer) Authorize(hostHeader string) (user, pass string, err error) { defer func() { log.WithFields(logrus.Fields{ - "host": host, + "host": hostHeader, "user": user, }).Info("authorizing registry access") }() - // Strip any port from the host if present - host = strings.Split(host, ":")[0] + parseHostHeader := func(hostHeader string) (string, string) { + hostHeaderSlice := strings.Split(hostHeader, ":") + hostname := strings.TrimSpace(hostHeaderSlice[0]) + var port string + if len(hostHeaderSlice) > 1 { + port = strings.TrimSpace(hostHeaderSlice[1]) + } + return hostname, port + } + hostname, port := parseHostHeader(hostHeader) + // gpl: Could be port 80 as well, but we don't know if we are servinc http or https, we assume https + if port == "" { + port = "443" + } + host := hostname + ":" + port explicitHostMatcher := func() (authConfig, bool) { + // 1. precise host match res, ok := a[host] + if ok { + return res, ok + } + + // 2. make sure we not have a hostname match + res, ok = a[hostname] return res, ok } ecrHostMatcher := func() (authConfig, bool) { - if isECRRegistry(host) { + if isECRRegistry(hostname) { res, ok := a[DummyECRRegistryDomain] return res, ok } return authConfig{}, false } dockerHubHostMatcher := func() (authConfig, bool) { - if isDockerHubRegistry(host) { + if isDockerHubRegistry(hostname) { res, ok := a["docker.io"] return res, ok } diff --git a/components/image-builder-bob/pkg/proxy/auth_test.go b/components/image-builder-bob/pkg/proxy/auth_test.go index 921f903519187f..e5c5f5f0e73242 100644 --- a/components/image-builder-bob/pkg/proxy/auth_test.go +++ b/components/image-builder-bob/pkg/proxy/auth_test.go @@ -32,7 +32,7 @@ func TestAuthorize(t *testing.T) { }, }, { - name: "docker auth format - valid credentials - host with port", + name: "docker auth format - valid credentials - host with :443 port", constructor: NewAuthorizerFromDockerEnvVar, input: `{"auths": {"registry.example.com": {"auth": "dXNlcjpwYXNz"}}}`, // base64(user:pass) testHost: "registry.example.com:443", @@ -110,6 +110,26 @@ func TestAuthorize(t *testing.T) { pass: "", }, }, + { + name: "Docker Hub", + constructor: NewAuthorizerFromEnvVar, + input: `{"docker.io": {"auth": "dXNlcjpwYXNz"}}`, + testHost: "registry-1.docker.io", + expected: expectation{ + user: "user", + pass: "pass", + }, + }, + { + name: "docker auth format - valid credentials - host with :5000 port", + constructor: NewAuthorizerFromDockerEnvVar, + input: `{"auths": {"registry.example.com:5000": {"auth": "dXNlcjpwYXNz"}}}`, // base64(user:pass) + testHost: "registry.example.com:5000", + expected: expectation{ + user: "user", + pass: "pass", + }, + }, } for _, tt := range tests { diff --git a/components/image-builder-mk3/pkg/auth/auth.go b/components/image-builder-mk3/pkg/auth/auth.go index cdef4d2c40c510..749f275c563f4c 100644 --- a/components/image-builder-mk3/pkg/auth/auth.go +++ b/components/image-builder-mk3/pkg/auth/auth.go @@ -383,9 +383,11 @@ func (a AllowedAuthFor) additionalAuth(domain string) *Authentication { dec, err := base64.StdEncoding.DecodeString(ath) if err == nil { segs := strings.Split(string(dec), ":") - if len(segs) > 1 { - res.Username = segs[0] - res.Password = strings.Join(segs[1:], ":") + numSegs := len(segs) + + if numSegs > 1 { + res.Username = strings.Join(segs[:numSegs-1], ":") + res.Password = segs[numSegs-1] } } else { log.Errorf("failed getting additional auth") diff --git a/components/image-builder-mk3/pkg/auth/auth_test.go b/components/image-builder-mk3/pkg/auth/auth_test.go index aa9f6ffe713478..0b41e908c0a151 100644 --- a/components/image-builder-mk3/pkg/auth/auth_test.go +++ b/components/image-builder-mk3/pkg/auth/auth_test.go @@ -5,6 +5,7 @@ package auth import ( + "encoding/base64" "testing" "github.com/google/go-cmp/cmp" @@ -30,3 +31,98 @@ func TestIsECRRegistry(t *testing.T) { }) } } + +func TestAdditionalAuth(t *testing.T) { + tests := []struct { + name string + domain string + additionalMap map[string]string + expectedAuth *Authentication + }{ + { + name: "standard host:token", + domain: "myregistry.com", + additionalMap: map[string]string{ + "myregistry.com": base64.StdEncoding.EncodeToString([]byte("myregistry.com:mytoken")), + }, + expectedAuth: &Authentication{ + Username: "myregistry.com", + Password: "mytoken", + Auth: base64.StdEncoding.EncodeToString([]byte("myregistry.com:mytoken")), + }, + }, + { + name: "buggy host:port:token", + domain: "myregistry.com:5000", + additionalMap: map[string]string{ + "myregistry.com:5000": base64.StdEncoding.EncodeToString([]byte("myregistry.com:5000:mytoken")), + }, + expectedAuth: &Authentication{ + Username: "myregistry.com:5000", + Password: "mytoken", + Auth: base64.StdEncoding.EncodeToString([]byte("myregistry.com:5000:mytoken")), + }, + }, + { + name: "only username, no password/token (single segment)", + domain: "useronly.com", + additionalMap: map[string]string{ + "useronly.com": base64.StdEncoding.EncodeToString([]byte("justauser")), + }, + expectedAuth: &Authentication{ + Auth: base64.StdEncoding.EncodeToString([]byte("justauser")), + }, + }, + { + name: "empty auth string", + domain: "emptyauth.com", + additionalMap: map[string]string{ + "emptyauth.com": base64.StdEncoding.EncodeToString([]byte("")), + }, + expectedAuth: &Authentication{ + Auth: base64.StdEncoding.EncodeToString([]byte("")), + }, + }, + { + name: "domain not in map", + domain: "notfound.com", + additionalMap: map[string]string{"someother.com": base64.StdEncoding.EncodeToString([]byte("someauth"))}, + expectedAuth: nil, + }, + { + name: "invalid base64 string", + domain: "invalidbase64.com", + additionalMap: map[string]string{ + "invalidbase64.com": "!!!INVALID_BASE64!!!", + }, + expectedAuth: &Authentication{ + Auth: "!!!INVALID_BASE64!!!", + }, + }, + { + name: "standard host:token where username in cred is different from domain key", + domain: "docker.io", + additionalMap: map[string]string{ + "docker.io": base64.StdEncoding.EncodeToString([]byte("user1:pass1")), + }, + expectedAuth: &Authentication{ + Username: "user1", + Password: "pass1", + Auth: base64.StdEncoding.EncodeToString([]byte("user1:pass1")), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + aaf := AllowedAuthFor{ + Additional: tt.additionalMap, + } + actualAuth := aaf.additionalAuth(tt.domain) + + if diff := cmp.Diff(tt.expectedAuth, actualAuth); diff != "" { + t.Errorf("additionalAuth() mismatch (-want +got):\n%s", diff) + } + }) + } +} diff --git a/components/supervisor/pkg/supervisor/docker.go b/components/supervisor/pkg/supervisor/docker.go index f2c3eb223b2a61..89bdb1507c9bd2 100644 --- a/components/supervisor/pkg/supervisor/docker.go +++ b/components/supervisor/pkg/supervisor/docker.go @@ -312,18 +312,28 @@ func insertCredentialsIntoConfig(imageAuth string) (int, error) { Auths: make(map[string]RegistryAuth), } authenticationPerHost := strings.Split(imageAuth, ",") - for _, hostCredentials := range authenticationPerHost { - parts := strings.SplitN(hostCredentials, ":", 2) - if len(parts) < 2 { + for _, hostCredentialsEntry := range authenticationPerHost { + parts := strings.SplitN(hostCredentialsEntry, ":", 3) + var host string + var token string + + switch len(parts) { + case 2: + host = parts[0] + token = parts[1] + case 3: + host = parts[0] + ":" + parts[1] + token = parts[2] + default: continue } - host := parts[0] + if host == "docker.io" || strings.HasSuffix(host, ".docker.io") { host = "https://index.docker.io/v1/" } authConfig.Auths[host] = RegistryAuth{ - Auth: parts[1], + Auth: token, } } if len(authConfig.Auths) == 0 { diff --git a/components/supervisor/pkg/supervisor/docker_test.go b/components/supervisor/pkg/supervisor/docker_test.go index ca4e1a7e08e661..84db9353f6df53 100644 --- a/components/supervisor/pkg/supervisor/docker_test.go +++ b/components/supervisor/pkg/supervisor/docker_test.go @@ -173,3 +173,187 @@ func TestInsertRegistryAuth(t *testing.T) { }) } } + +// tests the parsing logic of insertCredentialsIntoConfig +func TestInsertCredentialsIntoConfig_Parsing(t *testing.T) { + tests := []struct { + name string + imageAuthInput string + expectedAuthsJSON string + expectedCount int + expectError bool + }{ + { + name: "simple host and token", + imageAuthInput: "myregistry.com:secrettoken", + expectedAuthsJSON: `{ + "myregistry.com": {"auth": "secrettoken"} + }`, + expectedCount: 1, + }, + { + name: "docker.io host and token", + imageAuthInput: "docker.io:dockertoken", + expectedAuthsJSON: `{ + "https://index.docker.io/v1/": {"auth": "dockertoken"} + }`, + expectedCount: 1, + }, + { + name: "subdomain of docker.io and token", + imageAuthInput: "sub.docker.io:subdockertoken", + expectedAuthsJSON: `{ + "https://index.docker.io/v1/": {"auth": "subdockertoken"} + }`, + expectedCount: 1, + }, + { + name: "host with port and token", + imageAuthInput: "myregistry.com:5000:supersecret", + expectedAuthsJSON: `{ + "myregistry.com:5000": {"auth": "supersecret"} + }`, + expectedCount: 1, + }, + { + name: "multiple credentials, some with port", + imageAuthInput: "reg1.com:token1,reg2.com:6000:token2,docker.io:token3", + expectedAuthsJSON: `{ + "reg1.com": {"auth": "token1"}, + "reg2.com:6000": {"auth": "token2"}, + "https://index.docker.io/v1/": {"auth": "token3"} + }`, + expectedCount: 3, + }, + { + name: "empty imageAuth string", + imageAuthInput: "", + expectedAuthsJSON: `{}`, + expectedCount: 0, + }, + { + name: "credential with empty token part", + imageAuthInput: "myregistry.com:", + expectedAuthsJSON: `{ + "myregistry.com": {"auth": ""} + }`, + expectedCount: 1, + }, + { + name: "credential with empty host part", + imageAuthInput: ":mytoken", + expectedAuthsJSON: `{ + "": {"auth": "mytoken"} + }`, + expectedCount: 1, + }, + { + name: "credential with only a colon", + imageAuthInput: ":", + expectedAuthsJSON: `{ + "": {"auth": ""} + }`, + expectedCount: 1, + }, + { + name: "single invalid credential (no colon)", + imageAuthInput: "myregistry.com", + expectedAuthsJSON: `{}`, + expectedCount: 0, + }, + { + name: "mixed valid and invalid credentials", + imageAuthInput: "reg1.com:token1,invalidreg,reg2.com:token2", + expectedAuthsJSON: `{ + "reg1.com": {"auth": "token1"}, + "reg2.com": {"auth": "token2"} + }`, + expectedCount: 2, + }, + { + name: "input with leading/trailing spaces for the whole string", + imageAuthInput: " myregistry.com:spacedtoken ", + expectedAuthsJSON: `{ + "myregistry.com": {"auth": "spacedtoken"} + }`, + expectedCount: 1, + }, + { + name: "input with spaces around comma separator creating leading space in host", + imageAuthInput: "reg1.com:token1 , reg2.com:token2", + expectedAuthsJSON: `{ + "reg1.com": {"auth": "token1 "}, + " reg2.com": {"auth": "token2"} + }`, + expectedCount: 2, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "docker-auth-test-") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + originalHome := os.Getenv("HOME") + os.Setenv("HOME", tmpDir) + defer os.Setenv("HOME", originalHome) + + count, err := insertCredentialsIntoConfig(tc.imageAuthInput) + + if tc.expectError { + if err == nil { + t.Errorf("Expected an error, but got nil") + } + return + } + if err != nil { + t.Fatalf("insertCredentialsIntoConfig() returned an unexpected error: %v", err) + } + + if count != tc.expectedCount { + t.Errorf("Expected count %d, got %d", tc.expectedCount, count) + } + + configPath := filepath.Join(tmpDir, ".docker", "config.json") + + var expectedAuthsMap map[string]RegistryAuth + if err := json.Unmarshal([]byte(tc.expectedAuthsJSON), &expectedAuthsMap); err != nil { + t.Fatalf("Failed to unmarshal expectedAuthsJSON for tc '%s': %v. JSON: %s", tc.name, err, tc.expectedAuthsJSON) + } + + _, statErr := os.Stat(configPath) + if os.IsNotExist(statErr) { + if len(expectedAuthsMap) == 0 && tc.expectedCount == 0 { + return + } + t.Fatalf("Config file %s does not exist, but expected auths (count: %d, map: %s)", configPath, tc.expectedCount, tc.expectedAuthsJSON) + } + if statErr != nil && !os.IsNotExist(statErr) { + t.Fatalf("Error stating config file %s: %v", configPath, statErr) + } + + configBytes, readErr := os.ReadFile(configPath) + if readErr != nil { + t.Fatalf("Failed to read docker config file %s: %v. Expected auths: %s", configPath, readErr, tc.expectedAuthsJSON) + } + + var resultConfig DockerConfig + if err := json.Unmarshal(configBytes, &resultConfig); err != nil { + t.Fatalf("Failed to unmarshal result config: %v. Content: %s", err, string(configBytes)) + } + + if len(expectedAuthsMap) == 0 { + if len(resultConfig.Auths) != 0 { + t.Errorf("Expected empty auths, but got: %v", resultConfig.Auths) + } + } else { + if diff := cmp.Diff(expectedAuthsMap, resultConfig.Auths); diff != "" { + t.Errorf("Unexpected auths map in config.json (-want +got):\n%s", diff) + } + } + }) + } +}