From 46ba59a1d2ab7909c38e66601e7a79ec4dfa81a1 Mon Sep 17 00:00:00 2001
From: Sebastian Marcet <smarcet@gmail.com>
Date: Thu, 29 Dec 2016 00:16:59 -0300
Subject: [PATCH] Removed TLD validation

in the past we received several complains about
restrictive openstackid validation related to TLD
now that validation was removed, bc not added
to much value on security.

Change-Id: I47a842416ad898b9508831ee5f6e0d59e4bf3e5e
---
 .../OpenId/Exceptions/InvalidTLDException.php |  26 ----
 app/libs/OpenId/Helpers/OpenIdUriHelper.php   |  82 +++++------
 .../Requests/OpenIdAuthenticationRequest.php  |  47 +++++--
 tests/OpenIdProtocolTest.php                  | 133 ++++++++++++++++++
 4 files changed, 199 insertions(+), 89 deletions(-)
 delete mode 100644 app/libs/OpenId/Exceptions/InvalidTLDException.php

diff --git a/app/libs/OpenId/Exceptions/InvalidTLDException.php b/app/libs/OpenId/Exceptions/InvalidTLDException.php
deleted file mode 100644
index bdb6e88e..00000000
--- a/app/libs/OpenId/Exceptions/InvalidTLDException.php
+++ /dev/null
@@ -1,26 +0,0 @@
-<?php namespace OpenId\Exceptions;
-/**
- * Copyright 2016 OpenStack Foundation
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- * http://www.apache.org/licenses/LICENSE-2.0
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- **/
-use Exception;
-/**
- * Class InvalidTLDException
- * @package OpenId\Exceptions
- */
-class InvalidTLDException extends Exception {
-
-    public function __construct($message = "")
-    {
-        $message = "Invalid TDL: " . $message;
-        parent::__construct($message, 0, null);
-    }
-}
\ No newline at end of file
diff --git a/app/libs/OpenId/Helpers/OpenIdUriHelper.php b/app/libs/OpenId/Helpers/OpenIdUriHelper.php
index 1cfc95a0..1cd4ce46 100644
--- a/app/libs/OpenId/Helpers/OpenIdUriHelper.php
+++ b/app/libs/OpenId/Helpers/OpenIdUriHelper.php
@@ -11,27 +11,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  **/
-use OpenId\Exceptions\InvalidTLDException;
-
-define('OpenIdUriHelper_TLDs',
-    '/\.(ac|ad|ae|aero|af|ag|ai|al|am|an|ao|aq|ar|arpa|as|asia' .
-    '|at|au|aw|ax|az|ba|bb|bd|be|bf|bg|bh|bi|biz|bj|bm|bn|bo|br' .
-    '|bs|bt|bv|bw|by|bz|ca|cat|cc|cd|cf|cg|ch|ci|ck|cl|cm|cn|co' .
-    '|com|coop|cr|cu|cv|cx|cy|cz|de|dj|dk|dm|do|dz|ec|edu|ee|eg' .
-    '|er|es|et|eu|fi|fj|fk|fm|fo|fr|ga|gb|gd|ge|gf|gg|gh|gi|gl' .
-    '|gm|gn|gov|gp|gq|gr|gs|gt|gu|gw|gy|hk|hm|hn|hr|ht|hu|id|ie' .
-    '|il|im|in|info|int|io|iq|ir|is|it|je|jm|jo|jobs|jp|ke|kg|kh' .
-    '|ki|km|kn|kp|kr|kw|ky|kz|la|lb|lc|li|lk|lr|ls|lt|lu|lv|ly' .
-    '|ma|mc|md|me|mg|mh|mil|mk|ml|mm|mn|mo|mobi|mp|mq|mr|ms|mt' .
-    '|mu|museum|mv|mw|mx|my|mz|na|name|nc|ne|net|nf|ng|ni|nl|no' .
-    '|np|nr|nu|nz|om|org|pa|pe|pf|pg|ph|pk|pl|pm|pn|pr|pro|ps|pt' .
-    '|pw|py|qa|re|ro|rs|ru|rw|sa|sb|sc|sd|se|sg|sh|si|sj|sk|sl' .
-    '|sm|sn|so|sr|st|su|sv|sy|sz|tc|td|tel|tf|tg|th|tj|tk|tl|tm' .
-    '|tn|to|tp|tr|travel|tt|tv|tw|tz|ua|ug|uk|us|uy|uz|va|vc|ve' .
-    '|vg|vi|vn|vu|wf|ws|xn--0zwm56d|xn--11b5bs3a9aj6g' .
-    '|xn--80akhbyknj4f|xn--9t4b11yi5a|xn--deba0ad|xn--g6w251d' .
-    '|xn--hgbk6aj7f53bba|xn--hlcj6aya9esc7a|xn--jxalpdlp' .
-    '|xn--kgbechtv|xn--zckzah|ye|yt|yu|za|zm|zw)\.?$/');
 
 define('OpenIdUriHelper_HostSegmentRe', "/^(?:[-a-zA-Z0-9!$&'\\(\\)\\*+,;=._~]|%[a-zA-Z0-9]{2})*$/");
 
