Security Enforcement for Trusted Registries: Only allow module restore from trusted registries#19395
Conversation
| /// Validates a single trusted registry pattern string. | ||
| /// Returns null if the pattern is valid, or an error message string if it is invalid. | ||
| /// </summary> | ||
| public static string? ValidateRegistryPattern(string pattern) |
There was a problem hiding this comment.
Are there any libraries we can use for this validation and host matching?
There was a problem hiding this comment.
I haven't found any library that can do the matching out of the box yet. I checked the System.Uri library but it requires a scheme (like "https://") to construct the hostname and doesn't accept ".azurecr.io" hostnames with wildcards. Also some of the hostname match checks make it hard to adopt a library, and manual checks may still be needed. For example:
Accept ".azurecrl.io" and reject "*.com",
Match "contoso.azurecr.io" against *.azurecr.io", but does not match "azurecr.io" against *.azurecr.io"
| /// <see cref="GetConfiguration"/> re-scan the file system. Useful in tests where the | ||
| /// virtual file system is modified between compilation runs. | ||
| /// </summary> | ||
| void PurgeLookupCache() { } |
There was a problem hiding this comment.
I wonder whether we can avoid exposing implementation details through the interface. If the test needs to reset the configuration cache, we could instead cast the interface to ConfigurationManager. A better approach might be to inject IConcurrentCache into ConfigurationManager, and let the tests clear the cache via IConcurrentCache.TryRemove.
There was a problem hiding this comment.
Sure, I have removed the exposing API from the interface and instead make changes in the test code to solve the stall cache issue for parallel testcases running.
jiangmingzhe
force-pushed
the
mingzhejiang/module-restore-guard
branch
from
eec9fc1 to
ea1f9a4
Compare
| ? module.Registry[..module.Registry.IndexOf(':')] // "localhost:5000" → "localhost" | ||
| : module.Registry; | ||
| var publishSecurityJson = JsonDocument.Parse( | ||
| $"{{\"trustedRegistries\":[\"{registryHostOnly}\"]}}").RootElement; |
There was a problem hiding this comment.
JsonDocument implements IDisposable and may lead to memory not being released promptly if not disposed. We may need to change all JsonDocument.Parse().RootElement to use JsonElementFactory.CreateElement instead.
There was a problem hiding this comment.
Sure, I will fix this
…e from truested registries
jiangmingzhe
force-pushed
the
mingzhejiang/module-restore-guard
branch
from
9a85323 to
62e06f3
Compare
|
Test this change out locally with the following install scripts (Action run 25198811164) VSCode
Azure CLI
|
3 participants
Description: Block credential exfiltration via untrusted OCI registry restore
Problem
Opening an untrusted
.bicepfile triggers automatic OCI module restore, which sends Azure credentials to any referenced registry — including attacker-controlled ones — via the authenticate challenge. Simply opening a file is sufficient to exfiltrate credentials.Solution
A registry trust allowlist enforced before any network I/O. Restore is blocked entirely for registries not on the list — no connection, no credential challenge.
Built-in trusted registries (
*.azurecr.io,*.azurecr.cn,*.azurecr.us,mcr.microsoft.com,mcr.azure.cn,ghcr.io) are hardcoded. Users extend the list viasecurity.trustedRegistriesinbicepconfig.json.Two new diagnostics:
BCP446(registry not trusted) andBCP447(invalid pattern in config). Invalid patterns fail closed — all restore blocked until fixed.Trust is checked against the literal hostname string in the source file. No DNS resolution is performed, preventing DNS rebinding attacks.
Checklist
Microsoft Reviewers: Open in CodeFlow