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

remoteStorage as sync backend #87

Closed
CircleCode opened this issue Mar 28, 2018 · 19 comments
Closed

remoteStorage as sync backend #87

CircleCode opened this issue Mar 28, 2018 · 19 comments

Comments

@CircleCode
Copy link

current sync server are all proprietary.
Sync over a standardized protocol could be interesting.

Actually, I read #85, and I deduce that the way webdav manage authentication is not acceptable for pfp? (why it is?)

@palant
Copy link
Owner

palant commented Mar 28, 2018

There are two issues here:

  1. Usually, using WebDAV requires the client to have full credentials, not merely a limited access token like with OAuth. This is an issue because sync info cannot be encrypted - PfP needs to sync even if the user isn't logged in. Storing a WebDAV password as plain text is often a bad idea however. This can be worked around by creating a limited account specifically for the one use case, but it is definitely suboptimal.
  2. Multiple PfP clients might sync at the same time. It is important that one of them won't simply overwrite changes made by the other. My understanding is that the protocol itself doesn't specify a way to prevent such conflicts. Some implementations (Owncloud/Nextcloud) allow sending If-Match header so that the operation will fail if the file changed in the meantime, but I don't think that this is always supported.

Either way, any particular WebDAV server you had in mind? I don't think that speaking about all of them at the same time makes sense here, despite the standardized protocol.

@CircleCode
Copy link
Author

Thanks for the explanations.
Regarding 1. I now understand (I was not thinking about the password storage, only about the transmission part)
Regarding 2., I had no specific implementation in mind, it would have come after this discussion, but as you said, there can be significant differences between implementations, and the possibilities they offer


Maybe this should be another issue, but do you think to other non proprietary, self hostable backends?

@palant
Copy link
Owner

palant commented Mar 29, 2018

It seems that sabre is the prevalent WebDAV implementation, both Owncloud and Nextcloud are based on it. By itself, sabre supports If-Match but no clients with limited access rights. It's the same with Owncloud, in both cases one would have to create a user account with access to a single directory. Nextcloud supports OAuth but so far only in a suboptimal way - there are no scopes, every OAuth client gets full access. Looking at WsgiDAV for comparison, things are very similar there. I guess that limited clients aren't a typical use case for WebDAV.

In fact, WebDAV is way overdimensioned for this use case. So one consideration would be a simple PfP-specific server. A Python-based implementation wouldn't be a big deal, anybody could install it on their server then.

@CircleCode
Copy link
Author

In fact, WebDAV is way overdimensioned for this use case. So one consideration would be a simple PfP-specific server.

I fully agree.

However, I wonder if we cannot adopt an already existing storage backend to avoid having to install something specific to pfp.
For example something like https://remotestorage.io/ (I don't know it, only heard about it…)

What do you think?

@palant palant changed the title webdav sync remoteStorage as sync backend Mar 29, 2018
@palant
Copy link
Owner

palant commented Mar 29, 2018

Looks good. Clean spec, everything necessary seems to be present. I morphed this issue into a request to support remoteStorage, should be easy to implement.

@CircleCode
Copy link
Author

👍
and enough different server implementations to allow users to choose (and no need to develop a backend on your side)

@palant
Copy link
Owner

palant commented Mar 29, 2018

For reference: according to remotestorage/design#3 (comment), there is no logo+text combination for remoteStorage. The recommendation is to use the page's own font. I'll need to see whether it makes sense to do the same for Dropbox and Google Drive, to have things look consistently.

@palant
Copy link
Owner

palant commented May 23, 2019

Supporting remoteStorage in the web client is problematic, a flow that would involve copying and pasting the token manually is not supported. I created remotestorage/spec#174 to ask for this possibility to be added. In the meantime, I solved this by redirecting to https://pfp.works/oauth-endpoint/. This is not optimal as it adds one more place which could be compromised. But at least this webpage will only see the token but not the user address it belongs to.

@palant
Copy link
Owner

palant commented May 23, 2019

Also, at least 5apps.com made quite a mess with the OAuth authentication. Once you authorize an app, redirect_uri is remembered and reused for any subsequent authorizations. This makes it impossible to authorize both the regular extension and the web client, they have to use different values for redirect_uri. It seems that using a different client_id for the web client will solve it here, but I need to see how other servers deal with this.

@raucao
Copy link

raucao commented May 24, 2019

I do the same for rs-backup (source), which is a terminal client for backing up and restoring RS accounts.

The web server actually doesn't see the token at all, because with OAuth it is sent in a URL fragment and not in a query parameter. This also prevents it from appearing in access logs and such.

@palant
Copy link
Owner

palant commented May 24, 2019

