Skip to content

Commit 1a375d4

Browse files
authored
fix: scope cleartext-scheme enforcement so local-scene-development fetch is not broken
Scopes the cleartext-scheme policy so it no longer over-upgrades unsigned local-scene-development scene fetches (the Creator Hub preview / SDK network-testing flow). - RequestEnvelope: EnforceSecureScheme now runs only on signed requests (identity auth never travels over cleartext); unsigned wire URLs pass through — their scheme is decided at the infra resolution site or the fetch module's dev-mode gate. - WebRequestController: redirect guard changed to IsCleartextDowngrade(sentUrl, finalUrl) so a deliberately-sent cleartext request (dev fetch) is not misread as a mid-flight downgrade. - WebRequestUtils: adds IsCleartextDowngrade. v16 EditMode RED/GREEN (DCL.WebRequests.Tests.InsecureSchemePolicyShould): over-scoped global hook = 34 total / 3 failed (the non-loopback-http + texture passthrough guards); scoped fix = 34/34 passed. Loopback preview still works, production scene fetch still blocked by the module dev-mode gate, signed/infra requests still upgraded.
1 parent 141faae commit 1a375d4

5 files changed

Lines changed: 103 additions & 33 deletions

File tree

Explorer/Assets/DCL/SDKComponents/MediaStream/UrlResolverService.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,8 @@ private async UniTask<ResolvedMediaUrl> ResolveDirectUrlAsync(string url, Report
101101

102102
private static async UniTask<bool> IsGetReachableAsync(string url, CancellationToken ct)
103103
{
104-
// This request bypasses IWebRequestController, so the pre-send transport-security
105-
// policy the controller applies must hold here too: no cleartext to non-loopback hosts
104+
// This request bypasses IWebRequestController, so the media transport-security policy
105+
// (no cleartext to non-loopback hosts) binds directly at this send site
106106
UnityWebRequest request = UnityWebRequest.Get(WebRequestUtils.EnforceSecureScheme(url));
107107

108108
try

Explorer/Assets/DCL/WebRequests/RequestEnvelope.cs

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -98,16 +98,20 @@ public TWebRequest InitializedWebRequest(IWeb3IdentityCache web3IdentityCache)
9898
TWebRequest request = initializeRequest(CommonArguments.URL, ref args);
9999
UnityWebRequest unityWebRequest = request.UnityWebRequest;
100100

101-
// The scheme policy governs the wire URL of the request about to be sent; running it
102-
// after per-request composition keeps origin URLs embedded in it as data (e.g. the
103-
// media-converter's url query parameter) byte-identical
104-
string wireUrl = unityWebRequest.url;
105-
string secureUrl = WebRequestUtils.EnforceSecureScheme(wireUrl);
106-
107-
if (!ReferenceEquals(wireUrl, secureUrl))
101+
if (signInfo.HasValue)
108102
{
109-
ReportHub.LogWarning(ReportData, $"Cleartext http to a non-loopback host upgraded to https: {wireUrl}");
110-
unityWebRequest.url = secureUrl;
103+
// The identity auth chain attached below never travels over forbidden cleartext:
104+
// a signed request's wire URL is secure-enforced before signing. Unsigned wire
105+
// URLs are not policed here — their scheme policy binds at the resolution site
106+
// (infra URLs) or in the issuing module (scene fetch)
107+
string wireUrl = unityWebRequest.url;
108+
string secureUrl = WebRequestUtils.EnforceSecureScheme(wireUrl);
109+
110+
if (!ReferenceEquals(wireUrl, secureUrl))
111+
{
112+
ReportHub.LogWarning(ReportData, $"Cleartext http on a signed request upgraded to https: {wireUrl}");
113+
unityWebRequest.url = secureUrl;
114+
}
111115
}
112116

113117
AssignTimeout(unityWebRequest);

Explorer/Assets/DCL/WebRequests/Tests/InsecureSchemePolicyShould.cs

Lines changed: 62 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using CommunicationData.URLHelpers;
22
using DCL.Diagnostics;
33
using DCL.Multiplayer.Connections.DecentralandUrls;
4+
using DCL.Web3.Chains;
45
using DCL.Web3.Identities;
56
using NSubstitute;
67
using NUnit.Framework;
@@ -11,23 +12,44 @@
1112

1213
namespace DCL.WebRequests.Tests
1314
{
14-
// Transport-security policy invariants: cleartext http is loopback-only; http to any
15-
// other host is upgraded to https on the wire URL of the built request (after per-request
16-
// URL composition, so URLs embedded in it as data survive); the player-level
15+
// Transport-security policy invariants: cleartext http is loopback-only where the policy
16+
// binds — at infra URL-resolution sites (media resolution, sidecar realm root) and on the
17+
// wire URL of signed requests (the identity auth chain never travels over forbidden
18+
// cleartext). Unsigned wire URLs pass through the envelope unchanged: their scheme is a
19+
// module-level decision (local-scene-development fetch permits cleartext to any host).
20+
// The redirect guard blocks only mid-flight downgrades, and the player-level
1721
// insecureHttpOption stays AlwaysAllowed so the client-side policy is the single
1822
// enforcement point.
1923
public class InsecureSchemePolicyShould
2024
{
2125
private const string MEDIA_CONVERTER_TEMPLATE = "https://metamorph-api.decentraland.org/convert?url={0}";
2226

27+
[TestCase("http://192.168.1.50:8000/api")]
28+
[TestCase("http://peer.decentraland.org/x")]
29+
public void PassNonLoopbackHttpThroughUnchangedWhenUnsigned(string url)
30+
{
31+
// A local-scene-development scene fetch is an unsigned request whose cleartext
32+
// scheme is the fetch module's own vetted decision; the envelope must not rewrite it
33+
Assert.That(UrlAfterEnvelopeInitialization(url), Is.EqualTo(UnityCanonicalUrl(url)));
34+
}
35+
2336
[Test]
24-
public void UpgradeNonLoopbackHttpToHttps()
37+
public void UpgradeNonLoopbackHttpToHttpsWhenSigned()
2538
{
2639
Assert.That(
27-
UrlAfterEnvelopeInitialization("http://peer.decentraland.org/x"),
40+
UrlAfterEnvelopeInitialization("http://peer.decentraland.org/x", new WebRequestSignInfo(string.Empty)),
2841
Is.EqualTo(UnityCanonicalUrl("https://peer.decentraland.org/x")));
2942
}
3043

44+
[TestCase("http://127.0.0.1:8000/content/contents/bafkreib")]
45+
[TestCase("http://localhost:8001/x")]
46+
public void PassLoopbackHttpThroughUnchangedWhenSigned(string url)
47+
{
48+
Assert.That(
49+
UrlAfterEnvelopeInitialization(url, new WebRequestSignInfo(string.Empty)),
50+
Is.EqualTo(UnityCanonicalUrl(url)));
51+
}
52+
3153
[TestCase("http://127.0.0.1:8000/content/contents/bafkreib")]
3254
[TestCase("http://127.0.0.5:8000/x")]
3355
[TestCase("http://localhost:8001/x")]
@@ -45,11 +67,13 @@ public void PassNonHttpSchemesThroughUnchanged(string url)
4567
}
4668

4769
[Test]
48-
public void UpgradeNonConvertedTextureUrlAtTheWire()
70+
public void PassNonConvertedTextureUrlThroughUnchangedAtTheWire()
4971
{
72+
const string HTTP_URL = "http://textures.example.com/a.png";
73+
5074
Assert.That(
51-
TextureUrlAfterEnvelopeInitialization("http://textures.example.com/a.png", ktxEnabled: false),
52-
Is.EqualTo(UnityCanonicalUrl("https://textures.example.com/a.png")));
75+
TextureUrlAfterEnvelopeInitialization(HTTP_URL, ktxEnabled: false),
76+
Is.EqualTo(UnityCanonicalUrl(HTTP_URL)));
5377
}
5478

5579
[Test]
@@ -93,11 +117,22 @@ public void PreserveHttpOriginEmbeddedInConverterUrl()
93117
[TestCase("http://[::1]:8000/x", false)]
94118
[TestCase("https://peer.decentraland.org/x", false)]
95119
[TestCase("file:///tmp/streaming-asset.bin", false)]
96-
public void ClassifyForbiddenCleartextForTheRedirectGuard(string url, bool forbidden)
120+
public void ClassifyForbiddenCleartext(string url, bool forbidden)
97121
{
98122
Assert.That(WebRequestUtils.IsForbiddenCleartext(url), Is.EqualTo(forbidden));
99123
}
100124

125+
[TestCase("https://peer.decentraland.org/x", "http://peer.decentraland.org/x", true)]
126+
[TestCase("http://127.0.0.1:8000/x", "http://192.168.1.50:8000/x", true)]
127+
[TestCase("http://192.168.1.50:8000/api", "http://192.168.1.50:8000/api", false)]
128+
[TestCase("http://192.168.1.50:8000/x", "http://10.0.0.7:9000/y", false)]
129+
[TestCase("https://peer.decentraland.org/x", "https://cdn.decentraland.org/x", false)]
130+
[TestCase("https://peer.decentraland.org/x", "http://127.0.0.1:8000/x", false)]
131+
public void ClassifyCleartextDowngradeForTheRedirectGuard(string sentUrl, string finalUrl, bool downgrade)
132+
{
133+
Assert.That(WebRequestUtils.IsCleartextDowngrade(sentUrl, finalUrl), Is.EqualTo(downgrade));
134+
}
135+
101136
[Test]
102137
public void KeepPlayerSettingAlwaysAllowed()
103138
{
@@ -111,7 +146,7 @@ public void KeepPlayerSettingAlwaysAllowed()
111146
/// (WebRequestController.SendAsync -> InitializedWebRequest) and returns the URL the
112147
/// UnityWebRequest would actually be sent with. The request is never sent.
113148
/// </summary>
114-
private static string UrlAfterEnvelopeInitialization(string url)
149+
private static string UrlAfterEnvelopeInitialization(string url, WebRequestSignInfo? signInfo = null)
115150
{
116151
using var envelope = new RequestEnvelope<GenericGetRequest, GenericGetArguments>(
117152
GenericGetRequest.Initialize,
@@ -120,9 +155,9 @@ private static string UrlAfterEnvelopeInitialization(string url)
120155
CancellationToken.None,
121156
ReportData.UNSPECIFIED,
122157
WebRequestHeadersInfo.NewEmpty(),
123-
signInfo: null);
158+
signInfo);
124159

125-
GenericGetRequest request = envelope.InitializedWebRequest(Substitute.For<IWeb3IdentityCache>());
160+
GenericGetRequest request = envelope.InitializedWebRequest(SigningIdentityCache());
126161
using UnityWebRequest unityWebRequest = request.UnityWebRequest;
127162
return unityWebRequest.url;
128163
}
@@ -145,11 +180,25 @@ private static string TextureUrlAfterEnvelopeInitialization(string url, bool ktx
145180
WebRequestHeadersInfo.NewEmpty(),
146181
signInfo: null);
147182

148-
GetTextureWebRequest request = envelope.InitializedWebRequest(Substitute.For<IWeb3IdentityCache>());
183+
GetTextureWebRequest request = envelope.InitializedWebRequest(SigningIdentityCache());
149184
using UnityWebRequest unityWebRequest = request.UnityWebRequest;
150185
return unityWebRequest.url;
151186
}
152187

188+
/// <summary>
189+
/// An identity cache whose identity signs any payload with an empty auth chain, so
190+
/// signed envelopes can be initialized without a real wallet.
191+
/// </summary>
192+
private static IWeb3IdentityCache SigningIdentityCache()
193+
{
194+
IWeb3Identity identity = Substitute.For<IWeb3Identity>();
195+
identity.Sign(Arg.Any<string>()).Returns(_ => AuthChain.Create());
196+
197+
IWeb3IdentityCache cache = Substitute.For<IWeb3IdentityCache>();
198+
cache.Identity.Returns(identity);
199+
return cache;
200+
}
201+
153202
/// <summary>
154203
/// UnityWebRequest applies its own URL canonicalization; comparing against the same
155204
/// canonicalization keeps the assertions about the scheme policy only.

Explorer/Assets/DCL/WebRequests/WebRequestController.cs

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ public WebRequestController(
7272
// No matter what we must release UnityWebRequest, otherwise it crashes in the destructor
7373
using UnityWebRequest wr = request.UnityWebRequest;
7474

75+
// wr.url mutates to the final hop as redirects are followed; the as-sent form
76+
// anchors the downgrade checks below
77+
string sentUrl = wr.url;
78+
7579
try
7680
{
7781
analyticsContainer.OnBeforeBudgeting(in envelope, request);
@@ -89,10 +93,11 @@ public WebRequestController(
8993
analyticsContainer.OnRequestFinished(request);
9094
}
9195

92-
// A redirect can hop from the sent https URL to cleartext http after the pre-send
93-
// scheme policy ran; a response with a forbidden-cleartext final URL is never consumed
94-
if (WebRequestUtils.IsForbiddenCleartext(wr.url))
95-
throw new InvalidOperationException($"Insecure redirect blocked: request to {envelope.CommonArguments.URL} was redirected to {wr.url}");
96+
// A redirect can hop from an allowed sent URL to forbidden cleartext; a response
97+
// from such a downgraded exchange is never consumed. An exchange sent as
98+
// cleartext on purpose (local-scene-development fetch) is not a downgrade
99+
if (WebRequestUtils.IsCleartextDowngrade(sentUrl, wr.url))
100+
throw new InvalidOperationException($"Insecure redirect blocked: request to {sentUrl} was redirected to {wr.url}");
96101

97102
if (!realmClock.HasSample)
98103
realmClock.TryRecordHttpDate(wr.GetResponseHeader(DATE_HEADER));
@@ -110,9 +115,10 @@ public WebRequestController(
110115
{
111116
analyticsContainer.OnException(request, exception);
112117

113-
// An exchange whose final URL downgraded to forbidden cleartext fails permanently:
114-
// never ignored, never retried (a retry would re-send headers over cleartext)
115-
if (WebRequestUtils.IsForbiddenCleartext(wr.url))
118+
// An exchange that downgraded from an allowed sent URL to forbidden cleartext
119+
// fails permanently: never ignored, never retried (a retry would re-send
120+
// headers over cleartext)
121+
if (WebRequestUtils.IsCleartextDowngrade(sentUrl, wr.url))
116122
throw;
117123

118124
// No result can be concluded in this case

Explorer/Assets/DCL/WebRequests/WebRequestUtils.cs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,11 @@ public static string GetResponseContentEncoding(this UnityWebRequest unityWebReq
176176
/// Client-side transport-security policy, standing in for the player-level insecure-http
177177
/// block (which is global and cannot exempt loopback): cleartext http is permitted to
178178
/// loopback hosts only (local preview servers, sidecars); http to any other host is
179-
/// upgraded to https. Applied to the wire URL of the built request, never to URLs
180-
/// embedded inside it as data. Non-http URLs pass through unchanged (same reference).
179+
/// upgraded to https. Applied where a policed URL is resolved (media resolution, sidecar
180+
/// realm root) and to the wire URL of signed requests — never to URLs embedded in a
181+
/// request as data, and never to unsigned wire URLs, whose cleartext scheme is a
182+
/// module-level decision (local-scene-development fetch). Non-http URLs pass through
183+
/// unchanged (same reference).
181184
/// </summary>
182185
public static string EnforceSecureScheme(string url) =>
183186
IsForbiddenCleartext(url)
@@ -195,6 +198,14 @@ public static bool IsForbiddenCleartext(string url) =>
195198
&& url.StartsWith(HTTP_SCHEME_PREFIX, StringComparison.OrdinalIgnoreCase)
196199
&& !(Uri.TryCreate(url, UriKind.Absolute, out Uri? uri) && IsLoopbackHost(uri.Host));
197200

201+
/// <summary>
202+
/// True when an exchange left on an allowed scheme/host but its final (post-redirect)
203+
/// URL is forbidden cleartext. A request sent to forbidden cleartext in the first place
204+
/// is not a downgrade: that scheme is the sender's own policy decision.
205+
/// </summary>
206+
public static bool IsCleartextDowngrade(string sentUrl, string finalUrl) =>
207+
!IsForbiddenCleartext(sentUrl) && IsForbiddenCleartext(finalUrl);
208+
198209
/// <summary>
199210
/// Loopback means "localhost", 127.0.0.0/8 or [::1] — the hosts a local preview
200211
/// server or sidecar can be reached on.

0 commit comments

Comments
 (0)