Skip to content
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

[Bug]: 'Data directory protected' check failing #45087

Open
5 of 8 tasks
nmbgeek opened this issue Apr 29, 2024 · 22 comments
Open
5 of 8 tasks

[Bug]: 'Data directory protected' check failing #45087

nmbgeek opened this issue Apr 29, 2024 · 22 comments
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 29-feedback bug feature: settings

Comments

@nmbgeek
Copy link

nmbgeek commented Apr 29, 2024

⚠️ This issue respects the following points: ⚠️

Bug description

The Data directory protected check DataDirectoryProtected.php still appears to try and check the URL even when the data directory is not in the webroot. The error also indicated that the .htaccess file is not working however it is. The runHead function is being passed $dataUrl which when I modified the code to log $dataUrl showed as just //var/nc_data and the .oc_data was also not appended.

Steps to reproduce

  1. Upgraded from Nextcloud 28.0.5 to 29.0.0.19 with data directory not in webroot

Expected behavior

The check should pass if data directory is not inside of the webroot.

Installation method

Community Manual installation with Archive

Nextcloud Server version

29

Operating system

Debian/Ubuntu

PHP engine version

PHP 8.3

Web server

Apache (supported)

Database engine version

MySQL

Is this bug present after an update or on a fresh install?

Upgraded to a MAJOR version (ex. 22 to 23)

Are you using the Nextcloud Server Encryption module?

Encryption is Disabled

What user-backends are you using?

  • Default user-backend (database)
  • LDAP/ Active Directory
  • SSO - SAML
  • Other

Configuration report

{
    "system": {
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            ***Manually Removed***
        ],
        "trusted_proxies": "***REMOVED SENSITIVE VALUE***",
        "datadirectory": "/var/nc_data",
        "dbtype": "mysql",
        "version": "29.0.0.19",
        "overwrite.cli.url": "***Manually Removed***",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "dbport": "",
        "dbtableprefix": "oc_",
        "mysql.utf8mb4": true,
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "installed": true,
        "activity_expire_days": 14,
        "auth.bruteforce.protection.enabled": "true",
        "simpleSignUpLink.shown": false,
        "blacklisted_files": [
            ".htaccess",
            "Thumbs.db",
            "thumbs.db"
        ],
        "cron_log": true,
        "default_phone_region": "US",
        "enable_previews": true,
        "enabledPreviewProviders": [
            "OC\\Preview\\PNG",
            "OC\\Preview\\JPEG",
            "OC\\Preview\\GIF",
            "OC\\Preview\\BMP",
            "OC\\Preview\\XBitmap",
            "OC\\Preview\\Movie",
            "OC\\Preview\\PDF",
            "OC\\Preview\\MP3",
            "OC\\Preview\\TXT",
            "OC\\Preview\\MarkDown"
        ],
        "filesystem_check_changes": 0,
        "filelocking.enabled": true,
        "htaccess.RewriteBase": "\/",
        "integrity.check.disabled": false,
        "knowledgebaseenabled": false,
        "log_type": "file",
        "logfile": "\/var\/nc_data\/nextcloud.log",
        "loglevel": 2,
        "logdateformate": "F d, Y H:i:s",
        "logtimezone": "America\/New_York",
        "log_rotate_size": 104857600,
        "maintenance": false,
        "memcache.local": "\\OC\\Memcache\\Redis",
        "memcache.locking": "\\OC\\Memcache\\Redis",
        "overwriteprotocol": "https",
        "preview_max_x": 16000,
        "preview_max_y": 16000,
        "preview_max_scale_factor": 1,
        "preview_max_memory": 512,
        "redis": {
            "host": "***REMOVED SENSITIVE VALUE***",
            "port": 0,
            "timeout": 0
        },
        "quota_include_external_storage": false,
        "share_folder": "\/Shares",
        "skeletondirectory": "",
        "theme": "",
        "trashbin_retention_obligation": "auto, 7",
        "updater.release.channel": "stable",
        "mail_smtpmode": "smtp",
        "mail_smtpsecure": "ssl",
        "mail_sendmailmode": "smtp",
        "mail_from_address": "***REMOVED SENSITIVE VALUE***",
        "mail_domain": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpauthtype": "LOGIN",
        "mail_smtpauth": 1,
        "mail_smtphost": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpport": "465",
        "mail_smtpname": "***REMOVED SENSITIVE VALUE***",
        "mail_smtppassword": "***REMOVED SENSITIVE VALUE***",
        "maintenance_window_start": 5,
        "app_install_overwrite": [
            "files_automatedtagging",
            "imageconverter",
            "files_archive"
        ]
    }
}

