Take redirects off the Host header.

Same-site redirects are now relative paths, and a saved page on another site is ignored. The HTTP to HTTPS redirect takes its host from the new SITE_URL setting, and without it the pages refuse plain HTTP.

.env.example

@@ -7,6 +7,10 @@ DB_USER=xxx
 DB_PASS=
 DB_NAME=xxx
 
+# Address of the site. The pages send plain HTTP requests to this host
+# over HTTPS. If it is empty, the pages refuse plain HTTP.
+SITE_URL=https://xxx.org.za
+
 # Google reCAPTCHA v2 keys (https://www.google.com/recaptcha/admin).
 # Leave RECAPTCHA_SITE_KEY empty to turn the captcha off.
 RECAPTCHA_SITE_KEY=

config.lib.php

@@ -40,6 +40,25 @@ function config($key, $default = NULL) {
   return array_key_exists($key, $values) ? $values[$key] : $default;
 }
 
+/*
+ * Send a plain HTTP request to the same page over HTTPS, and stop. The
+ * host comes from SITE_URL, because the browser controls the Host header.
+ * Without SITE_URL, the page refuses plain HTTP.
+ */
+function require_https() {
+  if (isset($_SERVER['HTTPS']))
+    return;
+  $site = parse_url((string)config('SITE_URL', ''));
+  if (empty($site['host'])) {
+    http_response_code(403);
+    echo "This page needs HTTPS.\n";
+  } else {
+    $port = isset($site['port']) ? ':' . $site['port'] : '';
+    header('Location: https://' . $site['host'] . $port . $_SERVER['REQUEST_URI']);
+  }
+  exit;
+}
+
 // Always log PHP errors; show them only if DISPLAY_ERRORS is set (for development)
 ini_set('log_errors', '1');
 ini_set('display_errors', (string)config('DISPLAY_ERRORS', '0'));

publish/addeditor.php

@@ -1,15 +1,13 @@
 <?php 
