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

config option for external storage #12216

Closed
6 of 15 tasks
karlitschek opened this issue Nov 17, 2014 · 36 comments
Closed
6 of 15 tasks

config option for external storage #12216

karlitschek opened this issue Nov 17, 2014 · 36 comments

Comments

@karlitschek
Copy link
Contributor

karlitschek commented Nov 17, 2014

We have to make it possible to configure the behavior of externaly mounted storages in a more detailed way.

Edit: added user sharing

@karlitschek
Copy link
Contributor Author

@icewind1991

@karlitschek
Copy link
Contributor Author

@MTRichards FYI

@LukasReschke
Copy link
Member

Capturing Credentials on Login (an option on a per mount basis)

Requires discussion whether we want:

  1. Store the credentials encrypted in mount.json
    • Advantage: We can access the storage even if user is not logged-in
    • Contra: Password is permanently stored
  2. Store the credentials encrypted in the PHP sessions
    • Advantage: Password is not necessarily permanently stored

I see valid use-cases for both approaches, the first approach would make it possible to scan the FS using occ etc...

@PVince81
Copy link
Contributor

@craigpg should long term tickets like these be added to the sprint for visibility, even though they will certainly never be completed within a single sprint ?

@MTRichards
Copy link
Contributor

@LukasReschke I would like to have 1) set for ownCloud 8 first, so that sharing works. After that, we can look at 2) (or even a 3) option with tokens used). This would essentially just add 1 more option to the sharepoint / windows network drive options that ecist, and consolidate that admin experience into external storage so all external storage mounts can share the UX from a central location.

@PVince81
Copy link
Contributor

Whoever is going to work on these features is at risk to run against the bad code, horrible config format and issues mentionned here: #11261

Refactoring Sharepoint/WND to be based on that might still work, but I'm not sure about the extra UX gimmicks like detecting unavailability and showing icons in the files list.

@MTRichards what are the sharing issues you are talking about that would be solved by 1) UX enhancements ? Once we know we can raise a separate ticket for that specific part.

@MTRichards
Copy link
Contributor

@PVince81 Oh, let me clarify. 1) The idea is we always login to the external storage as the user that owns the file. If we don't store the password somewhere, than I share a file with you, you can't get access to the file I shared with you that lives on external storage because the system lacks the password.

@PVince81
Copy link
Contributor

Hmm I see, I believe that's the issue @Xenopathic met when writing SMB_OC. So if we have a generic mechanism that can be reused for all storage backends, it would be much better.

@MTRichards @karlitschek can you guys elaborate on point 1) in a separate more specific ticket ? Goal is to come up with a list of steps on things to implement for that part.

@MTRichards
Copy link
Contributor

@PVince81 Not wanting bad ugly code, but we should have a unified UX. The UX we put together for WND and SharePoint was built with @jancborchardt to make the UX easy for admins and users. This experience should be unified across all external storage, not unique to some and not others. That work just has to be done, and then the external storage app needs to be the one place that sets these UX elements going forward, as you said previously.

Then new connectors just use those elements rather than reinventing the wheel.

@PVince81
Copy link
Contributor

Agreed. The thing I wanted to make sure is that we properly separate "UX improvements" from "fix the sharing issue" which aren't exactly the same thing technically and would be at least two separate steps/tasks/tickets.

From what I understand the sharing issue is more point 2) where credentials are captured, and that part would solve the sharing issue.

For part 1) UX improvements, let's make a ticket and make a checklist of these improvements.

@RobinMcCorkell
Copy link
Member

@PVince81 A unified get-password-from-session and create-derived-mount-point would be very useful, and would avoid needless code duplication like with SMB_OC. Other mount types could benefit from this, as authentication is often shared between multiple applications via a directory like LDAP.

As for the sharing issue - we could always go the paranoid way and encrypt the user password for each person with whom a mountpoint is shared, with their password. So if user 'foo' with password 'bar' shares their SMB mount point with 'baz' with password 'qux', then 'bar' will be encrypted with 'qux' and stored in 'bar' 's configuration somewhere. (This approach is completely hypothetical and I don't think it is a good idea! files_encryption makes my head hurt as it is...)

@PVince81
Copy link
Contributor

@MTRichards I raised the UX improvements as separate ticket/task for further discussion: #12338

@MTRichards
Copy link
Contributor

Versions and trashbin are also about how integrated they are. Almost like saying that the external storage can implement a versions provider, and a trash bin provider for that folder. Thus we don't have duplicate, inconsistent and missing trash and versions between external storage and ownCloud.

@MTRichards
Copy link
Contributor

We also need to be sure there is a way to change the auth mechanism to use tokens, basic or something else when authenticating with external storage backends.

