getgrav / grav

Modern, Crazy Fast, Ridiculously Easy and Amazingly Powerful Flat-File CMS powered by PHP, Markdown, Twig, and Symfony
https://getgrav.org
MIT License
14.54k stars 1.41k forks source link

Multiple security.yaml files #3432

Closed pamtbaau closed 3 years ago

pamtbaau commented 3 years ago

Is the above by desing? If so, why?

If all three are required, why not using /user/env/cli/config/security.yaml and /user/env/localhost/config/security.yaml?

mahagr commented 3 years ago

The above doesn't sound correct. CLI shouldn't have its own salt and the admin should create the main salt...

But generally, every host/site should have its own salt as it is used for example for the form nonces and caching the content.

beamaria commented 3 years ago

I have a similar problem: my website is in a subfolder named "test" of the "mysite.com" main site (that is to say: https://www.mysite.com/test) Every time I log in the test site, a new folder called "mysite.com" is created in test/user and it contains a folder named "config" with a security.yaml file inside. I'm sure it didn't happen previously ( I always use the path above as a test environment)

TheoAcker12 commented 3 years ago

It's also saving plugin config files in this new folder.

pamtbaau commented 3 years ago

@TheoAcker12,

TheoAcker12 commented 3 years ago

@pamtbaau

It would be Grav/my site. I'm not sure what's confusing about plugin config files. The new folder is the /user/"mysite.com"/config/ folder.

In other words, when modifying plugin settings and clicking save (using the admin panel), /user/mysite.com/config/ creates a new plugins folder (if one doesn't already exist) and saves the plugin-name.yaml file there, instead of in /user/config/plugins/plugin-name.yaml.

pamtbaau commented 3 years ago

@beamaria , @TheoAcker12

Although the behaviour of the creation of folder /user/<domain>/config/ is new, it is a common configuration strategy of Grav to allow different configurations depending on the domain, eg. user/localhost for dev configs and user/mydomain.com for production configs. Think of different caching configuration, debugging, etc. See Environment Configuration

The folder structure /user/<domain>/config is the Grav 1,6 way (see here) of separating configurations. The 1.7 way (see here) would be /user/env/<domain>/config

As @mahagr explained (which was new to me) every domain/site should have its own salt. To achieve this, different config folders are required. Hence the new folder(s) being created.

So, the main issues here is the creation of the /user/cli folder and the fact that the 1.6 structure of Environment Configuration is being used.

beamaria commented 3 years ago

I wonder if this could be a problem when moving the site from a subfolder of "mysite.com http://mysite.com/" to its definitive domain (as I've said, I use this environment for testing)...

Il giorno 2 set 2021, alle ore 17:50, pamtbaau @.***> ha scritto:

@beamaria https://github.com/beamaria , @TheoAcker12 https://github.com/TheoAcker12 Although the behaviour of the creation of the /user//config/ is new, it is a common configuration strategy of Grav to allow different configurations depending on the domain, eg. user/localhost for dev configs and user/mydomain.com for production configs. Think of different caching configuration, debugging, etc. See Environment Configuration https://learn.getgrav.org/17/advanced/environment-config The folder structure /user//config* is the Grav 1,6 way (see here https://learn.getgrav.org/16/advanced/environment-config#automatic-environment-configuration) of separating configurations. The 1.7 way (see here https://learn.getgrav.org/17/advanced/environment-config#automatic-environment-configuration) would be /user/env//config

As @mahagr https://github.com/mahagr explained (which was new to me) every domain/site should have its own salt. To achieve this, different config folders are required. Hence the new folder(s) being created.

So, the main issues here is the creation of the /user/cli folder and the fact that the 1.6 structure of Environment Configuration is being used.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/getgrav/grav/issues/3432#issuecomment-911875788, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACZG6TVP2F2DXD5KS7EK5ATT76TONANCNFSM5DH7PCTQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

vitaminace33 commented 3 years ago

I confer. My sites (envs) folder name in setup.php is

$folder = "sites/{$name}"

and I have

/user/cli/config/system.yaml
/user/domain.com/config/system.yaml
/user/domain.org/config/system.yaml
/user/www.domain.com/config/system.yaml
/user/www.domain.org/config/system.yaml
fseesink commented 3 years ago

Whatever this is, it came with Grav 1.7.20, and it's definitely wrong. I just logged into my host, and found a pile of directories under ./user/ that weren't there before:

./user/<ip_of_server>
./user/<some_other_ip_I_dont_recognize>
./user/subdomain.mydomain.com
./user/<subdomain2>.mydomain.com
./user/<subdomain3>.mydomain.com
...
./user/<another_domain_I_own_that_points_to_this_server>
./user<nonexistent_subdomain>.<another_domain_I_own_that_points_to_this_server>
...

Mind you, my setup is an Ubuntu 20.04 LTS cloud server running NGINX and I have it configured to only serve up "subdomain.mydomain" as it were. I installed Grav in

/var/www/<mydomain>/grav

and that is what NGINX points the root at.

And while I MIGHT understand one or two of these domain directories (at least as far as that they exist, not that Grav went and added these subdirectories), along with IPs and domains that have NOTHING to do with my site, there are directories showing domains I own with subdomains that simply do not exist!!

All are timestamped beginning from when I upgraded Grav 1.7.18 to 1.7.20 on 2 Sep. through 4 Sep., with some FQDNs being ones I do not even recognize (e.g., seda9.bet) while others are not for this site but get redirected here (several I did not even realize were doing that).

Honestly, this is just nasty whatever this is. Not sure what you all were aiming for with this, but it's just poor form. If I had to guess, anyone who sets up an FQDN even with a forward to land on your Grav site is going to pollute your directory structure, as I have 2 directories with IP addresses (one which is accurate for the hosting system the site is on, the other absolutely not). All I can guess is that if bots/spiders/etc. are probing for FQDNs (whether you created them or not) and they land on your site, Grav is generating these directories. Why is anyone's guess, but it is, frankly, a horrible idea and just screams for abuse.

NOTE: I am going to delete all of these and see whether they regenerate over the next few days. But honestly... just no.

fseesink commented 3 years ago

I just tested, and sure enough, as soon as I logged into my admin page, it regenerated the directory ./user/mysubdomain.mydomain.com. I suspect over time I'll find my ./grav/user directory just littered with subdirectories once again. Honestly, who thought this was a good design?

To be clear, I have never run anything less than Grav 1.7. That's what I started with. I've updated over time using the Grav Admin panel, and to date things have worked quite well. But this... is just horrible frankly.

Oh, and I have no ./user/cli directory nor a ./user/env directory as some others have indicated. Not sure what that's about, but guessing if you use the gpm tool or something else?

Anyway, this feature really muddies up the directory structure something fierce. If you need more specifics, let me know. But truth is, the fact I found directories with names implying subdomains like support.mydomain.com or stage.mydomain.com that simply do not even exist, not to mention domains like seda9.bet which appears to be in... Korean?... it begs the question how Grav even got the idea to generate those directory names.

vitaminace33 commented 3 years ago

Grav uses user/env to store what they call environments, alternate configurations of the site which can serve, for instance, to separate site production from site development. This system is also used for multisite, reason why I changed it to user/sites instead.

What you comment about external sites is indeed worrying. Let's hope developers have a quick look into this.

pamtbaau commented 3 years ago

@fseesink, I can (partly) confirm the behaviour you're seeing. For each subdomain.domain with which the site can be reached, a new /user/subdomain.domain/config/security.yaml (or /user/env/subdomain.domain/config/security.yaml) is being created.

http://localhost/grav/site-dev        -> /user/localhost/config/security.yaml
http://xxx.localhost/grav/site-dev    -> /user/xxx.localhost/config/security.yaml
http://yyy.localhost/grav/site-dev    -> /user/yyy.localhost/config/security.yaml

However, this behaviour only happens on my local dev machine (Windows 10/WSL/Ubuntu-20.04/Apache.

On my production server, I cannot access sites using non-existing subdomains. Nor can I access xxx.google.com, xxx.grav.org, xxx.mozilla.org, xxx.netflix.com, xxx.facebook.com, etc. Error DNS_PROBE_FINISHED_NXDOMAIN is being thrown.

It makes me wonder how your sites can be accessed using non-existing subdomains and even unknown IP addresses.

Also note, when folder /user/env exists, all <domain>/config/security.yaml files are being created in folder /user/env.

As a side note, I experience the tone of voice in your posts as a bit, uhm, "unpleasant". But that might just be me, not being a native English speaker.

fseesink commented 3 years ago

@vitaminace33 Regarding ./user/env, is that something dating back to a version of Grav 1.6 or earlier? And/or is that a directory that should have been created/generated by Grav during installation? I only ask as I only started using Grav with v1.7.x (sorry, can't remember the 3rd number, but it earlier this year... circa 1.7.10 maybe?). So I have never been using Grav 1.6. And before installation, I read through the docs, and I don't recall having to manually add any directories. I simply downloaded/decompressed the Grav core + Admin plugin .ZIP file to my host, then ran through the setup in the Web UI. I think the most complicated part was making sure I had the NGINX config right. But it went smoothly, and until now, other than a few minor bugs here and there with certain updates, it's been a great experience. And going back through the docs, it's not clear to me whether I, as someone just setting up a simple blog site, needed to have that directory or not.

So to the Grav folks, if there is supposed to be a ./user/env directory in everyone's Grav install, maybe make that part of the initial installation? Or have the code that is creating all these FQDN directories automatically create the env subdirectory before adding these into it?

@pamtbaau Though not my intention to sound "unpleasant", I could see how it came across that way. Yes, I was surprised (and not in a good way) to see my ./user/ directory polluted with all kinds of new directories. Any time you see files being generated on your system that you don't expect, especially from a public-facing, web-based service, it tends to make one nervous. My mind immediately raced towards "How could someone leverage this?" such as setting up an FQDN that matched an expected directory under ./user and wreaking havoc on a Grav install, things like that. (To date I haven't figured out anything beyond simply littering someone's ./user/ directory with a ton of useless names.)

So yes, I wasn't thrilled. But you're right. It could have sounded better. So apologies to anyone if I came across too harshly. My main concern is to see this fixed ASAP, as this could require a bit of cleanup for folks the longer this lasts. If nothing else, it would be nice to disable this.

Now, regarding where all the directory names came from, I believe I at least have a partial answer. I've had this cloud server for a bit. It's a pretty stock setup of Ubuntu 20.04 LTS with NGINX using /etc/nginx/nginx.conf as the main config file, which in turn loads the virtual hosts defined in /etc/nginx/sites-available that are symlinked in /etc/nginx/sites-enabled.

Now I had configured my subdomain.domain site that is using Grav as the default_server, meaning it was the catch-all for the various domains I have that are all pointed at the IP of this server. I removed that, but it still kept serving up suddomain.domain when I used those other FQDNs. Digging further, I found that the basic nginx.conf simply includes the /etc/nginx/sites-enabled *.conf files. But as it does not define a default server, NGINX falls back to using using whichever server config it finds first. Yep, that was the subdomain.domain server config.🤦‍♂️

So in the end I tweaked the main nginx.conf to have a basic

http {
   ...
   server {
      return 444;
   }
}

before including those .conf files. So now for any FQDNs not specifically handled by those .conf files, the server simply does not respond.

Best I can guess on the IP address directory I did not recognize as well as that seda9.bet one is that due to my config, if anyone out there was using tools that somehow created FQDNS which pointed at my server's IP, that would create this mess. Now why they're doing that is anyone's guess, but I'm sure it wasn't for any good reason.

Hopefully this clamps things down to just the FQDNs I actually have/care about. But yes, I could see others being in a similar situation using stock setups.

fseesink commented 3 years ago

Quick note: I manually created the ./user/env directory, set its permissions to match all the others, cleared out all the FQDN directories from ./user/, then went to my site. And sure enough, it is now generating the FQDN directories under ./user/env. So at least now it's all in one place and easier to deal with. I'll see what all gets generated in there in the next few days, though hopefully I have this now sorted.

vitaminace33 commented 3 years ago

I also created user://env and things are under control (my sites are under user://sites.

fseesink commented 3 years ago

Wow. Even though I have things configured and working so that only subdomain.domain loads the Grav site, and that the other domains (also mapped to same IP) are sent a 444 code (so none return a proper webpage), I'm still seeing a directory being created for each of them. What the ?? The IP of the server is back, as is localhost now, along with the other domains in various forms. Ugh. At least it's now all in one folder.

fseesink commented 3 years ago

Bloody heck. Well this feature sure has made me go look at my whole setup.

Ok, I figured out how the nonexistent subdomains were creeping in. I started by tinkering, eventually noticing that if I did a host/nslookup/dig of any random subdomain, it would still resolve to the IP of my server! Turns out my domain registrar had wildcard (*) entries for my domains.🤦‍♂️ I've gone through and gutted those wildcard entries. So now only specifically defined subdomains will resolve. That should cut the legs out from those.

fseesink commented 3 years ago

Just FYI: For anyone running a Grav site where their webserver configuration responds to HTTP/S requests to the server's IP (which is a rather common configuration), be advised that polluting your Grav directory structure is a trivial task. I have reported this to the devs with an example.

beamaria commented 3 years ago

I can confirm that this behaviour is both in main websites (official FQDN) and in their subfolders. I have a website that is virtually ready to go online, but I'll wait till this issue is solved...Don't want an official website to be hacked or have security issues! It's a bit upsetting that after all these reports of a really big bug no one among the developers has given a reply.... By the way I've noticed that this issue is only in the sites updated to Grav 1.7.20. In 1.7.18 there are no problems at all

mahagr commented 3 years ago

This is the fix that triggers the files to be added: ab9783102e0a49d54ae7e1a43902a6d38b978f6c

It cannot be changed as it fixes another bad bug where environments without configuration files were ignored.

So I ended up using a bit different approach to fix this bug.

beamaria commented 3 years ago

Hi Matias, thanks for the explanation. Is this fixing already in 1.7.20 or we have to wait till the next version?

pamtbaau commented 3 years ago

@mahagr, Not sure if the fix fixes the issue...

Steps for installing Grav in folder /www/grav/grav-admin:

The following file is being created in the root of Grav:

/www/grav/grav-admin                            <-- root of installation
├──                                             <-- folder with space as name
│   └── www
│       └── grav
│           └── grav-admin
│               └── user
│                   └── config
│                       └── security.yaml
├── CHANGELOG.md
├── CODE_OF_CONDUCT.md
├── CONTRIBUTING.md
├── LICENSE.txt
├── README.md
├── SECURITY.md
├── assets
├── backup
├── bin

Also, you explained that each domain/site should have its own security.yaml file, however, only a single file is being created now.

mahagr commented 3 years ago

Looks like I had a typo. For some unknown reason it was working properly in my case.

pamtbaau commented 3 years ago

Issue solved.

But what about your initial remark that every site/domain should have its own salt? Now, there is only one salt inside /user/config/security.yaml no matter the domain I've used to access the site.

mahagr commented 3 years ago

If you create folder user/env/domain.com/config, it will automatically create the salt for you. If the folder doesn't exist, the more generic salt is used.

So I'm assuming that if you have a different site, you also have something custom in its configuration.

pamtbaau commented 3 years ago

Tested it and you are correct. File user/env/domain/config/security.yaml will be created if and only if folder user/env/domain/config exists.

mahagr commented 3 years ago

Sounds like it's fixed then!

beamaria commented 3 years ago

Hi Matias, I have a site developed in localhost with a folder /user/localhost/config/plugins folder and security.yaml file Is it ok if I cancel the folder localhost and then upload the zipped files of the site to the official domain? I made the local backup zipped site using the BU tool in admin Panel

Il giorno 14 set 2021, alle ore 11:42, Matias Griese @.***> ha scritto:

Sounds like it's fixed then!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/getgrav/grav/issues/3432#issuecomment-919032585, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACZG6TRYBQD2W3JSCKM5XFDUB4RJBANCNFSM5DH7PCTQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

mahagr commented 3 years ago

We will be releasing likely today, so no need to do that.

fseesink commented 3 years ago

To @mahagr and everyone who worked on this, thank you.

I have just updated to Grav 1.7.21 (along with patching various plugins) and can confirm the issue we noted earlier has been handled. I adjusted my webserver config to accept connections by IP (as I had that disabled), then tried brute forcing as I'd done before. But no new directories/etc.

However, if I did as mentioned and created a ./user/env/<domain>/config subdirectory structure, then visited that <domain>, Grav generated a security.yaml file with a unique salt there. Nice.

Again, appreciate the quick response, and all the work done on Grav. I really like using it.

beamaria commented 3 years ago

Thanks to Matias and to the Grav team!

fseesink commented 3 years ago

!@#$%^&*

I spoke too soon. YES, the issue of potential randomly generated directories appears fixed, but in the process something else has now been horribly borked. I can't seem to save ANY pages anymore since upgrading to Grav 1.7.21. sigh This is frustrating.

pamtbaau commented 3 years ago

I cannot reproduce the behaviour you are experiencing....

I'm using a fresh install of Grav 1.7.21 + Admin and pages Home and Typography can be changed and saved correctly... Also new pages are being saved successfully.

fseesink commented 3 years ago

Hey @pamtbaau . Yeah, that's been covered in issue #3442 .