@@ -239,13 +218,13 @@ final class OpenIdUriHelper
 
 	/**
 	 * @param $trust_root
-	 * @return bool|mixed
+	 * @return bool
 	 */
 	public static function isValidRealm($trust_root){
-		if (!self::_isSane($trust_root)) return false;
 
-		$trust_root_parsed = self::_parse($trust_root);
-		return $trust_root_parsed;
+		if (!self::isSaneRealm($trust_root)) return false;
+
+		return self::_parse($trust_root) != false;
 	}
 
     /**
@@ -264,8 +243,7 @@ final class OpenIdUriHelper
      */
     public static function checkRealm($trust_root, $url)
     {
-        if (!filter_var($url, FILTER_VALIDATE_URL)) return false;
-        if (!self::_isSane($trust_root)) return false;
+        if (!self::isSaneRealm($trust_root)) return false;
 
         $trust_root_parsed = self::_parse($trust_root);
         $url_parsed = self::_parse($url);
@@ -324,6 +302,11 @@ final class OpenIdUriHelper
     }
 
     /**
+     * http://openid.net/specs/openid-authentication-2_0.html#realms
+     * It is RECOMMENDED that OPs protect their users from making assertions with overly-general realms, like
+     * http://*.com/ or http://*.co.uk/. Overly general realms can be dangerous when the realm is used for identifying
+     * a particular Relying Party. Whether a realm is overly-general is at the discretion of the OP.
+     *
      * Is this trust root sane?
      *
      * A trust root is sane if it is syntactically valid and it has a
@@ -346,12 +329,12 @@ final class OpenIdUriHelper
      *
      * @static
      * @param string $trust_root The trust root to check
-     * @throws InvalidTLDException
      * @return bool $sanity Whether the trust root looks OK
      */
