[BETA] Filter by country - version 1.0.8

A place for Extension Authors to post and receive feedback on Extensions still in development. No Extensions within this forum should be used within a live environment!
Get Involved
Forum rules
READ: phpBB.com Board-Wide Rules and Regulations

IMPORTANT: Extensions Development rules

IMPORTANT FOR NEEDED EVENTS!!!
If you need an event for your extension please read this for the steps to follow to request the event(s)
mastnacek
Registered User
Posts: 41
Joined: Thu Oct 25, 2018 6:43 am

Re: [RC] Filter by country - version 1.0.7

Post by mastnacek » Mon Sep 23, 2019 9:46 am

I tried the extension because I was bothered by Yandex shoes from Russia. It does not work. They have a lot of IP addresses and even whois shows that they change the country of origin. Now, for example, the country F1.

User avatar
MarkDHamill
Registered User
Posts: 3940
Joined: Fri Aug 02, 2002 12:36 am
Location: Florence, MA USA
Contact:

Re: [RC] Filter by country - version 1.0.7

Post by MarkDHamill » Mon Sep 23, 2019 12:24 pm

Did you turn on the logging feature? Blocked IPs should show up in the phpBB admin log. The bad IP may be for a VPN. You can disable VPN access in the ACP settings page for the extension.

Knowing the IP of a spammer that fails is a good test case, so it would be useful to have.
Get the latest versions of my Digests and Smartfeed extensions.
Need phpBB services or a phpBB consultant? I offer most phpBB services.

mastnacek
Registered User
Posts: 41
Joined: Thu Oct 25, 2018 6:43 am

Re: [RC] Filter by country - version 1.0.7

Post by mastnacek » Mon Sep 23, 2019 1:05 pm

Thanks for reply, now i turned on logging.

User avatar
EA117
Registered User
Posts: 1050
Joined: Wed Aug 15, 2018 3:23 am
Contact:

Re: [RC] Filter by country - version 1.0.7

Post by EA117 » Sat Oct 12, 2019 12:04 am

I mis-read code all the time, but is this line in main_listener.php actually correct:

Code: Select all

if ($allow_ip && $this->config['phpbbservices_filterbycountry_allow_out_of_country_logins'])
Based on how I see $allow_ip being set in code prior, it seems like this condition was supposed to be !$allow_ip. But given the support for inverting the logic (allow versus restrict), I'm unwilling to commit to saying I'm reading it correctly. 😜


I'm thinking of trying this as an additional way to protect just the /contactadmin extension page, and maybe the ucp.php?mode=register page if that becomes necessary, rather than the whole site. So I'm going to put an exception in similar to the "is URI ucp.php?mode=login" in order to be more selective.

Not sure if that desire will be common enough that maybe the extension could have a multi-line edit box of "URLs to protect"; which would have a similar "allow" versus "restrict" decision of whether presence in the list means they need protected, versus presence in the list means they should be exempt. Since the database query and other overhead could be avoided in exempt cases, there is probably some optimization to doing the tests early when in that mode, too.