-if ( ! isset($_SERVER['HTTPS'])) {
+require_once('../config.lib.php');
-   header('Location: https://' . $_SERVER["SERVER_NAME"] . $_SERVER['REQUEST_URI']);
+require_https();
-   exit;
-}
 require_once("../session.lib.php"); 
 $added = FALSE;
 $csrf_failed = FALSE;
 $hash_failed = FALSE;
 $exists = FALSE;
 if (!isset($_SESSION['editor'])) {
-  header("Location: https://xxx.org.za/publish/login");
+  header("Location: /publish/login");
   return;
 } else if (isset($_POST['adduser']) && !csrf_valid()) {
   $csrf_failed = TRUE;
@@ -73,7 +71,7 @@ if (!isset($_SESSION['editor'])) {
   <p><strong>The editor was not added: the editors.password column is too short for the password hash. Widen it to VARCHAR(255).</strong></p>
 <?php } ?>
 
-<form action="<?= form_value("https://xxx.org.za".$_SERVER['REQUEST_URI']) ?>" method="post">
+<form action="" method="post">
   <?= csrf_field() ?>
   <div class="grid">
 

publish/edit.php

@@ -1,13 +1,11 @@
 <?php
-if ( ! isset($_SERVER['HTTPS'])) {
+require_once('../config.lib.php');
-   header('Location: https://' . $_SERVER["SERVER_NAME"] . $_SERVER['REQUEST_URI']);
+require_https();
-   exit;
-}
 require_once("../session.lib.php"); 
 if (!isset($_SESSION['editor'])) {
   $is_admin = FALSE;
   session_write_close();
-  header("Location: https://xxx.org.za/publish/login");
+  header("Location: /publish/login");
   return;
 }
 ?>

publish/index.php

@@ -1,8 +1,6 @@
 <?php
-if ( ! isset($_SERVER['HTTPS'])) {
+require_once('../config.lib.php');
-   header('Location: https://' . $_SERVER["SERVER_NAME"] . $_SERVER['REQUEST_URI']);
+require_https();
-   exit;
-}
 require_once("../session.lib.php");
 ?>
 <!DOCTYPE html>

publish/login.php

@@ -1,8 +1,6 @@
 <?php
-if ( ! isset($_SERVER['HTTPS'])) {
+require_once('../config.lib.php');
-   header('Location: https://' . $_SERVER["SERVER_NAME"] . $_SERVER['REQUEST_URI']);
+require_https();
-   exit;
-}
 require_once("../session.lib.php");
 $denied = NULL;
 $wait_minutes = 0;
@@ -10,7 +8,7 @@ $captcha_failed = FALSE;
 $is_admin = NULL;
 if (isset($_SESSION['editor'])) {
   session_write_close();
-  header("Location: https://xxx.org.za/publish/edit");
+  header("Location: /publish/edit");
   return;
 } else if (isset($_POST['login']) && !csrf_valid()) {
   $denied = TRUE;
@@ -51,19 +49,14 @@ if (isset($_SESSION['editor'])) {
     if (isset($_POST['remember'])) {
       create_persistent_login($username);
     }
-    if (isset($_SESSION['referrer'])) {
+    $referrer = (string)($_SESSION['referrer'] ?? '');
-      $url  = $_SERVER['HTTPS'] ? "https://" : "http://";
-      $url .= $_SERVER['HTTP_HOST'];
-      $url .= $_SESSION['referrer'];
     unset($_SESSION['referrer']);
     session_write_close();
-      header("Location: $url");
+    if (is_local_path($referrer))
+      header('Location: ' . $referrer);
+    else
+      header("Location: /publish/edit");
     return;
-    } else {
-      session_write_close();
-      header("Location: https://xxx.org.za/publish/edit");
-      return;
-    }
   }
 } 
 
@@ -100,7 +93,7 @@ else if ($denied)
   echo "<p><strong>Access denied. Please try again.</strong></p>\n";
 ?>
 
-<form action="<?php echo form_value("https://xxx.org.za".$_SERVER['REQUEST_URI']); ?>" method="post">
+<form action="" method="post">
   <?= csrf_field() ?>
   <div class="grid">
 

publish/logout.php

@@ -4,13 +4,11 @@ header( 'Cache-Control: no-store, no-cache, must-revalidate' );
 header( 'Cache-Control: post-check=0, pre-check=0', false );
 header( 'Pragma: no-cache' ); 
 include_once('../session.lib.php');
-if ( ! isset($_SERVER['HTTPS'])) {
+require_once('../config.lib.php');
-   header('Location: https://' . $_SERVER["SERVER_NAME"] . $_SERVER['REQUEST_URI']);
+require_https();
-   exit;
-}
 // Only log out from our own logout link, which carries the CSRF token
 if (!csrf_valid($_GET['token'] ?? '')) {
-  header("Location: https://xxx.org.za/");
+  header("Location: /");
   exit;
 }
 if (isset($_SESSION['editor'])) {
@@ -30,18 +28,11 @@ if (isset($_SESSION['editor'])) {
   session_destroy();
 }
 delete_persistent_login();
-if (isset($_SESSION['referrer'])) {
+$referrer = (string)($_SESSION['referrer'] ?? '');
-  $url  = $_SERVER['HTTPS'] ? "https://" : "http://";
+unset($_SESSION['referrer']);
-  $url .= $_SERVER['HTTP_HOST'];
+if (is_local_path($referrer) && !preg_match("/logout(\.php)?$/i", $referrer))
-  $url .= $_SESSION['referrer'];
+  header('Location: ' . $referrer);
-  unset($_SESSION['referrer']);
+else
-  if (preg_match("/logout(\.php)?$/i", $url)) {
+  header("Location: /");
-    $url = "/";
+return;
-  }
-  header("Location: $url");
-  return;
-} else {
-  header("Location: https://xxx.org.za/");
-  return;
-}
 ?>

session.lib.php

@@ -136,6 +136,15 @@ function csrf_valid($token = NULL) {
          hash_equals((string)$_SESSION['csrf_token'], $token);
 }
 
+/*
+ * TRUE for a path on this site, such as "/article?id=1". A browser sends
+ * a redirect to "//host" or "/\host" to another site, so those are not
+ * local paths.
+ */
+function is_local_path($path) {
+  return is_string($path) && preg_match('#^/(?![/\\\\])#', $path) === 1;
+}
+
 /*
  * Log an editor in under a new session id, so that a session id that
  * someone planted or saw before the login is of no use afterwards.

tests/php/Pages/AddEditorTest.php

@@ -20,13 +20,13 @@ class AddEditorTest extends PageTestCase {
 
   public function testVisitorsAreSentToTheLogin(): void {
     $this->cookies = array();
-    $this->assertRedirect('https://xxx.org.za/publish/login', $this->get('/publish/addeditor'));
+    $this->assertRedirect('/publish/login', $this->get('/publish/addeditor'));
-    $this->assertRedirect('https://xxx.org.za/publish/login', $this->add(array('csrf_token' => '')));
+    $this->assertRedirect('/publish/login', $this->add(array('csrf_token' => '')));
     $this->assertCount(1, $this->tables()['editors']);
   }
 
   public function testPlainHttpIsSentToHttps(): void {
-    $this->assertRedirect('https://xxx.org.za/publish/addeditor', $this->get('/publish/addeditor', array('X-Test-Plain' => '1')));
+    $this->assertRedirect('https://site.test/publish/addeditor', $this->get('/publish/addeditor', array('X-Test-Plain' => '1')));
   }
 
   public function testTheFormHasTheToken(): void {

tests/php/Pages/EditTest.php

@@ -23,11 +23,11 @@ class EditTest extends PageTestCase {
 
   public function testVisitorsAreSentToTheLogin(): void {
     $this->cookies = array();
-    $this->assertRedirect('https://xxx.org.za/publish/login', $this->get('/publish/edit'));
+    $this->assertRedirect('/publish/login', $this->get('/publish/edit'));
   }
 
   public function testPlainHttpIsSentToHttps(): void {
-    $this->assertRedirect('https://xxx.org.za/publish/edit?id=1', $this->get('/publish/edit?id=1', array('X-Test-Plain' => '1')));
+    $this->assertRedirect('https://site.test/publish/edit?id=1', $this->get('/publish/edit?id=1', array('X-Test-Plain' => '1')));
   }
 
   public function testTheQueueIsShownFirst(): void {

tests/php/Pages/HttpsTest.php

@@ -0,0 +1,23 @@
+<?php
+namespace Xxx\Tests\Pages;
+
+use Xxx\Tests\Support\PageTestCase;
+
+// require_https() in config.lib.php when SITE_URL is empty
+class HttpsTest extends PageTestCase {
+  protected static function serverEnv(): array {
+    // proc_open() drops a variable with an empty value, so use a blank one
+    return array('SITE_URL' => ' ');
+  }
+
+  public function testWithoutASiteUrlPlainHttpIsRefused(): void {
+    $answer = $this->get('/publish/login', array('X-Test-Plain' => '1'));
+    $this->assertSame(403, $answer['status']);
+    $this->assertNull($this->header($answer, 'Location'));
+    $this->assertSame("This page needs HTTPS.\n", $answer['body']);
+  }
+
+  public function testWithoutASiteUrlHttpsStillWorks(): void {
+    $this->assertSame(200, $this->get('/publish/login')['status']);
+  }
+}

tests/php/Pages/LoginTest.php

@@ -1,6 +1,7 @@
 <?php
 namespace Xxx\Tests\Pages;
 
+use PHPUnit\Framework\Attributes\DataProvider;
 use Xxx\Tests\Support\PageTestCase;
 
 // publish/login.php, publish/logout.php and persistent logins
@@ -25,16 +26,17 @@ class LoginTest extends PageTestCase {
    * sends an editor on to the edit page.
    */
   private function assertLoggedIn(array $answer): void {
-    $this->assertRedirect('https://' . $this->host() . '/publish/login', $answer);
+    $this->assertRedirect('/publish/login', $answer);
-    $this->assertRedirect('https://xxx.org.za/publish/edit', $this->get('/publish/login'));
+    $this->assertRedirect('/publish/edit', $this->get('/publish/login'));
-  }
-
-  private function host(): string {
-    return parse_url(self::$base, PHP_URL_HOST) . ':' . parse_url(self::$base, PHP_URL_PORT);
   }
 
   public function testPlainHttpIsSentToHttps(): void {
-    $this->assertRedirect('https://xxx.org.za/publish/login', $this->get('/publish/login', array('X-Test-Plain' => '1')));
+    $this->assertRedirect('https://site.test/publish/login', $this->get('/publish/login', array('X-Test-Plain' => '1')));
+  }
+
+  public function testAForgedHostDoesNotMoveTheHttpsRedirect(): void {
+    $this->assertRedirect('https://site.test/publish/login',
+                          $this->get('/publish/login', array('X-Test-Plain' => '1', 'Host' => 'evil.example')));
   }
 
   public function testTheFormHasATokenAndTheCaptcha(): void {
@@ -60,16 +62,38 @@ class LoginTest extends PageTestCase {
     $this->get('/article?id=1');
     $answer = $this->post('/publish/login', array('login' => '1', 'username' => 'ed', 'password' => 'secret',
                                                    'csrf_token' => $token, 'g-recaptcha-response' => 'pass'));
-    $this->assertRedirect('https://' . $this->host() . '/article?id=1', $answer);
+    $this->assertRedirect('/article?id=1', $answer);
     $this->assertArrayNotHasKey('referrer', $this->session());
   }
 
+  public function testAForgedHostDoesNotMoveTheLoginRedirect(): void {
+    $token = $this->csrfToken($this->get('/publish/login')['body']);
+    $this->get('/article?id=1');
+    $answer = $this->request('POST', '/publish/login', array('login' => '1', 'username' => 'ed', 'password' => 'secret',
+                                                             'csrf_token' => $token, 'g-recaptcha-response' => 'pass'),
+                             array('Host' => 'evil.example'));
+    $this->assertRedirect('/article?id=1', $answer);
+  }
+
+  public static function otherSites(): array {
+    return array(array('@evil.example/'), array('//evil.example/'), array('/\\evil.example/'), array('https://evil.example/'));
+  }
+
+  #[DataProvider('otherSites')]
+  public function testALastPageOnAnotherSiteIsIgnored(string $referrer): void {
+    $token = $this->csrfToken($this->get('/publish/login')['body']);
+    $this->startSession(array('csrf_token' => $token, 'referrer' => $referrer));
+    $answer = $this->post('/publish/login', array('login' => '1', 'username' => 'ed', 'password' => 'secret',
+                                                   'csrf_token' => $token, 'g-recaptcha-response' => 'pass'));
+    $this->assertRedirect('/publish/edit', $answer);
+  }
+
   public function testWithoutALastPageTheLoginGoesToTheEditPage(): void {
     $token = $this->csrfToken($this->get('/publish/login')['body']);
     $this->startSession(array('csrf_token' => $token));
     $answer = $this->post('/publish/login', array('login' => '1', 'username' => 'ed', 'password' => 'secret',
                                                    'csrf_token' => $token, 'g-recaptcha-response' => 'pass'));
-    $this->assertRedirect('https://xxx.org.za/publish/edit', $answer);
+    $this->assertRedirect('/publish/edit', $answer);
   }
 
   public function testTheWrongPasswordIsDenied(): void {
@@ -125,7 +149,7 @@ class LoginTest extends PageTestCase {
 
   public function testAnEditorWhoIsLoggedInGoesToTheEditPage(): void {
     $this->loginAs();
-    $this->assertRedirect('https://xxx.org.za/publish/edit', $this->get('/publish/login'));
+    $this->assertRedirect('/publish/edit', $this->get('/publish/login'));
   }
 
   public function testRememberMeKeepsTheEditorLoggedIn(): void {
@@ -151,7 +175,7 @@ class LoginTest extends PageTestCase {
 
   public function testLogoutNeedsTheToken(): void {
     $this->loginAs();
-    $this->assertRedirect('https://xxx.org.za/', $this->get('/publish/logout?token=wrong'));
+    $this->assertRedirect('/', $this->get('/publish/logout?token=wrong'));
     $this->assertSame('ed', $this->session()['editor']);
   }
 
@@ -164,13 +188,25 @@ class LoginTest extends PageTestCase {
     $session = self::$state . '/sessions/sess_' . $this->cookies['PHPSESSID'];
 
     $answer = $this->get('/publish/logout?token=' . $token);
-    $this->assertRedirect('https://' . $this->host() . '/article?id=2', $answer);
+    $this->assertRedirect('/article?id=2', $answer);
     $this->assertFileDoesNotExist($session);
     $this->assertArrayNotHasKey('PHPSESSID', $this->cookies);
     $this->assertArrayNotHasKey('PersistentAuth', $this->cookies);
     $this->assertSame(array(hash('sha256', 'other device')), array_column($this->tables()['persistent_sessions'], 'session'));
   }
 
+  public function testAForgedHostDoesNotMoveTheLogoutRedirect(): void {
+    $token = $this->loginAs('ed', array('referrer' => '/article?id=2'));
+    $this->assertRedirect('/article?id=2',
+                          $this->get('/publish/logout?token=' . $token, array('Host' => 'evil.example')));
+  }
+
+  #[DataProvider('otherSites')]
+  public function testLogoutIgnoresALastPageOnAnotherSite(string $referrer): void {
+    $token = $this->loginAs('ed', array('referrer' => $referrer));
+    $this->assertRedirect('/', $this->get('/publish/logout?token=' . $token));
+  }
+
   public function testLogoutDoesNotGoBackToTheLogoutPage(): void {
     $token = $this->loginAs('ed', array('referrer' => '/publish/logout'));
     $this->assertRedirect('/', $this->get('/publish/logout?token=' . $token));

tests/php/Pages/SubmitTest.php

@@ -16,7 +16,7 @@ class SubmitTest extends PageTestCase {
   }
 
   public function testPlainHttpIsSentToHttps(): void {
-    $this->assertRedirect('https://xxx.org.za/publish', $this->get('/publish', array('X-Test-Plain' => '1')));
+    $this->assertRedirect('https://site.test/publish', $this->get('/publish', array('X-Test-Plain' => '1')));
   }
 
   public function testTheFormHasATokenAndTheCaptcha(): void {

tests/php/Support/PageTestCase.php

@@ -15,6 +15,11 @@ abstract class PageTestCase extends \PHPUnit\Framework\TestCase {
   protected static string $base;
   private static $server = NULL;
 
+  // Environment variables for the server, on top of the usual ones
+  protected static function serverEnv(): array {
+    return array();
+  }
+
   // The cookies that the site set, name => value
   protected array $cookies = array();
 
@@ -43,7 +48,7 @@ abstract class PageTestCase extends \PHPUnit\Framework\TestCase {
       'XXX_TEST_LOG'    => self::$state . '/php.log',
       'MAIL_LOG'        => self::$state . '/mail.jsonl',
       'UPLOAD_DIR'      => self::$state . '/tmp/uploads',
-    );
+    ) + static::serverEnv();
     $log = array('file', self::$state . '/server.log', 'a');
     self::$server = proc_open($command, array(0 => array('file', '/dev/null', 'r'), 1 => $log, 2 => $log), $pipes, XXX_ROOT, $env);
 

tests/php/fixtures/router.php

@@ -47,7 +47,8 @@ function xxx_page_route() {
 
   if (!isset($_SERVER['HTTP_X_TEST_PLAIN']))
     $_SERVER['HTTPS'] = 'on';
-  $_SERVER['SERVER_NAME'] = 'xxx.org.za';
+  // Apache with UseCanonicalName Off also takes SERVER_NAME from the Host header
+  $_SERVER['SERVER_NAME'] = preg_replace('/:\d+$/', '', $_SERVER['HTTP_HOST'] ?? '');
   $_SERVER['REMOTE_ADDR'] = $_SERVER['HTTP_X_TEST_ADDRESS'] ?? '192.0.2.1';
   $_SERVER['SCRIPT_FILENAME'] = $file;
   $_SERVER['SCRIPT_NAME'] = $_SERVER['PHP_SELF'] = substr($file, strlen(XXX_ROOT));

tests/php/fixtures/test.env

@@ -3,6 +3,7 @@ DB_HOST=db.test
 DB_USER=test-user
 DB_PASS=test-pass
 DB_NAME=test-db
+SITE_URL=https://site.test
 RECAPTCHA_SITE_KEY=test-site-key
 RECAPTCHA_SECRET=test-secret
 GOOGLE_ANALYTICS_ID=