-    private static function _isSane($trust_root)
+    private static function isSaneRealm($trust_root)
     {
         $parts = self::_parse($trust_root);
+
         if ($parts === false) {
             return false;
         }
@@ -360,6 +343,11 @@ final class OpenIdUriHelper
         if ($parts['host'] == 'localhost') {
             return true;
         }
+        // if host its a valid ip address
+        if( filter_var($parts['host'], FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) !== FALSE ||
+            filter_var($parts['host'], FILTER_VALIDATE_IP, FILTER_FLAG_IPV4) !== FALSE){
+            return true;
+        }
 
         $host_parts = explode('.', $parts['host']);
         if ($parts['wildcard']) {
@@ -380,24 +368,17 @@ final class OpenIdUriHelper
             return false;
         }
 
-        // Get the top-level domain of the host. If it is not a valid TLD,
-        // it's not sane.
-        preg_match(OpenIdUriHelper_TLDs, $parts['host'], $matches);
-        if (!$matches) {
-            throw new InvalidTLDException(sprintf("current host %s",$parts['host']));
-        }
-        $tld = $matches[1];
-
-        if (count($host_parts) == 1) {
-            return false;
-        }
-
         if ($parts['wildcard']) {
+
+            if(count($host_parts) == 1) {
+                // like *.uk
+                return false;
+            }
             // It's a 2-letter tld with a short second to last segment
             // so there needs to be more than two segments specified
             // (e.g. *.co.uk is insane)
             $second_level = $host_parts[count($host_parts) - 2];
-            if (strlen($tld) == 2 && strlen($second_level) <= 3) {
+            if (strlen($second_level) <= 3) {
                 return count($host_parts) > 2;
             }
         }
@@ -425,9 +406,10 @@ final class OpenIdUriHelper
             return false;
         }
 
-        $required_parts = array('scheme', 'host');
-        $forbidden_parts = array('user', 'pass', 'fragment');
-        $keys = array_keys($parts);
+        $required_parts  = ['scheme', 'host'];
+        $forbidden_parts = ['user', 'pass', 'fragment'];
+        $keys            = array_keys($parts);
+
         if (array_intersect($keys, $required_parts) != $required_parts) {
             return false;
         }
@@ -440,15 +422,18 @@ final class OpenIdUriHelper
             return false;
         }
 
-        $scheme = strtolower($parts['scheme']);
-        $allowed_schemes = array('http', 'https');
+        $scheme          = strtolower($parts['scheme']);
+        $allowed_schemes = ['http', 'https'];
+
         if (!in_array($scheme, $allowed_schemes)) {
             return false;
         }
+
         $parts['scheme'] = $scheme;
 
-        $host = strtolower($parts['host']);
+        $host      = strtolower($parts['host']);
         $hostparts = explode('*', $host);
+
         switch (count($hostparts)) {
             case 1:
                 $parts['wildcard'] = false;
@@ -485,7 +470,6 @@ final class OpenIdUriHelper
             $parts['port'] = false;
         }
 
-
         $parts['unparsed'] = $trust_root;
 
         return $parts;
