-
-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implementing trusted_proxies CIDR notation capability for IPv6 #12535
Changes from all commits
4dd4379
5efc12f
54c3694
9c98e43
fc7b744
9674d3b
7b9f5bc
587025a
8d35b0e
5fa3396
af610cd
a5503ad
cf554e0
0920691
b26e862
b4a5d8f
02c6ed1
ad76224
361871d
efc02c6
29488ec
caf3df2
4ad17fc
9b66b4f
36aec3f
811befa
f82f4fb
733b416
638d876
bc413ea
a8f7ea4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,149 @@ | ||
<?php | ||
declare(strict_types=1); | ||
/** | ||
* @copyright Copyright (c) 2018, Oliver Wegner (void1976@gmail.com) | ||
* | ||
* @author Oliver Wegner <void1976@gmail.com> | ||
* | ||
* @license GNU AGPL version 3 or any later version | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU Affero General Public License as | ||
* published by the Free Software Foundation, either version 3 of the | ||
* License, or (at your option) any later version. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU Affero General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU Affero General Public License | ||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
* | ||
*/ | ||
|
||
namespace OC\Net; | ||
|
||
use OCP\Net\IIpAddress; | ||
|
||
abstract class AbstractIpAddress implements IIpAddress { | ||
abstract public function getMaxBitlength(): int; | ||
abstract protected function getCidrRegex(): string; | ||
abstract protected function matchCidr(IIpAddress $other): bool; | ||
|
||
protected $original = ''; | ||
protected $netPart = ''; | ||
protected $netmaskBits = 0; | ||
|
||
/** | ||
* Constructor that takes an IP address in string form and | ||
* initializes this instance to represent that address | ||
* | ||
* @param string $address | ||
*/ | ||
public function __construct(string $address) { | ||
$this->setOriginal($address); | ||
|
||
if (preg_match($this->getCidrRegex(), $address, $match)) { | ||
$this->setNetPart($match[1]); | ||
$this->setNetmaskBits(max(0, min($this->getMaxBitlength(), intval($match[2])))); | ||
} else { | ||
$this->setNetPart($address); | ||
$this->setNetmaskbits($this->getMaxBitlength()); | ||
} | ||
} | ||
|
||
/** | ||
* Sets the literal address string that this instance | ||
* represents | ||
* | ||
* @param string $original | ||
*/ | ||
protected function setOriginal(string $original) { | ||
$this->original = $original; | ||
} | ||
|
||
/** | ||
* Returns the literal address string that this instance | ||
* represents | ||
* | ||
* @return string | ||
*/ | ||
protected function getOriginal(): string { | ||
return $this->original; | ||
} | ||
|
||
/** | ||
* Sets the network part of the | ||
* address/range represented by this instance | ||
* | ||
* @param string $netPart | ||
*/ | ||
protected function setNetPart(string $netPart) { | ||
$this->netPart = $netPart; | ||
} | ||
|
||
/** | ||
* Returns the network part of the | ||
* address/range represented by this instance | ||
* | ||
* @return string | ||
*/ | ||
protected function getNetPart(): string { | ||
return $this->netPart; | ||
} | ||
|
||
/** | ||
* Sets the number of bits of the net part of the IP | ||
* address/range represented by this instance | ||
* | ||
* @param int $bits | ||
*/ | ||
protected function setNetmaskBits(int $bits) { | ||
$this->netmaskBits = $bits; | ||
} | ||
|
||
/** | ||
* Returns the number of bits of the net part of the IP | ||
* address/range represented by this instance | ||
* | ||
* @return int | ||
*/ | ||
protected function getNetmaskBits(): int { | ||
return $this->netmaskBits; | ||
} | ||
|
||
/** | ||
* Returns whether $other is literally equivalent to this instance | ||
* | ||
* @return bool | ||
*/ | ||
protected function matchOriginal(IIpAddress $other): bool { | ||
return $other->getOriginal() === $this->getOriginal(); | ||
} | ||
|
||
/** | ||
* Returns whether this instance represents an IP range (vs. | ||
* a single IP address) | ||
* | ||
* @return bool | ||
*/ | ||
public function isRange(): bool { | ||
return $this->getNetmaskBits() < $this->getMaxBitlength(); | ||
} | ||
|
||
/** | ||
* Returns whether given $other address is either | ||
* - equal to this instance regarding its IP address or | ||
* - is contained in the IP address range represented by this instance | ||
* | ||
* @param IIpAddress $other | ||
* @return bool | ||
*/ | ||
public function containsAddress(IIpAddress $other): bool { | ||
return $this->isRange() | ||
? $this->matchCidr($other) | ||
: $this->matchOriginal($other); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
<?php | ||
declare(strict_types=1); | ||
/** | ||
* @copyright Copyright (c) 2018, Oliver Wegner (void1976@gmail.com) | ||
* | ||
* @author Oliver Wegner <void1976@gmail.com> | ||
* | ||
* @license GNU AGPL version 3 or any later version | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU Affero General Public License as | ||
* published by the Free Software Foundation, either version 3 of the | ||
* License, or (at your option) any later version. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU Affero General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU Affero General Public License | ||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
* | ||
*/ | ||
|
||
namespace OC\Net; | ||
|
||
use OCP\Net\IIpAddressFactory; | ||
use OCP\Net\IIpAddress; | ||
use OC\Net\IpAddressV4; | ||
use OC\Net\IpAddressV6; | ||
|
||
/** | ||
* This factory creates instances of IIpAddress, given an IP address | ||
* string (e.g. "192.168.1.2" or subnet string in CIDR format (e.g. | ||
* "192.168.1.0/24"). | ||
*/ | ||
class IpAddressFactory implements IIpAddressFactory { | ||
/** | ||
* Returns whether $address represents an IPv6 address | ||
* | ||
* @param string $address | ||
* @return bool | ||
*/ | ||
protected static function isIpv6(string $address): bool { | ||
return strpos($address, ':') !== false; | ||
} | ||
|
||
/** | ||
* Creates a new instance conforming to IIpAddress and | ||
* representing the given $address. | ||
* | ||
* @param string $address | ||
* @return IIpAddress | ||
*/ | ||
public function getInstance(string $address): IIpAddress { | ||
if (self::isIpv6($address)) { | ||
return new IpAddressV6($address); | ||
} else { | ||
return new IpAddressV4($address); | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
<?php | ||
declare(strict_types=1); | ||
/** | ||
* @copyright Copyright (c) 2018, Oliver Wegner (void1976@gmail.com) | ||
* | ||
* @author Oliver Wegner <void1976@gmail.com> | ||
* | ||
* @license GNU AGPL version 3 or any later version | ||
* | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU Affero General Public License as | ||
* published by the Free Software Foundation, either version 3 of the | ||
* License, or (at your option) any later version. | ||
* | ||
* This program is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU Affero General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU Affero General Public License | ||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
* | ||
*/ | ||
|
||
namespace OC\Net; | ||
|
||
use OCP\Net\IIpAddress; | ||
use OC\Net\AbstractIpAddress; | ||
|
||
class IpAddressV4 extends AbstractIpAddress { | ||
/** | ||
* Returns the length of the represented IP address format in bits. | ||
* | ||
* @return int | ||
*/ | ||
public function getMaxBitlength(): int { | ||
return 32; | ||
} | ||
|
||
/** | ||
* Returns the regular expression for recognizing CIDR notation. | ||
* | ||
* @return string | ||
*/ | ||
protected function getCidrRegex(): string { | ||
return '/^([0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3})\/([0-9]{1,2})$/'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This regex matches invalid IPv4 subnets: https://regex101.com/r/tjtE95/1 (same for IPv6 in the As IP address are numbers, I think you could use numerical comparison and avoid relying on regexes (that will most certainly be incorrect in a way or another). I'm not really into PHP so I'm not sure what's the best way to do that (to be honest it bugs me that this is not part of the standard library). Also, I think it would be useful to warn the administrator when they put a wrong value in the configuration (thus using another "parsing" method). |
||
} | ||
|
||
/** | ||
* Returns whether given $other address is either | ||
* - equal to this instance regarding its IP address or | ||
* - is contained in the IP address range represented by this instance | ||
* | ||
* @param IIpAddress $other | ||
* @return bool | ||
*/ | ||
protected function matchCidr(IIpAddress $other): bool { | ||
$shiftbits = $this->getMaxBitlength() - $this->getNetmaskBits(); | ||
$thisnum = ip2long($this->getNetPart()) >> $shiftbits; | ||
$othernum = ip2long($other->getNetPart()) >> $shiftbits; | ||
|
||
return $othernum === $thisnum; | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any particular reason why this is nullable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not from an application logic point of view and I really don't like it this way. However, making it a mandatory argument will force me to touch quite a few lines of code elsewhere (most in tests though).
I had initially been happy that there were only two locations in the code that really instantiate
Request
, but I had not found all of them in production code (e.g. inbase.php
) and had also not taken the tests into account.It'll take some time for me to put that additional effort into this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately this is the correct way to do this. Otherwise this is bound to fail someone in the future that assumes the argument is optional.