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

Public shares return 401 with federation disabled #20132

Closed
jhesketh opened this issue Mar 24, 2020 · 19 comments
Closed

Public shares return 401 with federation disabled #20132

jhesketh opened this issue Mar 24, 2020 · 19 comments
Labels
1. to develop Accepted and waiting to be taken care of bug feature: dav feature: sharing
Milestone

Comments

@jhesketh
Copy link

Thanks to @skjnldsv for help in debugging and finding this.

How to use GitHub

  • Please use the 👍 reaction to show that you are affected by the same issue.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

Steps to reproduce

  1. Disable the option "Allow users on this server to send shares to other servers"
  2. Create a share link for a folder with images
  3. Open the link in a new browser session
  4. The list of files/images will load, but trying to open one with the viewer will fail

Expected behaviour

The image should open successfully in the viewer.

Actual behaviour

The image is not opened and the viewer just spins. The debug console and apache logs show a 401 response for the file request.

Server configuration

Operating system:
nexcloud-apache docker image (ubuntu)

Web server:
apache

Database:
mariadb

PHP version:
7...

Nextcloud version: (see Nextcloud admin page)
Daily build: Nextcloud 18.0.2 Build:2020-03-23T23:01:54+00:00 4ea22ab

Updated from an older Nextcloud/ownCloud or fresh install:
Updated.

Where did you install Nextcloud from:
Docker

Signing status:

Signing status

No errors have been found.

List of activated apps:

App list
Enabled:
  - activity: 2.11.0
  - admin_audit: 1.8.0
  - apporder: 0.9.0
  - bruteforcesettings: 1.5.0
  - cloud_federation_api: 1.1.0
  - comments: 1.8.0
  - dav: 1.14.0
  - external: 3.5.0
  - extract: 1.2.3
  - federatedfilesharing: 1.8.0
  - federation: 1.8.0
  - files: 1.13.1
  - files_antivirus: 2.2.1
  - files_downloadactivity: 1.7.0
  - files_fulltextsearch: 1.4.1
  - files_fulltextsearch_tesseract: 1.4.1
  - files_pdfviewer: 1.7.0
  - files_rightclick: 0.15.2
  - files_sharing: 1.10.1
  - files_trashbin: 1.8.0
  - files_versions: 1.11.0
  - files_videoplayer: 1.7.0
  - fulltextsearch: 1.4.1
  - fulltextsearch_elasticsearch: 1.5.0
  - impersonate: 1.5.0
  - logreader: 2.3.0
  - lookup_server_connector: 1.6.0
  - maps: 0.1.6
  - metadata: 0.11.1
  - news: 14.1.4-rc1
  - notifications: 2.6.0
  - oauth2: 1.6.0
  - ocr: 6.0.25
  - onlyoffice: 4.1.4
  - photos: 1.0.0
  - previewgenerator: 2.2.0
  - privacy: 1.2.0
  - provisioning_api: 1.8.0
  - recommendations: 0.6.0
  - serverinfo: 1.8.0
  - settings: 1.0.0
  - sharebymail: 1.8.0
  - spreed: 8.0.5
  - systemtags: 1.8.0
  - text: 2.0.0
  - theming: 1.9.0
  - twofactor_backupcodes: 1.7.0
  - twofactor_totp: 4.1.3
  - viewer: 1.2.0
  - workflowengine: 2.0.0
Disabled:
  - accessibility
  - encryption
  - files_external
  - firstrunwizard
  - nextcloud_announcements
  - password_policy
  - richdocuments
  - sharerenamer
  - sociallogin
  - support
  - survey_client
  - updatenotification
  - user_ldap

Nextcloud configuration:

Config report

{
"system": {
"memcache.local": "\OC\Memcache\APCu",
"apps_paths": [
{
"path": "/var/www/html/apps",
"url": "/apps",
"writable": false
},
{
"path": "/var/www/html/custom_apps",
"url": "/custom_apps",
"writable": true
}
],
"memcache.distributed": "\OC\Memcache\Redis",
"memcache.locking": "\OC\Memcache\Redis",
"redis": {
"host": "REMOVED SENSITIVE VALUE",
"port": 6379
},
"instanceid": "REMOVED SENSITIVE VALUE",
"passwordsalt": "REMOVED SENSITIVE VALUE",
"secret": "REMOVED SENSITIVE VALUE",
"trusted_domains": [
"cloud.hesketh.net.au"
],
"trusted_proxies": "REMOVED SENSITIVE VALUE",
"datadirectory": "REMOVED SENSITIVE VALUE",
"dbtype": "mysql",
"version": "18.0.2.2",
"overwrite.cli.url": "https://",
"overwriteprotocol": "https",
"dbname": "REMOVED SENSITIVE VALUE",
"dbhost": "REMOVED SENSITIVE VALUE",
"dbport": "",
"dbtableprefix": "oc_",
"mysql.utf8mb4": true,
"dbuser": "REMOVED SENSITIVE VALUE",
"dbpassword": "REMOVED SENSITIVE VALUE",
"installed": true,
"mail_smtpmode": "smtp",
"mail_sendmailmode": "smtp",
"mail_smtpsecure": "ssl",
"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",
"twofactor_enforced": "false",
"twofactor_enforced_groups": [],
"twofactor_enforced_excluded_groups": [],
"loglevel": 2,
"maintenance": false,
"app_install_overwrite": [
"impersonate"
],
"data-fingerprint": "",
"theme": ""
}
}

