search

yiisoft / yii2

Yii 2: The Fast, Secure and Professional PHP Framework
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
14.23k stars 6.91k forks source link

Trusted host config: alternative matching methods #14691

Closed cebe closed 7 years ago

cebe commented 7 years ago

@SamMousa implemented configuration for trusted hosts in #13780. The matching is currently done by regex, which caused some discussion in https://github.com/yiisoft/yii2/pull/13780#discussion_r106126378 and https://github.com/yiisoft/yii2/pull/13780#discussion_r134188182.

Current implementation for Request::$trustedHosts

[
    '/\.trusted\.com$/', // match a host ending in .trusted.com
    '^192\.168\.0\.\d+$', // match ip in the range `192.168.0.0/24`
]

Alternatives / Problems

Alternative to the above using fnmatch would look like this:

[
    '*.trusted.com', // match a host ending in .trusted.com
    '192.168.0.*', // match ip in the range `192.168.0.0/24`
]

Allowing CIDR would allow this:

[
    '*.trusted.com', // match a host ending in .trusted.com
    '192.168.0.0/24', // match ip in the range `192.168.0.0/24`
]

Decision should be made before 2.0.13 as it would probably break BC.

samdark commented 7 years ago

I don't think it's BC breaking. The feature wasn't released yet.

cebe commented 7 years ago

seems we need to figure out what we mean when applying the label :-D I attached it because the change would be BC breaking if not made before the release ;)

rob006 commented 7 years ago

https://github.com/symfony/http-foundation/blob/master/IpUtils.php

cebe commented 7 years ago

https://github.com/yiisoft/yii2/blob/9e713dba29246babefaa40ef45c4792d2d620158/framework/validators/IpValidator.php#L530-L558

rob006 commented 7 years ago

I'm for using fnmatch() here for consistency. The best option would be use own helper which accepts both fnmatch() and CIDR patterns, but it could be added after 2.0.13 release.

We should probably not even allow matching host names here as it is easy to fake a reverse lookup to point to a host.

That is good point, we should allow only IPs here.

SilverFire commented 7 years ago

We should probably not even allow matching host names here as it is easy to fake a reverse lookup to point to a host.

Very good point. Voting strictly for it.

I'm for CIDR notation and using IpValidator.

SilverFire commented 7 years ago

Actually, there is a totally safe case with host names: application works behind a proxy server and all requests to the application are received ONLY from the local network. However, it's a bit advanced topic and I would not like to recommend to rely on it.

SamMousa commented 7 years ago

Btw, don't SMTP servers often use something like reverse DNS + DNS to validate hostnames? So I can spoof the reverse DNS, but not the normal one:

1.2.3.4 --reverse-dns--> very.secure.server
very.secure.server --forward-dns--> 192.168.0.1

In scenario's where my Yii2 application runs inside some containerized environment, chances are:

  1. I do not know the IP of the web server.
  2. Trusting the whole private network is not restrictive enough
  3. I can trust the forward-dns.

In that case host name matching would be very valuable; it would just require an extra forward lookup.. http://en.wikipedia.org/wiki/Forward_Confirmed_reverse_DNS

cebe commented 7 years ago

Btw, don't SMTP servers often use something like reverse DNS + DNS to validate hostnames?

SMTP servers use reverse lookups to check whether the reverse matches back to the original IP or whether someone wants to fake here. They are not taking the reverse lookup only but verify the IP it points to. Basically what you linked.

Adding the overhead of that kind of validation is kind of overkill here.

Trusting the whole private network is not restrictive enough

in which case would you not trust your own network?