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

New setting: API/Repo Trust level #3278

Open
kevina opened this issue Oct 1, 2016 · 10 comments
Open

New setting: API/Repo Trust level #3278

kevina opened this issue Oct 1, 2016 · 10 comments
Labels
status/deferred Conscious decision to pause or backlog

Comments

@kevina
Copy link
Contributor

kevina commented Oct 1, 2016

Currently we do not verify blocks read from local storage. There is a setting for that, but it defaults to off (#1152). If blocks need to be verified really depends on the level of trust of the disk storage.

There are some similar issues with the pending filestore (#2634) code. For example it is currently possible to add files with the server online. This works by sending both the absolute path of file and the contents. The server will then open the file on its end and verify that the contents are the same as is being sent by the client. This seams secure, but unless blocks are always verified there is a small attack vector. That if the file than changes someone could possible see the contents of the new file. This can only be exploited by someone with access to the API server as they will need to know hash XYZ originally corresponded to the contents of file ZZZ.

Both of these depends on the trust we have with the API/Repo. I therefore propose a new setting, lets call it RepoAPITrust, that has three values 'Low' (assume nothing, always check blocks), 'Medium' (assume the repo storage and API are reasonable secure), High (assume the repo storage and API are highly secure). The default will be 'Low'. For more performance users can be advised to set this to 'Medium'. Using the 'High' setting will be discouraged unless the user really knows what they are doing.

For obvious security reasons this setting should only be allowed to be changed locally. Maybe even by forcing a user to edit a file. In addition this setting will independent of normal settings such as HashOnRead. However, for example, HashOnRead will not be allowed to to False unless the trust level is Medium of Higher.

Thoughts? If there are not any strong objections I am likely to try to implement something soon to avoid possible security problems with the filestore (#2634) code.

@whyrusleeping
Copy link
Member

Hrm... I think this requires some more thought. What would setting this level to 'Medium' or 'High' imply?

Also, how would the filestore work if the client and server are different machines? That seems a little odd, you would have to ensure the same file structures exist on both ends.

@kevina
Copy link
Contributor Author

kevina commented Oct 3, 2016

Also, how would the filestore work if the client and server are different machines? That seems a little odd, you would have to ensure the same file structures exist on both ends.

If the client and server where on different machine than it by default will not be possible to add files after all the idea behind the filestore is to add local files.

There is a way to add files if the two are on different machines, but it is opens up a huge security hole. You would just specify the path of the file on the server. The filestore supports this mode of operation, but is not enabled by default. (The filestore README has details on this mode of operation.) If the trust level setting is implemented than this feature will not be allowed to be enabled unless the trust setting is High.

@whyrusleeping
Copy link
Member

Hrm... I think we should leave that out until we get the filestore actually in and working. Limit scope until we actually land it.

@kevina
Copy link
Contributor Author

kevina commented Oct 3, 2016

if you are talking about just specifying the path on the server, I can disable the code, but I rather not remove it. It is too useful of a feature.

@whyrusleeping
Copy link
Member

Yeah, I mean to disable it so we don't have to worry about all the implications of that scenario in the short term.

@kevina
Copy link
Contributor Author

kevina commented Oct 3, 2016

So the feature is disabled by default...

@whyrusleeping
Copy link
Member

disabled such that you would have to recompile to enable it, yes

@kevina
Copy link
Contributor Author

kevina commented Oct 3, 2016

Okay, I can do that by removing the setting, but would also have to disable some test cases.

@whyrusleeping
Copy link
Member

I'm just worried about leaving some opportunity for arbitrarily reading files from the system open, which is definitely something i can see happening as a result of this

@kevina
Copy link
Contributor Author

kevina commented Oct 5, 2016

Okay I disabled "server side adds" in the code for now. See ipfs-filestore#20.

@whyrusleeping whyrusleeping added the status/deferred Conscious decision to pause or backlog label Nov 28, 2016
@aschmahmann aschmahmann mentioned this issue Dec 1, 2021
80 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/deferred Conscious decision to pause or backlog
Projects
None yet
Development

No branches or pull requests

2 participants