diff --git a/app/libs/OpenId/Requests/OpenIdAuthenticationRequest.php b/app/libs/OpenId/Requests/OpenIdAuthenticationRequest.php
index 213148ed..32d55a91 100644
--- a/app/libs/OpenId/Requests/OpenIdAuthenticationRequest.php
+++ b/app/libs/OpenId/Requests/OpenIdAuthenticationRequest.php
@@ -71,30 +71,49 @@ class OpenIdAuthenticationRequest extends OpenIdRequest
      */
     public function isValid()
     {
-        $return_to       = $this->getReturnTo();
-        $claimed_id      = $this->getClaimedId();
-        $identity        = $this->getIdentity();
-        $mode            = $this->getMode();
-        $realm           = $this->getRealm();
-        $valid_id        = $this->isValidIdentifier($claimed_id, $identity);
-        $valid_return_to = OpenIdUriHelper::checkReturnTo($return_to);
-        $valid_realm     = OpenIdUriHelper::checkRealm($realm, $return_to);
+        $return_to                     = $this->getReturnTo();
+        $claimed_id                    = $this->getClaimedId();
+        $identity                      = $this->getIdentity();
+        $mode                          = $this->getMode();
+        $realm                         = $this->getRealm();
+        $valid_id                      = $this->isValidIdentifier($claimed_id, $identity);
+        $valid_realm                   = OpenIdUriHelper::isValidRealm($realm);
+        $valid_return_to_against_realm = OpenIdUriHelper::checkRealm($realm, $return_to);
+        $valid_return_to               = OpenIdUriHelper::checkReturnTo($return_to);
 
         if (empty($return_to)) {
             throw new InvalidOpenIdMessageException('return_to is empty.');
         }
 
-        if (!$valid_return_to) {
-            throw new InvalidOpenIdMessageException(sprintf('invalid return_to %s', $return_to));
-        }
-
         if (empty($realm)) {
             throw new InvalidOpenIdMessageException('realm is empty.');
         }
 
         if (!$valid_realm) {
-            throw new InvalidOpenIdMessageException(sprintf('realm check is not valid realm %s - return_to %s.', $realm,
-                $return_to));
+            throw new InvalidOpenIdMessageException
+            (
+                sprintf
+                (
+                    'realm is not valid ( %s )',
+                    $realm
+                )
+            );
+        }
+
+        if (!$valid_return_to_against_realm) {
+            throw new InvalidOpenIdMessageException
+            (
+                sprintf
+                (
+                    'return to url check against provided realm is not valid ( realm %s - return_to %s).',
+                    $realm,
+                    $return_to
+                )
+            );
+        }
+
+        if (!$valid_return_to) {
+            throw new InvalidOpenIdMessageException(sprintf('invalid return_to url ( %s )', $return_to));
         }
 
         if (empty($claimed_id)) {
diff --git a/tests/OpenIdProtocolTest.php b/tests/OpenIdProtocolTest.php
index 9064a9b9..78170d32 100644
--- a/tests/OpenIdProtocolTest.php
+++ b/tests/OpenIdProtocolTest.php
@@ -260,6 +260,139 @@ class OpenIdProtocolTest extends OpenStackIDBaseTest
         $this->assertTrue($openid_response['is_valid'] === 'true');
     }
 
+    public function testInvalidTLD_Wilcard_COM_UK()
+    {
+        $params = array(
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_NS)        => OpenIdProtocol::OpenID2MessageType,
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_Mode)      => OpenIdProtocol::SetupMode,
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_Realm)     => "https://*.com.uk",
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_ReturnTo)  => "https://devbranch.openstack.com.uk/return_to.php",
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_Identity)  => "http://specs.openid.net/auth/2.0/identifier_select",
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_ClaimedId) => "http://specs.openid.net/auth/2.0/identifier_select",
+        );
+
+        $response = $this->action("POST", "OpenId\OpenIdProviderController@endpoint", $params);
+
+        $this->assertResponseStatus(302);
+
+        $url        = $response->getTargetUrl();
+        $parsed_url = @parse_url($url);
+        $query      = isset($parsed_url['query']) ?  $parsed_url['query']: null;
+        $this->assertTrue(!empty($query));
+        parse_str($query, $query_array);
+        $this->assertTrue(isset($query_array['openid_error']));
+        $error = $query_array['openid_error'];
+        $this->assertTrue(str_contains($error,"Invalid OpenId Message : realm is not valid" ));
+    }
+
+    public function testInvalidTLD_Wilcard_UK()
+    {
+        $params = array(
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_NS)        => OpenIdProtocol::OpenID2MessageType,
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_Mode)      => OpenIdProtocol::SetupMode,
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_Realm)     => "https://*.uk",
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_ReturnTo)  => "https://devbranch.openstack.com.uk/return_to.php",
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_Identity)  => "http://specs.openid.net/auth/2.0/identifier_select",
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_ClaimedId) => "http://specs.openid.net/auth/2.0/identifier_select",
+        );
+
+        $response = $this->action("POST", "OpenId\OpenIdProviderController@endpoint", $params);
+
+        $this->assertResponseStatus(302);
+
+        $url        = $response->getTargetUrl();
+        $parsed_url = @parse_url($url);
+        $query      = isset($parsed_url['query']) ?  $parsed_url['query']: null;
+        $this->assertTrue(!empty($query));
+        parse_str($query, $query_array);
+        $this->assertTrue(isset($query_array['openid_error']));
+        $error = $query_array['openid_error'];
+        $this->assertTrue(str_contains($error,"Invalid OpenId Message : realm is not valid" ));
+    }
+
+    public function testValidTLD_Wilcard()
+    {
+        $params = array(
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_NS)        => OpenIdProtocol::OpenID2MessageType,
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_Mode)      => OpenIdProtocol::SetupMode,
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_Realm)     => "https://*.openstack.org",
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_ReturnTo)  => "https://devbranch.openstack.org/return_to.php",
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_Identity)  => "http://specs.openid.net/auth/2.0/identifier_select",
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_ClaimedId) => "http://specs.openid.net/auth/2.0/identifier_select",
+        );
+
+        $response = $this->action("POST", "OpenId\OpenIdProviderController@endpoint", $params);
+
+        $this->assertResponseStatus(302);
+
+        $url        = $response->getTargetUrl();
+        $parsed_url = @parse_url($url);
+        $this->assertTrue(!isset($parsed_url['query']));
+        $this->assertTrue(str_contains($url, "https://local.openstackid.openstack.org/accounts/user/consent"));
+    }
+
+    public function testValidTLD_SameDomain()
+    {
+        $params = array(
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_NS)        => OpenIdProtocol::OpenID2MessageType,
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_Mode)      => OpenIdProtocol::SetupMode,
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_Realm)     => "https://devbranch.openstack.org",
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_ReturnTo)  => "https://devbranch.openstack.org/return_to.php",
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_Identity)  => "http://specs.openid.net/auth/2.0/identifier_select",
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_ClaimedId) => "http://specs.openid.net/auth/2.0/identifier_select",
+        );
+
+        $response = $this->action("POST", "OpenId\OpenIdProviderController@endpoint", $params);
+
+        $this->assertResponseStatus(302);
+
+        $url        = $response->getTargetUrl();
+        $parsed_url = @parse_url($url);
+        $this->assertTrue(!isset($parsed_url['query']));
+        $this->assertTrue(str_contains($url, "https://local.openstackid.openstack.org/accounts/user/consent"));
+    }
+
+    public function testValidTLD_SingleLevelDomain()
+    {
+        $params = array(
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_NS)        => OpenIdProtocol::OpenID2MessageType,
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_Mode)      => OpenIdProtocol::SetupMode,
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_Realm)     => "https://myrefstack",
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_ReturnTo)  => "https://myrefstack/return_to.php",
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_Identity)  => "http://specs.openid.net/auth/2.0/identifier_select",
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_ClaimedId) => "http://specs.openid.net/auth/2.0/identifier_select",
+        );
+
+        $response = $this->action("POST", "OpenId\OpenIdProviderController@endpoint", $params);
+
+        $this->assertResponseStatus(302);
+
+        $url        = $response->getTargetUrl();
+        $parsed_url = @parse_url($url);
+        $this->assertTrue(!isset($parsed_url['query']));
+        $this->assertTrue(str_contains($url, "https://local.openstackid.openstack.org/accounts/user/consent"));
+    }
+
+    public function testValidTLD_IPDomain()
+    {
+        $params = array(
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_NS)        => OpenIdProtocol::OpenID2MessageType,
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_Mode)      => OpenIdProtocol::SetupMode,
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_Realm)     => "https://192.0.0.1",
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_ReturnTo)  => "https://192.0.0.1/return_to.php",
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_Identity)  => "http://specs.openid.net/auth/2.0/identifier_select",
+            OpenIdProtocol::param(OpenIdProtocol::OpenIDProtocol_ClaimedId) => "http://specs.openid.net/auth/2.0/identifier_select",
+        );
+
+        $response = $this->action("POST", "OpenId\OpenIdProviderController@endpoint", $params);
+
+        $this->assertResponseStatus(302);
+
+        $url        = $response->getTargetUrl();
+        $parsed_url = @parse_url($url);
+        $this->assertTrue(!isset($parsed_url['query']));
+        $this->assertTrue(str_contains($url, "https://local.openstackid.openstack.org/accounts/user/consent"));
+    }
 
     public function testAuthenticationSetupModeSessionAssociationDHSha256()
     {