Client configuration

Browser:
Firefox and Chromium

Operating system:

Linux

Logs

Web server error log

Web server error log

app_1 | 172.30.0.2 - 5r8NPAKzB [23/Mar/2020:09:31:22 +0000] "PROPFIND /public.php/webdav/IMG_2019_21**.jpg HTTP/1.1" 401 1055 "-" "Mozilla/5.0 (X11; Linux x86_64; rv:72.0) Gecko/20100101 Firefox/72.0"

Nextcloud log (data/nextcloud.log)

Nextcloud log

NA

Browser log

Browser log

Content Security Policy: Directive ‘child-src’ has been deprecated. Please use directive ‘worker-src’ to control workers, or directive ‘frame-src’ to control frames respectively.
Content Security Policy: The page’s settings blocked the loading of a resource at inline (“script-src”). 3 content-script.js:40:65
JQMIGRATE: Migrate is installed, version 1.4.1 jquery-migrate.min.js:2:551
This page uses the non standard property “zoom”. Consider using calc() in the relevant property values, or using “transform” along with “transform-origin: 0 0”. addStylesClient.js:220:17
Handlebars is deprecated: please ship your own, this will be removed in Nextcloud 20 2 globals.js:66:15
OCA.Viewer initialized Viewer.js:41:10
Content Security Policy: The page’s settings blocked the loading of a resource at eval (“script-src”). getInferredName.js:6:19
TypeError: "OCA.Text is undefined"
data RichWorkspace.vue:75
VueJS 6
Me
_data
_n
_n
_init
a
render files.js:188
vue.esm.js:1897:12
VueJS 8
We
Je
Ve
_data
_n
n
init
a
render files.js:188
The humanFileSize library is deprecated! It will be removed in nextcloud 19. 2 globals.js:66:15
The humanFileSize library is deprecated! It will be removed in nextcloud 19. globals.js:66:15
The humanFileSize library is deprecated! It will be removed in nextcloud 19. globals.js:66:15
The humanFileSize library is deprecated! It will be removed in nextcloud 19. globals.js:66:15
The humanFileSize library is deprecated! It will be removed in nextcloud 19. 5 globals.js:66:15
The humanFileSize library is deprecated! It will be removed in nextcloud 19. globals.js:66:15
The humanFileSize library is deprecated! It will be removed in nextcloud 19. 2 globals.js:66:15
Opening the viewer with a single string parameter is deprecated. Please use a destructuring object instead OCA.Viewer.open({ path: '/IMG
.jpg' }) Viewer.js:97:11
Opening viewer for file /IMG
.jpg Viewer.vue:211
Error: "Request failed with status code 401"
exports createError.js:16
exports settle.js:17
onreadystatechange xhr.js:61
Viewer.vue:383
t Viewer.vue:383
c runtime.js:45
_invoke runtime.js:271
t runtime.js:97
H viewer.js:355
a viewer.js:355

@jhesketh jhesketh added 0. Needs triage Pending check for reproducibility or if it fits our roadmap bug labels Mar 24, 2020
@skjnldsv
Copy link
Member

@skjnldsv skjnldsv added 1. to develop Accepted and waiting to be taken care of feature: dav feature: sharing and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Mar 24, 2020
@nickvergessen
Copy link
Member

It's like this since ever:
f2a900e

The problem is we can not tell the difference if it is a remote server grabbing the file or a user, so from my perspective it is a "will not fix" and instead the description needs adjusting.

@skjnldsv
Copy link
Member

I'm not sure I understand the issue here @nickvergessen. What difference does it make that it's a remote server or not? The share link is public to whomever have the link.

@nickvergessen
Copy link
Member

Federated shares work via public links 😱

@neufeind
Copy link

Not really "intuitive" while "technically correct". If you tend to have this as a a "won't fix" then let's at least improve error-handling by pointing a user/admin in the right direction. Currently it can happen that the photo-viewer (see nextcloud/photos#128) just keeps spinning without a proper error and the 401 is silently returned in the background :-(

@jhesketh
Copy link
Author

To be honest, I'm not sure if this is "technically correct". Why do the federated shares need to work via public links? I get that they can't be authenticated in the same way, but they could have a flag on them that denotes they are different to a normal public link.

Additionally an extra configuration option to allow public links at all could replace the current security option. And, as mentioned, allow for a better error when it is used.

@jospoortvliet
Copy link
Member

To be honest, I'm not sure if this is "technically correct". Why do the federated shares need to work via public links? I get that they can't be authenticated in the same way, but they could have a flag on them that denotes they are different to a normal public link.

So if I understand it correctly - if you share a file with a public link another Nextcloud could use that to show the data to its users... You can't stop it from doing that. And that is exactly what federation does... So if you want to stop other Nextcloud's from being able to have the files available to their users, you disable public links.

Look at it this way - if you share an image publicly over twitter and somebody downloads it and shares it on Instagram, you might not like that, but the only way to stop that is by not sharing it on twitter in the first place. That's just how digital things work.

If it works that way, we should perhaps add a warning that disabling federation also disables public links in general, as nickvergessen said.

@jean-io
Copy link

jean-io commented Apr 24, 2020

Federated shares work via public links 😱

So federation is all public? at no point there is authentication between servers?

It doesn't seem very secure...

@jhesketh
Copy link
Author

So if I understand it correctly - if you share a file with a public link another Nextcloud could use that to show the data to its users... You can't stop it from doing that. And that is exactly what federation does... So if you want to stop other Nextcloud's from being able to have the files available to their users, you disable public links.

Look at it this way - if you share an image publicly over twitter and somebody downloads it and shares it on Instagram, you might not like that, but the only way to stop that is by not sharing it on twitter in the first place. That's just how digital things work.

Yep, that's absolutely true. Anybody could screenshot or save an image and then distribute it however they like.

That said, it's not really a security issue in my mind. It's a feature issue. Specifically, if you've made shares by links in the past and you want to disable it system wide, then it should stop them all from working. Similarly if you've shared with a federated cloud, you could immediately disable all those existing shares. Lastly, if you have federated links enabled and share links disabled, then the share link is never actually exposed to the user (as far as I know) thus preventing them from passing it along.

Additionally, I'm not sure if this is how it is implemented, but it would be possible to add extra verification to the receiving link. For example, it is possible for only the receiving server to know the origin's URL. The server can then import or proxy the share to the authenticated user. This way you are sure that the user you are sharing it with is the one viewing it. You can also ensure they aren't passing along a link to anybody else (since they need to be logged in). (Obviously there is still the risk that the user saves and redistributes the image themselves, but again that's always a risk and at least you know where the leak was likely from).

To do all this, there should be a separate mechanism for sharing links vs sharing federated shares. (Even if in the background it is just a hashed URL).

@neufeind
Copy link

But taking that into account I wonder why with federation disabled it's possible to open a public share at all, meaning: get a list of files. And only after clicking on a file you get a 401 from the webdav-backend - which, in case of the image-viewer, is hidden in the background and the loader keeps spinning. If there was an error in the first place that the public link is invalid or stating that public/federation-sharing is disabled it would point the user in the right direction.
And maybe the administration-UI could then also stress more that "federation" also includes anything "public" - and by enabling/disabling it the public functionality is affected as well.

@jhesketh
Copy link
Author

Agreed. Ideally they are separate options, but at a minimum or perhaps interim the current option should be explained and the errors handled.

Also, why do thumbnails and file listings work?

@Walterfilms
Copy link

Walterfilms commented Apr 27, 2020

until nextcloud update 18.0.4 we never activated federation, we always used shared links to send to our customers links to videos and it was working well...

I might be wrong but it seems that nextcloud is not using "video player" app for public shares but the "viewer" app so that might explain the problem ?

@skjnldsv
Copy link
Member

I might be wrong but it seems that nextcloud is not using "video player" app for public shares but the "viewer" app so that might explain the problem ?

Both are using the same endpoint for this. So it should not.
I'll invesitage.

@Walterfilms
Copy link

My bad, I was wrong and confused nextcloud is still using "videoplayer" app but only when sharing directly a video, and using "viewer" app when sharing a folder containing videos and i guess it has been like this since a long time...

thanks for investigating anyway ;)

@skjnldsv skjnldsv added this to the Nextcloud 20 milestone Apr 30, 2020
@skjnldsv
Copy link
Member

skjnldsv commented Apr 30, 2020

Let's see if we can find another way to pass the info we need for 201.
I'd like us not to polute the dav endpoint with improper use of data that are not supposed to be here.

EDIT: wrong issue, this is this one #19700
But they're close to each other to be honest

@GuyPaddock

This comment has been minimized.

@rullzer rullzer modified the milestones: Nextcloud 21, Nextcloud 22 Mar 1, 2021
@skjnldsv skjnldsv removed their assignment Jul 5, 2021
@skjnldsv skjnldsv modified the milestones: Nextcloud 22, Nextcloud 23 Jul 5, 2021
@GuyPaddock
Copy link

Any ETA on a fix for this issue?

@szaimen
Copy link
Contributor

szaimen commented Jul 29, 2021

I am not sure what has changed but at least I cannot reproduce this issue anymore on NC21.0.3

@skjnldsv
Copy link
Member

Yes, seems fixed.
We still need to improve with #19700

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of bug feature: dav feature: sharing
Projects
None yet
Development

No branches or pull requests

10 participants