@PVince81
Copy link
Contributor

The storage specific versions/trashbin was already raised here: #12274

For the auth mechanism this depends on the plugin itself. I believe that the configuration itself isn't flexible enough to accomodate multiple auth modes. It might be possible for a plugin to register itself multiple times, each time with a different configuration format. So one could be called "SFTP with password" and another one "SFTP with auth keys". Not sure if this needs additions, I'll simply add it as bullet point in the requirements.

@PVince81
Copy link
Contributor

@MTRichards I re-read your comment here #12216 (comment) about the priorities, we need to clear this out again.
You said 1) UX improvements should be done in OC 8 because you thought it would fix sharing, but it will NOT (see #12338 (comment) for clarification). I believe that it's 2) that would fix sharing through credential storing, so that should probably have a higher priority than the UX improvements from 1).

Is that correct ?

@cedric-dufour
Copy link

about Capturing Credentials on Login (an option on a per mount basis)
(coming from #11939 and having looked at #11747)

Hello,

I think it should be possible for the administrator to choose between:

  1. mount.json
  2. PHP session

In many environments - corporate, academia (I stand there), etc. - it would go against recommended best practices (and likely policies) to have passwords stored on disk in a manner that allows their retrieval (as in 1.).

Given this constraint, encrypted PHP session storage (2.) would be acceptable if:
a. the PHP session ID is used to derive the encryption key (along potential additional secrets)
b. the PHP session handler is changed (http://php.net/manual/en/session.customhandler.php) such as to not derive the PHP session file name from the plain session ID (but from a cryptographic hash of the session ID instead)
Having access to the disk - a very likely scenario when a server has been hacked - would thus not be enough to uncover the encrypted material in the PHP session file (because the PHP session ID used as part of the encryption key would not be recoverable from that session file(name))

Point a. can easily be implemented in ownCloud, if it chooses to also support 2. (users could thus choose between 1. and 2., based on the pros/cons of each)
Point b. could be left to those sysadmins who need it (at least in a first step; http://php.net/manual/en/class.sessionhandlerinterface.php seems quite straightforward though)

PS: If you deem it appropriate, I could give this approach a shot (starting next year)

@LukasReschke
Copy link
Member

Having access to the disk - a very likely scenario when a server has been hacked - would thus not be enough to uncover the encrypted material in the PHP session file (because the PHP session ID used as part of the encryption key would not be recoverable from that session file(name))

For that case we'd also need to promote #11843 otherwise this would be not a that big advantage as an attacker would be able to execute arbitrary PHP code, therefore being able to gain access to all secrets. (admittedly, requires write-access, but this is not that unlikely)

This also requires some handling on files_external what happens with shares etc. - That said, the approach seems legit to me.

@PVince81
Copy link
Contributor

PVince81 commented Dec 5, 2014

Raised the credential capture as #12635

@PVince81
Copy link
Contributor

PVince81 commented Mar 4, 2015

What was "cache refresh behavior" option again ? Is it to allow a "force full rescan" to detect outside changes, with a configurable interval ? (could be a solution for #5036)

@PVince81
Copy link
Contributor

The config option for encryption was added here: #10731

Setting milestone to 8.2 to look into the other entries.

@ghost
Copy link

ghost commented Sep 10, 2015

@PVince81 @Xenopathic update?
@karlitschek @MTRichards ping

@karlitschek
Copy link
Contributor Author

i think this should be done as part of the external storage UI consolidation

@RobinMcCorkell
Copy link
Member

There are a lot of improvements listed here, they won't all come in one go, and certainly not for 8.2

@RobinMcCorkell RobinMcCorkell modified the milestones: 9.0-next, 8.2-current Sep 10, 2015
@MTRichards
Copy link
Contributor

Thanks @cmonteroluque
This is the long standing list for additional configuration options for all external storage. I think iterative and incremental is just fine, and we can roll it all in as @karlitschek says.

We do want to, however, keep it in sight and make progress against better and more complete UFA over time. I'll take any advice on implementation order, but we need to do some of this each release to get there long term or we will be here again next year.

@PVince81
Copy link
Contributor

I edited the original post to split "User sharing" from "Link sharing" as discussed in #19157

@ghost ghost modified the milestones: 9.1-next, 9.0-current Feb 17, 2016
@PVince81
Copy link
Contributor

PVince81 commented Jul 6, 2016

Moving to 9.2 to continue working on the remaining items.

CC @dragotin @DeepDiver1975

@PVince81
Copy link
Contributor

@pmaier1 would move to backlog or future for now when we have time to resume working on the remaining items.

@PVince81
Copy link
Contributor

moving to backlog now, needs rescheduling of individual items

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

10 participants