List of activated Apps

- activity: 2.21.1
  - admin_audit: 1.19.0
  - bruteforcesettings: 2.9.0
  - calendar: 4.7.1
  - circles: 29.0.0-dev
  - cloud_federation_api: 1.12.0
  - comments: 1.19.0
  - contacts: 6.0.0
  - contactsinteraction: 1.10.0
  - dashboard: 7.9.0
  - dav: 1.30.1
  - federatedfilesharing: 1.19.0
  - federation: 1.19.0
  - files: 2.1.0
  - files_archive: 1.2.3
  - files_automatedtagging: 1.19.0
  - files_downloadlimit: 2.0.0
  - files_external: 1.21.0
  - files_pdfviewer: 2.10.0
  - files_reminders: 1.2.0
  - files_sharing: 1.21.0
  - files_trashbin: 1.19.0
  - files_versions: 1.22.0
  - imageconverter: 2.0.1
  - libresign: 9.0.0
  - logreader: 2.14.0
  - lookup_server_connector: 1.17.0
  - mail: 3.6.0
  - maps: 1.4.0
  - nextcloud_announcements: 1.18.0
  - notifications: 2.17.0
  - oauth2: 1.17.0
  - password_policy: 1.19.0
  - photos: 2.5.0
  - previewgenerator: 5.5.0
  - privacy: 1.13.0
  - provisioning_api: 1.19.0
  - recommendations: 2.1.0
  - related_resources: 1.4.0
  - richdocuments: 8.4.0
  - richdocumentscode: 24.4.103
  - serverinfo: 1.19.0
  - settings: 1.12.0
  - sharebymail: 1.19.0
  - support: 1.12.0
  - systemtags: 1.19.0
  - text: 3.10.0
  - theming: 2.4.0
  - twofactor_backupcodes: 1.18.0
  - twofactor_totp: 11.0.0-dev
  - updatenotification: 1.19.1
  - user_status: 1.9.0
  - viewer: 2.3.0
  - weather_status: 1.9.0
  - workflowengine: 2.11.0

Nextcloud Signing status

No errors have been found.

Nextcloud Logs

{"reqId":"OCPRKEMAJw49JilEK9oK","level":2,"time":"2024-04-28T19:51:20-04:00","remoteAddr":"10.0.0.70","user":"user-omitted","app":"no app in context","method":"GET","url":"/core/preview?fileId=604398&x=32&y=32&mimeFallback=true&a=0","message":"Transaction took 1.2166879177094s","userAgent":"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/124.0.0.0 Safari/537.36","version":"29.0.0.19","exception":{"Exception":"Exception","Message":"Transaction took 1.2166879177094s","Code":0,"Trace":[{"file":"/var/www/nextcloud/lib/private/DB/ConnectionAdapter.php","line":154,"function":"commit","class":"OC\\DB\\Connection","type":"->"},{"file":"/var/www/nextcloud/lib/private/Files/Cache/Scanner.php","line":523,"function":"commit","class":"OC\\DB\\ConnectionAdapter","type":"->"},{"file":"/var/www/nextcloud/lib/private/Files/Cache/Scanner.php","line":404,"function":"handleChildren","class":"OC\\Files\\Cache\\Scanner","type":"->"},{"file":"/var/www/nextcloud/lib/private/Files/Cache/Scanner.php","line":354,"function":"scanChildren","class":"OC\\Files\\Cache\\Scanner","type":"->"},{"file":"/var/www/nextcloud/lib/private/Files/Cache/LocalRootScanner.php","line":39,"function":"scan","class":"OC\\Files\\Cache\\Scanner","type":"->"},{"file":"/var/www/nextcloud/lib/private/Files/View.php","line":1341,"function":"scan","class":"OC\\Files\\Cache\\LocalRootScanner","type":"->"},{"file":"/var/www/nextcloud/lib/private/Files/View.php","line":1380,"function":"getCacheEntry","class":"OC\\Files\\View","type":"->"},{"file":"/var/www/nextcloud/lib/private/Files/Node/Root.php","line":208,"function":"getFileInfo","class":"OC\\Files\\View","type":"->"},{"file":"/var/www/nextcloud/lib/private/Files/Node/LazyFolder.php","line":161,"function":"get","class":"OC\\Files\\Node\\Root","type":"->"},{"file":"/var/www/nextcloud/lib/private/Files/AppData/AppData.php","line":98,"function":"get","class":"OC\\Files\\Node\\LazyFolder","type":"->"},{"file":"/var/www/nextcloud/lib/private/Files/AppData/AppData.php","line":147,"function":"getAppDataFolder","class":"OC\\Files\\AppData\\AppData","type":"->"},{"file":"/var/www/nextcloud/lib/private/Preview/Storage/Root.php","line":74,"function":"newFolder","class":"OC\\Files\\AppData\\AppData","type":"->"},{"file":"/var/www/nextcloud/lib/private/Preview/Generator.php","line":607,"function":"newFolder","class":"OC\\Preview\\Storage\\Root","type":"->"},{"file":"/var/www/nextcloud/lib/private/Preview/Generator.php","line":133,"function":"getPreviewFolder","class":"OC\\Preview\\Generator","type":"->"},{"file":"/var/www/nextcloud/lib/private/Preview/Generator.php","line":110,"function":"generatePreviews","class":"OC\\Preview\\Generator","type":"->"},{"file":"/var/www/nextcloud/lib/private/PreviewManager.php","line":187,"function":"getPreview","class":"OC\\Preview\\Generator","type":"->"},{"file":"/var/www/nextcloud/core/Controller/PreviewController.php","line":174,"function":"getPreview","class":"OC\\PreviewManager","type":"->"},{"file":"/var/www/nextcloud/core/Controller/PreviewController.php","line":142,"function":"fetchPreview","class":"OC\\Core\\Controller\\PreviewController","type":"->"},{"file":"/var/www/nextcloud/lib/private/AppFramework/Http/Dispatcher.php","line":232,"function":"getPreviewByFileId","class":"OC\\Core\\Controller\\PreviewController","type":"->"},{"file":"/var/www/nextcloud/lib/private/AppFramework/Http/Dispatcher.php","line":138,"function":"executeController","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->"},{"file":"/var/www/nextcloud/lib/private/AppFramework/App.php","line":184,"function":"dispatch","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->"},{"file":"/var/www/nextcloud/lib/private/Route/Router.php","line":338,"function":"main","class":"OC\\AppFramework\\App","type":"::"},{"file":"/var/www/nextcloud/lib/base.php","line":1050,"function":"match","class":"OC\\Route\\Router","type":"->"},{"file":"/var/www/nextcloud/index.php","line":49,"function":"handleRequest","class":"OC","type":"::"}],"File":"/var/www/nextcloud/lib/private/DB/Connection.php","Line":691,"message":"Transaction took 1.2166879177094s","exception":{},"CustomMessage":"Transaction took 1.2166879177094s"}}

Additional info

The errors in logs appear when browsing various file heavy folders and/or the Photos app.

@nmbgeek nmbgeek added 0. Needs triage Pending check for reproducibility or if it fits our roadmap bug labels Apr 29, 2024
@nmbgeek
Copy link
Author

nmbgeek commented Apr 29, 2024

Modified function in DataDirectoryProtected.php:

        public function run(): SetupResult {
                $datadir = str_replace(\OC::$SERVERROOT . '/', '', $this->config->getSystemValue('datadirectory', ''));
                $webRoot = $this->urlGenerator->getWebroot();
                $dataUrl = $this->urlGenerator->getWebroot() . '/' . $datadir . '/.ocdata';
                error_log('webRoot: '. $webRoot . ' DataDir: '. $datadir . ' DataUrl: ' . $dataUrl, 0);
                $noResponse = true;
                foreach ($this->runHEAD($dataUrl, httpErrors:false) as $response) {
                        $noResponse = false;
                        if ($response->getStatusCode() === 200) {
                                return SetupResult::error($this->l10n->t('Your data directory and files are probably accessible from >                        } else {
                                $this->logger->debug('[expected] Could not access data directory from outside.', ['url' => $dataUrl]);                        }
                }

                if ($noResponse) {
                        return SetupResult::warning($this->l10n->t('Could not check that the data directory is protected. Please chec>                }
                return SetupResult::success();

        }

Error logs this:
Got error 'PHP message: webRoot: DataDir: /var/nc_data DataUrl: //var/nc_data/.ocdata'

Not sure why it doesn't get my webRoot directory though or why the if ($response->getStatusCode() === 200) is returning true as my thought would be the URL would make the runHEAD fail.

@joshtrichards
Copy link
Member

Not sure why it doesn't get my webRoot directory though or why the if ($response->getStatusCode() === 200) is returning true as my thought would be the URL would make the runHEAD fail.

Unless you've deployed Nextcloud in a subfolder, webroot is expected to be empty (well, or /). That part seems right.

What happens if you try to access the full URLs being used in the check by utilizing each of your configured trusted_domains from a web browser (or with curl)?

https://domain1.com/var/nc_data/.ocdata
https://domain2.com/var/nc_data/.ocdata

Or, if you prefer:

https://domain1.com//var/nc_data/.ocdata

@nmbgeek
Copy link
Author

nmbgeek commented Apr 29, 2024

That would be the problem. One of my trusted domains was the box's internal FQDN but the box's DNS is set to public servers. Is there any reason for not bypassing the check if the data directory isn't inside of the webroot?

A check like I created in this possibly: https://github.com/nmbgeek/nextcloud-server/blob/master/apps/settings/lib/SetupChecks/DataDirectoryProtected.php

or at least have it only check the value in 'overwrite.cli.url'?

@joshtrichards
Copy link
Member

joshtrichards commented Apr 29, 2024

That would be the problem. One of my trusted domains was the box's internal FQDN but the box's DNS is set to public servers.

That shouldn't be a problem. We already anticipate that scenario; a similar check is used for .mjs files. If one of the domains tested isn't reachable we move on and try the others.

You didn't answer my question: what happens if you try to access each of the full URLs being used in the check? Does one of them succeed?

Is there any reason for not bypassing the check if the data directory isn't inside of the webroot?

Symbolic links come to mind (maybe? I haven't had my morning coffee yet). But it shouldn't matter either way, the test should pass unless we've either got a bug or an .ocdata file really is accessible via one of your URLs.

or at least have it only check the value in 'overwrite.cli.url'?

It doesn't matter if some of the trusted_domains aren't reachable/etc. We just look for one of them (trusted_domains or the overwrite.cli.url) to work.

@nmbgeek
Copy link
Author

nmbgeek commented Apr 29, 2024

That shouldn't be a problem. We already anticipate that scenario; a similar check is used for .mjs files. If one of the domains tested isn't reachable we move on and try the others.
You didn't answer my question: what happens if you try to access each of the full URLs being used in the check? Does one of them succeed?

The second domain resolves to a different webserver on public DNS due to wildcard subdomain dns record which returns a 200 status code for its generic default page. Internal DNS resolves it directly rather than having to hairpin at my router which then forwards it to Nginx Proxy Manager before hitting the nextcloud server. The nextcloud server's DNS is set to public DNS rather than internal so it is resolving the domain to a public address unlike clients using internal dns which resolve it directly to the nextcloud box.

It was setup some time ago like that for some obscure reason, I believe it had something to do with troubleshooting NPM, and I have removed that domain so the "issue" I was having of it reporting the erroneous error is technically resolved. I think the check could be done better as it never was the issue reported by nextcloud that my .htaccess wasn't working and that my data directory was possibly publicly available.

@NeurohrByteS
Copy link

I can confirm that removing the network's central proxy server (also Nginx Proxy Manager) from the trusted domains in Nextcloud's config resolved the issue.

And I also forgot why I had to add the proxy server in the first place.

@small1
Copy link

small1 commented May 23, 2024

For me neither works. Everything in the config is correct but still for some reason the error is there. i have checked several times that the data directory is not accessible. folder is even deleted. Still the check fails. On the current system where it shows we only have one trusted domain. overwrite is correct.

@small1
Copy link

small1 commented May 23, 2024

curl -I https://.ddns.net/mnt/ncdata/.ocdata
HTTP/2 404
expires: Thu, 19 Nov 1981 08:52:00 GMT
pragma: no-cache

@small1
Copy link

small1 commented May 23, 2024

This is strange. Curl gives a 404 but the code it self suggest that its getting a 200 ok.

@NeurohrByteS
Copy link

I think there is actually an improvement possible for the implementation of the check:

If your request is redirected, and you don't receive /.ocdata then some mechanism is protecting the data directory.

@enoch85
Copy link
Member

enoch85 commented May 24, 2024

Posting this here as well nextcloud/docker#2214 (comment) since I think it's relevant for this issue.

@nmbgeek
Copy link
Author

nmbgeek commented May 25, 2024

What about if the check was for the .ocdata file with a non-forbidden status code and a content-length of 0? Since the .ocdata file is just an empty file if it is actually getting the real ocdata file it should have a content length of 0.

@small1
Copy link

small1 commented May 25, 2024

I have been testing around more. I added debug to the actual check itself to se what url and what host it checks. On this system it is only one host. And the complete url that it checks gives a 404 with curl.

@small1
Copy link

small1 commented May 25, 2024

Found it! I dont know why i missed this. Rewrite rule on port 80 for https generates a 301 as response. That is why its failing.

@enoch85
Copy link
Member

enoch85 commented May 25, 2024

Found it! I dont know why i missed this. Rewrite rule on port 80 for https generates a 301 as response. That is why its failing.

So, the Nextcloud PHP code should be able to handle redirects. That's the solution.

@errror
Copy link

errror commented May 25, 2024

Found it! I dont know why i missed this. Rewrite rule on port 80 for https generates a 301 as response. That is why its failing.

So, the Nextcloud PHP code should be able to handle redirects. That's the solution.

Or correctly generate Test-URLs honoring overwriteprotocol and overwrite.cli.url. In my setup both point to https so why does the test try to access the server using http anyway!

@enoch85
Copy link
Member

enoch85 commented May 25, 2024

Found it! I dont know why i missed this. Rewrite rule on port 80 for https generates a 301 as response. That is why its failing.

So, the Nextcloud PHP code should be able to handle redirects. That's the solution.

Or correctly generate Test-URLs honoring overwriteprotocol and overwrite.cli.url. In my setup both point to https so why does the test try to access the server using http anyway!

Good point!

@small1
Copy link

small1 commented May 25, 2024

it tests for missconfiguraton. Imagine if you forgot to protect stuff on port 80 but had a perfect config for ssl.

@enoch85
Copy link
Member

enoch85 commented May 25, 2024

it tests for missconfiguraton. Imagine if you forgot to protect stuff on port 80 but had a perfect config for ssl.

Also good point! 😆

@svenkubiak
Copy link

Found it! I dont know why i missed this. Rewrite rule on port 80 for https generates a 301 as response. That is why its failing.

So, the Nextcloud PHP code should be able to handle redirects. That's the solution.

I have only set trusted_domains to the domain nextcloud is running (no trusted_proxy) and I can confirm that once I remove 80 -> 443 redirect, the error is finally gone! \o/

@small1
Copy link

small1 commented May 25, 2024

you can add a redirect without generationg a 301 and it would work. Either a better rewrite rule or a Redirct statement

@nmbgeek
Copy link
Author

nmbgeek commented May 25, 2024

I have only set trusted_domains to the domain nextcloud is running (no trusted_proxy) and I can confirm that once I remove 80 -> 443 redirect, the error is finally gone! \o/

I'm not following why you would want to disable https redirects though.

you can add a redirect without generationg a 301 and it would work. Either a better rewrite rule or a Redirct statement

The default certbot rule that it creates for port 80 is below. Why wouldn't you want all http traffic redirected to https?

RewriteEngine on
RewriteCond %{SERVER_NAME} =example.com
RewriteRule ^ https://%{SERVER_NAME}%{REQUEST_URI} [END,NE,R=permanent]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 29-feedback bug feature: settings
Projects
None yet
Development

No branches or pull requests

8 participants