fix: harden OCI Bearer authentication against SSRF and credential forwarding

.github/workflows/dev-containers.yml

@@ -61,10 +61,11 @@ jobs:
           "src/test/cli.podman.test.ts",
           "src/test/cli.test.ts",
           "src/test/cli.up.test.ts",
+          "src/test/httpOCIRegistry.test.ts",
           "src/test/imageMetadata.test.ts",
           "src/test/container-features/containerFeaturesOCIPush.test.ts",
           # Run all except the above:
-          "--exclude src/test/container-features/containerFeaturesOrder.test.ts --exclude src/test/container-features/registryCompatibilityOCI.test.ts --exclude src/test/container-features/containerFeaturesOCIPush.test.ts --exclude src/test/container-features/e2e.test.ts --exclude src/test/container-features/featuresCLICommands.test.ts --exclude src/test/cli.build.test.ts --exclude src/test/cli.exec.buildKit.1.test.ts --exclude src/test/cli.exec.buildKit.2.test.ts --exclude src/test/cli.exec.nonBuildKit.1.test.ts --exclude src/test/cli.exec.nonBuildKit.2.test.ts --exclude src/test/cli.podman.test.ts --exclude src/test/cli.test.ts --exclude src/test/cli.up.test.ts --exclude src/test/imageMetadata.test.ts 'src/test/**/*.test.ts'",
+          "--exclude src/test/container-features/containerFeaturesOrder.test.ts --exclude src/test/container-features/registryCompatibilityOCI.test.ts --exclude src/test/container-features/containerFeaturesOCIPush.test.ts --exclude src/test/container-features/e2e.test.ts --exclude src/test/container-features/featuresCLICommands.test.ts --exclude src/test/cli.build.test.ts --exclude src/test/cli.exec.buildKit.1.test.ts --exclude src/test/cli.exec.buildKit.2.test.ts --exclude src/test/cli.exec.nonBuildKit.1.test.ts --exclude src/test/cli.exec.nonBuildKit.2.test.ts --exclude src/test/cli.podman.test.ts --exclude src/test/cli.test.ts --exclude src/test/cli.up.test.ts --exclude src/test/httpOCIRegistry.test.ts --exclude src/test/imageMetadata.test.ts 'src/test/**/*.test.ts'",
         ]
     steps:
     - name: Checkout

src/spec-configuration/containerCollectionsOCI.ts

@@ -3,6 +3,7 @@ import * as semver from 'semver';
 import * as tar from 'tar';
 import * as jsonc from 'jsonc-parser';
 import * as crypto from 'crypto';
+import { isIP } from 'net';
 
 import { Log, LogLevel } from '../spec-utils/log';
 import { isLocalFile, mkdirpLocal, readLocalFile, writeLocalFile } from '../spec-utils/pfs';
@@ -116,6 +117,55 @@ const regexForPath = /^[a-z0-9]+([._-][a-z0-9]+)*(\/[a-z0-9]+([._-][a-z0-9]+)*)*
 // MUST be at most 128 characters in length and MUST match the following regular expression:
 const regexForVersionOrDigest = /^[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}$/;
 
+// Validate authority syntax only; local and private registries remain supported by policy.
+function isValidRegistryAuthority(registry: string): boolean {
+	let hostname = registry;
+	let port: string | undefined;
+
+	if (registry.startsWith('[')) {
+		// IPv6 literals must use bracketed URL-authority form so the port is unambiguous.
+		const match = /^\[([^\]]+)\](?::([0-9]+))?$/.exec(registry);
+		if (!match || isIP(match[1]) !== 6) {
+			return false;
+		}
+		hostname = match[1];
+		port = match[2];
+	} else {
+		const firstColon = registry.indexOf(':');
+		if (firstColon !== -1) {
+			if (firstColon !== registry.lastIndexOf(':')) {
+				return false;
+			}
+			hostname = registry.slice(0, firstColon);
+			port = registry.slice(firstColon + 1);
+		}
+
+		if (isIP(hostname) === 0) {
+			if (hostname.length === 0 || hostname.length > 253) {
+				return false;
+			}
+			const labels = hostname.split('.');
+			if (!labels.every(label => label.length > 0
+				&& label.length <= 63
+				&& /^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$/.test(label))) {
+				return false;
+			}
+		}
+	}
+
+	if (port !== undefined) {
+		if (!/^[0-9]+$/.test(port)) {
+			return false;
+		}
+		const portNumber = Number(port);
+		if (portNumber < 1 || portNumber > 65535) {
+			return false;
+		}
+	}
+
+	return true;
+}
+
 // https://go.dev/doc/install/source#environment
 // Expected by OCI Spec as seen here: https://github.com/opencontainers/image-spec/blob/main/image-index.md#image-index-property-descriptions
 export function mapNodeArchitectureToGOARCH(arch: NodeJS.Architecture): GoARCH {
@@ -214,6 +264,10 @@ export function getRef(output: Log, input: string): OCIRef | undefined {
 	const namespace = splitOnSlash.slice(1, -1).join('/');
 
 	const path = `${namespace}/${id}`;
+	if (!isValidRegistryAuthority(registry)) {
+		output.write(`Registry '${registry}' for input '${input}' failed validation.`, LogLevel.Error);
+		return;
+	}
 
 	if (!regexForPath.exec(path)) {
 		output.write(`Path '${path}' for input '${input}' failed validation.  Expected path to match regex '${regexForPath}'.`, LogLevel.Error);
@@ -252,6 +306,10 @@ export function getCollectionRef(output: Log, registry: string, namespace: strin
 	// Normalize input by downcasing entire string
 	registry = registry.toLowerCase();
 	namespace = namespace.toLowerCase();
+	if (!isValidRegistryAuthority(registry)) {
+		output.write(`Registry '${registry}' failed validation.`, LogLevel.Error);
+		return;
+	}
 
 	const path = namespace;
 	const resource = `${registry}/${path}`;
@@ -279,9 +337,13 @@ export function getCollectionRef(output: Log, registry: string, namespace: strin
 export async function fetchOCIManifestIfExists(params: CommonParams, ref: OCIRef | OCICollectionRef, manifestDigest?: string): Promise<ManifestContainer | undefined> {
 	const { output } = params;
 
-	// Simple mechanism to avoid making a DNS request for
-	// something that is not a domain name.
-	if (ref.registry.indexOf('.') < 0 && !ref.registry.startsWith('localhost')) {
+	const registryHostname = ref.registry.startsWith('[')
+		? ref.registry.slice(1, ref.registry.indexOf(']'))
+		: ref.registry.split(':', 1)[0];
+	// Preserve legacy owner/repository/feature IDs while allowing explicit local and IP registries.
+	if (!registryHostname.includes('.')
+		&& registryHostname !== 'localhost'
+		&& isIP(registryHostname) === 0) {
 		return;
 	}
 

src/spec-configuration/httpOCIRegistry.ts

@@ -3,7 +3,7 @@ import * as path from 'path';
 import * as jsonc from 'jsonc-parser';
 
 import { runCommandNoPty, plainExec } from '../spec-common/commonUtils';
-import { requestResolveHeaders } from '../spec-utils/httpRequest';
+import { requestResolveHeaders, requestResolveHeadersNoRedirects } from '../spec-utils/httpRequest';
 import { LogLevel } from '../spec-utils/log';
 import { isLocalFile, readLocalFile } from '../spec-utils/pfs';
 import { CommonParams, OCICollectionRef, OCIRef } from './containerCollectionsOCI';
@@ -35,6 +35,69 @@ const realmRegex = /realm="([^"]+)"/;
 const serviceRegex = /service="([^"]+)"/;
 const scopeRegex = /scope="([^"]+)"/;
 
+type RegistryCredentialType = 'basic' | 'refreshToken';
+
+// Endpoint admission and credential forwarding are separate policies: an allowed
+// token service does not automatically receive credentials stored for the registry.
+export function canForwardCredentialToTokenService(realm: string, registry: string, credentialType: RegistryCredentialType): boolean {
+	let realmUrl: URL;
+	try {
+		realmUrl = new URL(realm);
+	} catch {
+		return false;
+	}
+
+	const normalizedRegistry = registry.toLowerCase();
+	if (realmUrl.host.toLowerCase() === normalizedRegistry) {
+		// Preserve HTTP localhost registries used for local Feature development.
+		return realmUrl.protocol === 'https:'
+			|| realmUrl.protocol === 'http:' && realmUrl.hostname.toLowerCase() === 'localhost';
+	}
+
+	// Docker Hub is the only supported cross-authority credential exchange.
+	return credentialType === 'basic'
+		&& realmUrl.protocol === 'https:'
+		&& !realmUrl.port
+		&& realmUrl.hostname.toLowerCase() === 'auth.docker.io'
+		&& (normalizedRegistry === 'docker.io'
+			|| normalizedRegistry === 'registry.docker.io'
+			|| normalizedRegistry === 'registry-1.docker.io');
+}
+
+// Pin registry-directed token requests to the registry authority or known OCI token services.
+export function isAllowedTokenServiceRealm(realm: string, registry: string): boolean {
+	let realmUrl: URL;
+	try {
+		realmUrl = new URL(realm);
+	} catch {
+		return false;
+	}
+
+	const sameAuthority = realmUrl.host.toLowerCase() === registry.toLowerCase();
+	if (realmUrl.protocol !== 'https:') {
+		return realmUrl.protocol === 'http:'
+			&& realmUrl.hostname.toLowerCase() === 'localhost'
+			&& sameAuthority;
+	}
+
+	if (sameAuthority) {
+		return true;
+	}
+
+	if (realmUrl.port) {
+		return false;
+	}
+
+	// Cross-authority services must use their standard HTTPS authority.
+	const hostname = realmUrl.hostname.toLowerCase();
+	const azureRegistryLabels = hostname.endsWith('.azurecr.io')
+		? hostname.slice(0, -'.azurecr.io'.length).split('.')
+		: [];
+	return hostname === 'auth.docker.io'
+		|| hostname === 'ghcr.io'
+		|| azureRegistryLabels.length > 0 && azureRegistryLabels.every(Boolean);
+}
+
 // https://docs.docker.com/registry/spec/auth/token/#how-to-authenticate
 export async function requestEnsureAuthenticated(params: CommonParams, httpOptions: { type: string; url: string; headers: HEADERS; data?: Buffer }, ociRef: OCIRef | OCICollectionRef) {
 	// If needed, Initialize the Authorization header cache.
@@ -100,6 +163,11 @@ export async function requestEnsureAuthenticated(params: CommonParams, httpOptio
 				output.write(`[httpOci] WWW-Authenticate header is not in expected format. Got:  ${wwwAuthenticate}`, LogLevel.Trace);
 				return;
 			}
+			// Reject the challenge before credential lookup or token-endpoint I/O.
+			if (!isAllowedTokenServiceRealm(realmGroup[1], ociRef.registry)) {
+				output.write(`[httpOci] ERR: Refusing bearer token realm '${realmGroup[1]}' for registry '${ociRef.registry}'.`, LogLevel.Error);
+				return;
+			}
 
 			const wwwAuthenticateData = {
 				realm: realmGroup[1],
@@ -335,15 +403,6 @@ async function fetchRegistryBearerToken(params: CommonParams, ociRef: OCIRef | O
 	const { output } = params;
 	const { realm, service, scope } = wwwAuthenticateData;
 
-	// TODO: Remove this.
-	if (realm.includes('mcr.microsoft.com')) {
-		return undefined;
-	}
-
-	const headers: HEADERS = {
-		'user-agent': 'devcontainer'
-	};
-
 	// The token server should first attempt to authenticate the client using any authentication credentials provided with the request.
 	// From Docker 1.11 the Docker engine supports both Basic Authentication and OAuth2 for getting tokens. 
 	// Docker 1.10 and before, the registry client in the Docker Engine only supports Basic Authentication. 
@@ -353,61 +412,87 @@ async function fetchRegistryBearerToken(params: CommonParams, ociRef: OCIRef | O
 	const userCredential = await getCredential(params, ociRef);
 	const basicAuthCredential = userCredential?.base64EncodedCredential;
 	const refreshToken = userCredential?.refreshToken;
+	const canForwardBasicCredential = canForwardCredentialToTokenService(realm, ociRef.registry, 'basic');
+	const canForwardRefreshToken = canForwardCredentialToTokenService(realm, ociRef.registry, 'refreshToken');
 
 	let httpOptions: { type: string; url: string; headers: Record<string, string>; data?: Buffer };
+	let sentCredentials = false;
+
+	const createGetHttpOptions = (authorization?: string) => {
+		// URLSearchParams preserves existing realm parameters and encodes challenge values.
+		const url = new URL(realm);
+		url.searchParams.set('service', service);
+		url.searchParams.set('scope', scope);
+
+		const headers: Record<string, string> = {
+			'user-agent': 'devcontainer',
+		};
+		if (authorization) {
+			headers.authorization = authorization;
+		}
+
+		return {
+			type: 'GET',
+			url: url.toString(),
+			headers,
+		};
+	};
+
+	if (refreshToken && !canForwardRefreshToken) {
+		output.write(`[httpOci] Refusing to send refresh token to bearer token realm '${realm}' for registry '${ociRef.registry}'.`, LogLevel.Warning);
+	}
+	if (basicAuthCredential && !canForwardBasicCredential) {
+		output.write(`[httpOci] Refusing to send Basic credential to bearer token realm '${realm}' for registry '${ociRef.registry}'.`, LogLevel.Warning);
+	}
 
 	// There are several different ways registries expect to handle the oauth token exchange. 
 	// Depending on the type of credential available, use the most reasonable method.
-	if (refreshToken) {
+	if (refreshToken && canForwardRefreshToken) {
 		const form_url_encoded = new URLSearchParams();
 		form_url_encoded.append('client_id', 'devcontainer');
 		form_url_encoded.append('grant_type', 'refresh_token');
 		form_url_encoded.append('service', service);
 		form_url_encoded.append('scope', scope);
 		form_url_encoded.append('refresh_token', refreshToken);
 
-		headers['content-type'] = 'application/x-www-form-urlencoded';
-
 		const url = realm;
 		output.write(`[httpOci] Attempting to fetch bearer token from:  ${url}`, LogLevel.Trace);
 
 		httpOptions = {
 			type: 'POST',
 			url,
-			headers: headers,
+			headers: {
+				'user-agent': 'devcontainer',
+				'content-type': 'application/x-www-form-urlencoded',
+			},
 			data: Buffer.from(form_url_encoded.toString())
 		};
+		sentCredentials = true;
 	} else {
-		if (basicAuthCredential) {
-			headers['authorization'] = `Basic ${basicAuthCredential}`;
-		}
-
 		// realm="https://auth.docker.io/token"
 		// service="registry.docker.io"
 		// scope="repository:samalba/my-app:pull,push"
 		// Example:
 		// https://auth.docker.io/token?service=registry.docker.io&scope=repository:samalba/my-app:pull,push
-		const url = `${realm}?service=${service}&scope=${scope}`;
-		output.write(`[httpOci] Attempting to fetch bearer token from:  ${url}`, LogLevel.Trace);
-
-		httpOptions = {
-			type: 'GET',
-			url: url,
-			headers: headers,
-		};
+		const authorization = basicAuthCredential && canForwardBasicCredential
+			? `Basic ${basicAuthCredential}`
+			: undefined;
+		httpOptions = createGetHttpOptions(authorization);
+		sentCredentials = !!authorization;
+		output.write(`[httpOci] Attempting to fetch bearer token from:  ${httpOptions.url}`, LogLevel.Trace);
 	}
 
-	let res = await requestResolveHeaders(httpOptions, output);
-	if (res && res.statusCode === 401 || res.statusCode === 403) {
+	let res = await requestResolveHeadersNoRedirects(httpOptions, output);
+	if (sentCredentials && (res.statusCode === 401 || res.statusCode === 403)) {
 		output.write(`[httpOci] ${res.statusCode}: Credentials for '${service}' may be expired. Attempting request anonymously.`, LogLevel.Info);
 		const body = res.resBody?.toString();
 		if (body) {
 			output.write(`${res.resBody.toString()}.`, LogLevel.Info);
 		}
 
-		// Try again without user credentials. If we're here, their creds are likely expired.
-		delete headers['authorization'];
-		res = await requestResolveHeaders(httpOptions, output);
+		// Build a fresh GET so neither an Authorization header nor a refresh-token POST body is reused.
+		httpOptions = createGetHttpOptions();
+		res = await requestResolveHeadersNoRedirects(httpOptions, output);
 	}
 
 	if (!res || res.statusCode > 299 || !res.resBody) {

src/spec-utils/httpRequest.ts

@@ -79,9 +79,25 @@ export async function headRequest(options: { url: string; headers: Record<string
 	});
 }
 
+type RequestResolveHeadersOptions = {
+	type: string;
+	url: string;
+	headers: Record<string, string>;
+	data?: Buffer;
+};
+
 // Send HTTP Request.
 // Does not throw on status code, but rather always returns 'statusCode', 'resHeaders', and 'resBody'.
-export async function requestResolveHeaders(options: { type: string; url: string; headers: Record<string, string>; data?: Buffer }, output: Log) {
+export async function requestResolveHeaders(options: RequestResolveHeadersOptions, output: Log) {
+	return requestResolveHeadersInternal(options, output);
+}
+
+// Token endpoints must not redirect around their validated authority boundary.
+export async function requestResolveHeadersNoRedirects(options: RequestResolveHeadersOptions, output: Log) {
+	return requestResolveHeadersInternal(options, output, 0);
+}
+
+async function requestResolveHeadersInternal(options: RequestResolveHeadersOptions, output: Log, maxRedirects?: number) {
 	const secureContext = await secureContextWithExtraCerts(output);
 	return new Promise<{ statusCode: number; resHeaders: Record<string, string>; resBody: Buffer }>((resolve, reject) => {
 		const parsed = new url.URL(options.url);
@@ -95,6 +111,9 @@ export async function requestResolveHeaders(options: { type: string; url: string
 			agent: new ProxyAgent(),
 			secureContext,
 		};
+		if (maxRedirects !== undefined) {
+			reqOptions.maxRedirects = maxRedirects;
+		}
 
 		const plainHTTP = parsed.protocol === 'http:' || parsed.hostname === 'localhost';
 		if (plainHTTP) {