(I should have confirmed: I'm only interested in the VPN mode. It's not that I want to limit registration or the contact form to specific countries. It's that I'm happy for you to view the site from a VPN; but if you're going to register or use the contact form, you'll need to do so without using your VPN. Even if subsequently you're allowed to login from a VPN.)

Seeing strstr() used to detect "is URI ucp.php?mode=login" does make me think "this needs to enforce a match starting specifically from the beginning of the URI" instead of "anywhere in the URI". So that someone cannot jump the restriction by creating a match for this magic substring elsewhere within the URL.

The ACP_FBC_DENY_ACCESS_VPN message is "Your access is denied because VPN only access is allowed." Seems like this intended to say "because VPN access is not allowed." Since "VPN access" in this context is "was not found in the database", it seems superfluous to try and include "country" in the message, knowing it will always be undefined.

Seeing REMOTE_ADDRESS queried from the request does bring to mind "what about the CloudFlare and other X_FORWARDED users?" Indeed, they can install other extensions which put "the real address" into REMOTE_ADDRESS, or reconfigure their proxy to be transparent if that's an option. But when the core responsibility of this extension is "action based on the origin country of the request", it does also feel appropriate that maybe the extension consider supporting reacting to the presence of header fields which reveal "the actual source address."

edit: To prevent being spoofed by a rogue HTTP client, maybe we even need to verify both. i.e. Your proxy address in REMOTE_ADDRESS needs to be permitted, in addition to your X_FORWARDED address needs to be permitted. Even though for logging and statistics purposes you would still assume the X_FORWARDED address is "the real one", if and when present.

Anyway, just different thoughts that came to mind when looking at this again. Thanks for all the great work in making this available. The automatic database download alone feels like premium functionality. 😁

User avatar
MarkDHamill
Registered User
Posts: 3940
Joined: Fri Aug 02, 2002 12:36 am
Location: Florence, MA USA
Contact:

Re: [RC] Filter by country - version 1.0.7

Post by MarkDHamill » Sat Oct 12, 2019 1:47 am

I think you caught a bug. That line of code should be:

Code: Select all

			if (!$allow_ip && $this->config['phpbbservices_filterbycountry_allow_out_of_country_logins'])
A "URLs to protect" feature is an interesting idea. Like a lot of extensions I write, I like to see how it evolves based on user input.

The VPN mode is in my mind somewhat suspect. I depend on the integrity of the Maxmind database and I make the assumption that if the IP is not in that database it should be from a VPN. Maxmind sells a list of VPN addresses, but it's a commercial product. Even if I wanted to integrate it as an optional feature, I'm not sure it would get approved.

Actually, I use stristr(), case insensitive:

Code: Select all

				if (stristr($url, "ucp.$this->phpEx?mode=login"))
I believe stristr() starts from the start of $url, but PHP doesn't say how the algorithm works.

ACP_FBC_DENY_ACCESS_VPN means that VPN only mode is enabled but the IP is assigned to a country, so it's not allowed.

Is there a better way to determine the actual IP address if REMOTE_ADDRESS is suspicious?

I appreciate the in depth look into the extension's logic.
Get the latest versions of my Digests and Smartfeed extensions.
Need phpBB services or a phpBB consultant? I offer most phpBB services.

User avatar
EA117
Registered User
Posts: 1050
Joined: Wed Aug 15, 2018 3:23 am
Contact:

Re: [RC] Filter by country - version 1.0.7

Post by EA117 » Sat Oct 12, 2019 3:42 am

MarkDHamill wrote:
Sat Oct 12, 2019 1:47 am
I believe stristr() starts from the start of $url, but PHP doesn't say how the algorithm works.
The strstr() functions find "the first instance of this substring". Which indeed "starts at the beginning", but is willing to find the substring anywhere within the string being searched. Presumably using strpos() would suit the concern; is the substring present, and where is the substring present? Yes, the case-insensitive version, stripos().

MarkDHamill wrote:
Sat Oct 12, 2019 1:47 am
ACP_FBC_DENY_ACCESS_VPN means that VPN only mode is enabled but the IP is assigned to a country, so it's not allowed.
Ah, my bad assumption. I had gone looking for whether the message would need customized in order to reflect "you are denied because you're on a VPN", and didn't look closely enough after thinking one already existed. That is one other thing I'll be adding then; a "you're denied because you're on a VPN address" message. Seems like that would be useful to report distinctly, for what it's worth. I'm concerned about it just because its the only message any visitor would ever see in my use case.


Agreed on the "loose" definition of VPN, and this can only work as well or as poor as MaxMind's database continues to fit these assumptions. That's perfectly fine, and an acceptable trade-off in exchange for "free database." It makes sense they wouldn't want to ID known VPNs as "comes from this country", since that's exactly what a VPN can help mask for the user (not actually from that country). So maybe this assumption continues working well; we'll see. We'll just count ourselves lucky for however long it lasts.

User avatar
MarkDHamill
Registered User
Posts: 3940
Joined: Fri Aug 02, 2002 12:36 am
Location: Florence, MA USA
Contact:

Re: [RC] Filter by country - version 1.0.7

Post by MarkDHamill » Sat Oct 12, 2019 12:49 pm

I don't quite see the advantage to using stripos() vs. stristr(). All I want to know is if the string is present; I don't care about its location. If present, it tells me the user is on the login page link. The return statement means access is allowed. It's allowed because even though access would normally be disallowed, an out of country login is permitted by the extension's setting. If it's not there, access is only prohibited because a trigger_error() statement is encountered.
Get the latest versions of my Digests and Smartfeed extensions.
Need phpBB services or a phpBB consultant? I offer most phpBB services.

User avatar
kasimi
Extension Customisations
Extension Customisations
Posts: 3937
Joined: Sat Sep 10, 2011 7:12 pm
Location: Germany
Contact:

Re: [RC] Filter by country - version 1.0.7

Post by kasimi » Sat Oct 12, 2019 2:19 pm

MarkDHamill wrote:
Sat Oct 12, 2019 1:47 am

Code: Select all

if (stristr($url, "ucp.$this->phpEx?mode=login"))
It should be possible to order URL parameters in any way, but this check requires the mode to be the first parameter. You should only check the URL for the script name and use the request service to check for the mode parmeter. Example:

Code: Select all

if (stripos($this->user->page['page'], 'ucp.' . $this->phpEx) === 0 && $this->request->variable('mode', '') == 'login')
About str(i)str, see the second note here: https://www.php.net/manual/en/function.strstr.php

User avatar
MarkDHamill
Registered User
Posts: 3940
Joined: Fri Aug 02, 2002 12:36 am
Location: Florence, MA USA
Contact:

Re: [RC] Filter by country - version 1.0.7

Post by MarkDHamill » Sat Oct 12, 2019 2:37 pm

Thanks for the clarification. This will show up on the next version.
Get the latest versions of my Digests and Smartfeed extensions.
Need phpBB services or a phpBB consultant? I offer most phpBB services.

User avatar
EA117
Registered User
Posts: 1050
Joined: Wed Aug 15, 2018 3:23 am
Contact:

Re: [RC] Filter by country - version 1.0.7

Post by EA117 » Sat Oct 12, 2019 9:48 pm

Thanks Kasimi. That $request has already parsed out the page at the start of the URI is indeed an even better solution. And of course you're correct about it being possible for parameters to be in an alternate order.

The strpos() functions being faster and less memory is a great additional reason. My original reason (responding back to Mark here) was actually to ensure someone wouldn't maliciously attempt to bypass the extension's blocking behavior. By using a URL that intentionally creates a match for the "ucp.php?mode=login" string strstr() was searching for, knowing the strstr() call "doesn't care where it appears."

e.g. If the user was able to send "memberlist.php?mode=team&ucp.php?mode=login", or in any other way "embed ucp.php?mode=login such that it appears elsewhere in the URL", our strstr() test was going to accept that even though ucp.php wasn't the page being accessed. Using strpos() would have allowed testing "yes, the magic string I'm looking for is found, but where was it found?"

Leveraging the parsing of the URL that $request has already performed solves all of that, in an even better way. I'm just saying what the original strpos() intention was, and the kind of malicious access it intended to prevent.

MarkDHamill wrote:
Sat Oct 12, 2019 1:47 am
Is there a better way to determine the actual IP address if REMOTE_ADDRESS is suspicious?
The REMOTE_ADDR itself won't "look suspicious" in some detectable way; it will just be the IP address of the proxy server, and likely to be the same for all requests. And it's not that we shouldn't still validate that REMOTE_ADDR represents "a country allowed to access the server." But in order to achieve the "Filter by Country" extension's true intention, we probably need to validate additional IP addresses that are present in the HTTP header in addition to REMOTE_ADDR. i.e. HTTP_CF_CONNECTING_IP (CloudFlare) and X_FORWARDED_FOR (other proxies or accelerators in front of your site).

phpBB itself currently optionally evaluates X_FORWARDED_FOR, but my understanding is that it's only for the purposes of "matching the current request to an existing session." i.e. It doesn't cause $session->ip or $user->ip to be replaced with the X_FORWARDED_FOR address, which is why phpBB users still end up with "the same IP address for everyone" in the who's online and user logs.

That's fine; you're not trying to solve that. But "Filter by Country" does intend, "as authoritatively as possible", to deny or grant access to users based on what country the IP address indicates they are coming from. And in cases where a HTTP_CF_CONNECTING_IP or X_FORWARDED_FOR header is also present, the IP address in REMOTE_ADDRESS may not be indicating "what country they are coming from."

The problem is that the HTTP header that REMOTE_ADDR gets populated from is the only "reliable" one, in that its the least likely to become spoofed. (If it does become spoofed, the malicious HTTP client that spoofed it never receives back any response; so spoofing it is not particularly useful except for things like DDoS attacks where the response isn't needed.)

On the other hand, any malicious HTTP client could add HTTP_CF_CONNECTING_IP or X_FORWARDED_FOR headers, even when REMOTE_ADDR is not the IP address of a proxy server. I expect that's why phpBB's usage of X_FORWARDED_FOR is "opt-in", so that you only "risk" matching a phpBB session using the X_FORWARDED_FOR address when the phpBB admin knows "I'm behind a proxy and the X_FORWARD_FOR will be from my proxy, not from a rogue HTTP client."

To my view though, "Filter by Country" doesn't have that problem. If a rogue HTTP client wants to throw in a false HTTP_CF_CONNECTING_IP or X_FORWARDED_FOR header, great. Let him. Because "Filter by Country" would simply validate all of the IP addresses provided, and require that they all evaluate as coming from an allowed country. You're not going to do what phpBB does and "evaluate against X_FORWARDED_FOR instead of REMOTE_ADDR"; at which point you would have allowed a rogue HTTP client to "trick" you into only checking the IP address they wanted you to check, when the HTTP response will actually go back to REMOTE_ADDR.

It's probably not any kind of quick change, and will need to wait for a little bit of code re-organization to allow easily checking multiple IP addresses in the course of a single access attempt. I do expect that the information you need is already available through $request, same as phpBB already relies on it, and same as you can see extensions like https://www.phpbb.com/customise/db/exte ... dflare_ip/ accessing in their event handlers. Others who actually live behind proxies (my site currently does not) might have additional input that needs considered, too.

User avatar
MarkDHamill
Registered User
Posts: 3940
Joined: Fri Aug 02, 2002 12:36 am
Location: Florence, MA USA
Contact:

Re: [RC] Filter by country - version 1.0.7

Post by MarkDHamill » Sun Oct 13, 2019 12:34 pm

This is all quite interesting. I hadn't thought that HTTP headers might provide multiple IP addresses.

So is the suggestion that I test for IPs attached to any HTTP_CF_CONNECTING_IP and X_FORWARDED_FOR HTTP headers and if present check these IPs too? And if present, then determine the country of origin based on the Maxmind database? I would think it would be possible that one header might assert it's coming from China and another from the United States and that could lead to an issue. Or one could be inferred to be a VPN IP because it's not in the Maxmind database. Then which rules would apply? Because in VPN-only mode it might be allowed even though the IP is not in the REMOTE_ADDR header.

I would think REMOTE_ADDR would be consider the "trusted" header to use. Do you agree?
Get the latest versions of my Digests and Smartfeed extensions.
Need phpBB services or a phpBB consultant? I offer most phpBB services.

User avatar
EA117
Registered User
Posts: 1050
Joined: Wed Aug 15, 2018 3:23 am
Contact:

Re: [RC] Filter by country - version 1.0.7

Post by EA117 » Sun Oct 13, 2019 6:31 pm

MarkDHamill wrote:
Sun Oct 13, 2019 12:34 pm
So is the suggestion that I test for IPs attached to any HTTP_CF_CONNECTING_IP and X_FORWARDED_FOR HTTP headers and if present check these IPs too?
Correct; that is my thought on it, anyway. Because to me, the purpose of the "Filter by Country" extension is uniquely able to side-step the question that others including phpBB must ask, which is "Do I actually trust the HTTP_CF_CONNECTING_IP or X_FORWARDED_FOR header when it is present?" Meaning when these headers are present, are they there because there actually is a proxy involved? Or is a rogue HTTP agent trying to trick me in some way?

"Filter by Country" doesn't need to care about that, if it simply checks all of the IP addresses. If someone adds a HTTP_CF_CONNECTING_IP or X_FORWARDED_FOR header, it's just "yet another IP address that will be validated, and must also be from a country allowed to access this board." Presence of these headers won't be evaluated "instead of REMOTE_ADDR" as in the other cases where trust is important; these headers will simply be evaluated "in addition to REMOTE_ADDR" in the "Filter by Country" use case.

A rouge HTTP agent would therefore only be able to "shoot itself in the foot" by adding a deceptive HTTP_CF_CONNECTING_IP or X_FORWARDED_FOR header, because all they've done is make it so now there are two or more IPs which must pass the "Filter by Country" test instead of just one IP address.

MarkDHamill wrote:
Sun Oct 13, 2019 12:34 pm
I would think it would be possible that one header might assert it's coming from China and another from the United States and that could lead to an issue.
Correct; that's exactly the case we're saying "Filter by Country" would be intending to detect and reject.

e.g. If "REMOTE_ADDR = VPN or Restricted Country" (which is the IP address of the user who actually wants a response, since REMOTE_ADDR is the address the HTTP response is actually going to be sent to), but "HTTP_CF_CONNECTING_IP = USA IP address" (because rogue HTTP agent wants to trick us into evaluating and thinking he's actually in the USA), "Filter by Country" is expected to reject this case. All IP addresses present must successfully pass whatever the current "Filter by Country" restrictions are.

There is no "conundrum" when one IP address passes and another fails; that simply means we failed. It doesn't matter whether "REMOTE_ADDR passes but X_FORWARDED_FOR failed" versus "X_FORWARDED_FOR passes but REMOTE_ADDR failed". In either case, either the user was pretending to be from a blocked country, or was in fact actually from a blocked country; and so "Filter by Country" should block the request either way.

We're saying recognition of these headers is important for the non-rouge-HTTP agent case, too.

e.g. If someone is legitimately using Cloudflare, and Cloudflare has added the HTTP_CF_CONNECTING_IP header to reflect "whom has actually made a request to Cloudflare cache server", evaluating the REMOTE_ADDR in that case is going to be evaluating the IP address of Cloudflare's cache server. Which is probably just one of several addresses from a server farm, somewhere in a country that may have no relation to where the user is actually coming from.

The IP address in REMOTE_ADDR should still be evaluated, because someone can still talk to the phpBB server directly instead of through Cloudflare. But in order to actually achieve the goal of "block access from computers in other countries", the HTTP_CF_CONNECTING_IP header would have to be evaluated too, when present. Because that's the only IP address in this equation which represents "from what country did the request actually come from."

So yes, "the country your Cloudflare or legitimate proxy servers are located in" would need to be permitted in "Filter by Country", same as any other IP address that wants to talk directly to the phpBB server instead of going through the proxy. But when a proxy is involved, in addition to "evaluate the proxy server's address", you want to also evaluate "what country did the actual original request come from", too. Since that's the purpose for which "Filter by Country" was installed.

MarkDHamill wrote:
Sun Oct 13, 2019 12:34 pm
I would think REMOTE_ADDR would be consider the "trusted" header to use. Do you agree?
Yes, it absolutely is "the most trusted of the IP address headers" being discussed here. For the "Filter by Country" extension's purposes, the only issue with REMOTE_ADDR is that "this IP address may have nothing to do with the country the request actually originated from."

The suggestion is not to "stop" evaluating REMOTE_ADDR, nor to even "conditionally" evaluate REMOTE_ADDR; this can continue always being evaluated, regardless. The suggestion is to evaluate additional headers too, if and when they are present, in order to even more definitively achieve the extension's high-level goal.

The extension is very useful in it's current state of evaluating only REMOTE_ADDR, since that covers a great many phpBB use cases. Obviously its your call whether to adopt any plan to include additional address evaluation or not. Or to maybe wait until there is a customer saying they're literally hitting this case and need a solution, so that you can develop and verify against an actual customer. (I am not that customer.) All of this discussion came up for me simply when looking at the code, and seeing REMOTE_ADDR was being used, and thinking of the implications.

User avatar
MarkDHamill
Registered User
Posts: 3940
Joined: Fri Aug 02, 2002 12:36 am
Location: Florence, MA USA
Contact:

Re: [RC] Filter by country - version 1.0.7

Post by MarkDHamill » Sun Oct 13, 2019 7:26 pm

I thank you and others for your thoughts on these matters. Looks like I need to work on another version of the extension incorporating these suggestions. I'm always amazed by how learned my peers around here are, generally more so than me.
Get the latest versions of my Digests and Smartfeed extensions.
Need phpBB services or a phpBB consultant? I offer most phpBB services.

User avatar
warmweer
Registered User
Posts: 2983
Joined: Fri Jul 04, 2003 6:34 am
Location: Van Allen Belt ... well actually Belgium

Re: [RC] Filter by country - version 1.0.7

Post by warmweer » Sun Oct 13, 2019 7:40 pm

MarkDHamill wrote:
Sun Oct 13, 2019 7:26 pm
I'm always amazed by how learned my peers around here are, generally more so than me.
Even though I've been using phpBB for more than 15 years, I still get that feeling every day :o
The year is 2192. The British Prime Minister visits Brussels to ask for an extension of the Brexit deadline. No one remembers where this tradition originated, but every year it attracts many tourists from all over the world.

User avatar
MarkDHamill
Registered User
Posts: 3940
Joined: Fri Aug 02, 2002 12:36 am
Location: Florence, MA USA
Contact:

Re: [RC] Filter by country - version 1.0.7

Post by MarkDHamill » Sun Oct 13, 2019 8:55 pm

You are a newbie. I started in 2002.
Get the latest versions of my Digests and Smartfeed extensions.
Need phpBB services or a phpBB consultant? I offer most phpBB services.

Post Reply

Return to “Extensions in Development”