The web server actually doesn't see the token at all, because with OAuth it is sent in a URL fragment and not in a query parameter. This also prevents it from appearing in access logs and such.

Sure, the web server won't see anything - as long as this page isn't modified. If the server is compromised, the web page could be modified to log the token. Meaning an unnecessary point of failure.

@palant
Copy link
Owner

palant commented May 24, 2019

The spec says:

if no client registration is required, the server MUST ignore the value of the client_id parameter in favor of relying on the origin of the redirect_uri parameter for unique client identification.

Looking at how this is implemented in various servers:

  • 5apps.com: As already described, their logic is completely bonkers.
  • IndieHosters: Only testable with a very expensive account.
  • php-remote-storage: Also broken, though in a different way (https://github.com/fkooman/php-remote-storage/issues/68).
  • mysteryshack: Correct implementation, but the app is unmaintained.
  • reStore and Armadietto: Does not ignore client_id but displays the host extracted from redirect_uri in addition to it. There is a scheme validation issue here but it should be considered a spec issue (OAuth: Specify allowed schemes for redirect_uri parameter remotestorage/spec#175).
  • rs-serve: Supposed to display redirect_uri origin in the authentication but AFAICT it is never calculated, so there is no info about the client here whatsoever.

@raucao
Copy link

raucao commented May 24, 2019

Sure, the web server won't see anything - as long as this page isn't modified. If the server is compromised, the web page could be modified to log the token. Meaning an unnecessary point of failure.

That's the very security model of OAuth redirect flow, RS, origins, Web Storage, and so on. Of course you want the trusted origin, which is authorized to acquire tokens on behalf of the user, to not be compromised.

When there's no client registration, this is also the only way for storage providers to direct the user to a place that can tell them what the app and who the app provider is, of course.

@raucao
Copy link

raucao commented May 24, 2019

Also, at least 5apps.com made quite a mess with the OAuth authentication. Once you authorize an app, redirect_uri is remembered and reused for any subsequent authorizations. This makes it impossible to authorize both the regular extension and the web client, they have to use different values for redirect_uri.

IIRC that is exactly what the OAuth specs require. The redirect URI is the entire URI btw, so it's very easy to use multiple different ones for different clients (which is also recommended for RS, because of the missing client registration).

@palant
Copy link
Owner

palant commented May 24, 2019

The authentication behavior of 5apps.com is problematic because:

  1. OAuth servers usually support multiple endpoints per client, useful for development vs. production server at the very least. Sure, that's normally paired with client registration, but it's still the expected scenario.
  2. Most remoteStorage servers (as in: all but 5apps.com) won't require unchanged redirect URIs per client, neither is this required by the remoteStorage spec. So while most clients won't work with multiple OAuth endpoints, they won't expect changing endpoind URL (different path on the same domain) to cause issues either.
  3. 5apps.com does not reject an unexpected redirect_uri parameter, it will simply redirect to its stored redirect URI instead. If the two are different, I cannot see how this could possibly result in anything useful. Also, the user won't have any hint towards what went wrong or how to correct this (revoke app token and restart the process).
  4. Finally, by displaying client ID and even linking to it 5apps.com clearly violates the spec and facilitates phishing attacks. Anybody could set client_id to https://www.google.com/, it's redirect_uri that matters - and this one could point to some phishing website.

Note how none of this would be an issue if 5apps.com used the origin of redirect_uri as "client ID" as required per spec. Having different client IDs for different endpoints would happen automatically and not something that developers have to worry about.

@palant palant closed this as completed in 610c496 May 24, 2019
@palant
Copy link
Owner

palant commented May 24, 2019

I think I'm done. As things are now, it should work with any remoteStorage servers. It has only really been tested against 5apps.com however, and there are significant differences between implementations.

@raucao
Copy link

raucao commented May 24, 2019

I understand the issue now, and it's actually quite surprising to me, because I do not remember it working this way. Looks like a fairly recent change in preparation for the new dashboard caused this bug. We will fix it asap and require client ID to be the same as redirect URI again.

(For historic context: we were actually one of the first projects doing just that, and arguing for it to be a reasonable choice for open web apps. In the meantime, others have validated this opinion.)

@raucao
Copy link

raucao commented May 24, 2019

... by the way, be sure to add this app to the list on the RS wiki, whenever RS support is released: https://wiki.remotestorage.io/Apps

Looks promising! Looking forward to trying it out myself.

palant added a commit that referenced this issue Jun 2, 2019
@palant
Copy link
Owner

palant commented Jun 12, 2019

... by the way, be sure to add this app to the list on the RS wiki, whenever RS support is released: https://wiki.remotestorage.io/Apps

Done (release came out two days ago). Filed #104 on an outstanding issue here but otherwise everything seems to be working fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants