@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Fix protocol serve detection for clustered repositories that terminate HTTPS

Summary:
Ref T10927. Pretty sure the issue is:

- User makes an HTTPS request.
- Load balancer terminates it, but with an `X-Forwarded-Proto` header.
- `secure001` (or whatever; acting as web host) proxies it to `secure002` (or whatever; acting as a repository host). **This** connection is plain HTTP.
- Since this proxied connection is plain HTTP, we check if the repository can serve over "http", but it can't: only "https". So we fail incorrectly, even though the original user request was HTTPS.

In the long run we should probably forward the `X-Forwarded-Proto` header, but that has some weird implications and it's broadly fine to allow either protocol to serve as long as the other one is active: configuration like `security.require-https` is already stronger than these settings.

Test Plan: This is likely only observable in production, but normal cloning still works locally.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10927

Differential Revision: https://secure.phabricator.com/D15856

+14 -7
+14 -7
src/applications/diffusion/controller/DiffusionServeController.php
··· 267 267 // token from SSH. If they're using HTTP username + password auth, they 268 268 // have to obey the normal HTTP rules. 269 269 } else { 270 - if ($request->isHTTPS()) { 271 - $protocol = PhabricatorRepositoryURI::BUILTIN_PROTOCOL_HTTPS; 272 - } else { 273 - $protocol = PhabricatorRepositoryURI::BUILTIN_PROTOCOL_HTTP; 274 - } 270 + // For now, we don't distinguish between HTTP and HTTPS-originated 271 + // requests that are proxied within the cluster, so the user can connect 272 + // with HTTPS but we may be on HTTP by the time we reach this part of 273 + // the code. Allow things to move forward as long as either protocol 274 + // can be served. 275 + $proto_https = PhabricatorRepositoryURI::BUILTIN_PROTOCOL_HTTPS; 276 + $proto_http = PhabricatorRepositoryURI::BUILTIN_PROTOCOL_HTTP; 275 277 276 - if (!$repository->canServeProtocol($protocol, false)) { 278 + $can_read = 279 + $repository->canServeProtocol($proto_https, false) || 280 + $repository->canServeProtocol($proto_http, false); 281 + if (!$can_read) { 277 282 return new PhabricatorVCSResponse( 278 283 403, 279 284 pht('This repository is not available over HTTP.')); 280 285 } 281 286 282 287 if ($is_push) { 283 - $can_write = $repository->canServeProtocol($protocol, true); 288 + $can_write = 289 + $repository->canServeProtocol($proto_https, true) || 290 + $repository->canServeProtocol($proto_http, true); 284 291 if (!$can_write) { 285 292 return new PhabricatorVCSResponse( 286 293 403,