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

NodePublish – Secure access to shared mounted volumes #17

Open
oritnm opened this issue May 25, 2017 · 22 comments
Open

NodePublish – Secure access to shared mounted volumes #17

oritnm opened this issue May 25, 2017 · 22 comments
Labels

Comments

@oritnm
Copy link

oritnm commented May 25, 2017

We see couple of challenges with the current model.

  • The users namespace may span multiple clusters and identity management systems.
  • The container and host namespaces are decoupled of one another and should be kept as such.
  • The users within the containers may be unknown to the storage provider or run with uid 0 (providing root access to the underlying files)
  • A shared mounted volume may be accessible to multiple containers, each having different read/write access permission to the underlying files.

To provide authorized access to shared mounted volume, the IO request need to be accounted on behalf of a user known to the storage provider allowing authorizations such as POSIX and others to take place.
The user/group identity should be added along with secrets to the node publish request. (Using the volume metadata is insufficient since it would require all containers to use the same user identity).

Can see https://www.quobyte.com/blog/2017/03/17/the-state-of-secure-storage-access-in-container-infrastructures/
https://github.com/sigma/cifs_k8s_plugin

@casusbelli
Copy link

Good point, i'm looking at this aspect, too.
Currently i'm preparing a pullreq with some change regarding this. I'll reference this issue in the pr.

@jieyu
Copy link
Member

jieyu commented Jun 1, 2017

That brings up an interesting point to the current workflow. Currently, the spec assumes that the CO will publish the volume to the node once. If there are multiple containers on the node that want to access to the volume, the CO will be responsible for bind mount the volume into the container. What if these two containers have different identities?

@julian-hj
Copy link
Contributor

This is a very good catch. We handle this use case in Cloud Foundry, but CF has a slightly different flow today. Our version of the "controller" plugin is filled by an implementation of the CNCF Open Service Broker API. That API includes a "Bind" endpoint that allows the plugin to do some work when the volume is associated with a container workload (or "application" in CF parlance). The service broker plugin has the opportunity to create credentials in that binding response that are eventually handed back to the node plugin at mount time.

Maybe we need to consider adding some similar endpoint to the master API? IMO it will be difficult for the CO to know how to concoct the credentials data that is needed by the node plugin. In CF we have a few different service brokers that all use the same volume driver (or node plugin) to mount nfs shares, but have totally different schemes to assign UIDs to a running application.

@cpuguy83
Copy link
Contributor

cpuguy83 commented Jun 2, 2017

I think it makes sense to always call NodePublish (likely with a unique ID for publish/unpublish pairs) for each container. The plugin can choose how it wants to handle that, be it by doing nothing at all and returning the same path for each request or by returning different paths.

If we later realize that it's too much then we can just switch to making only 1 publish/unpublish call and it should be no difference to the plugin itself, but going the other way (adding multiple publish/unpublish later) is more difficult.

@jieyu
Copy link
Member

jieyu commented Jun 5, 2017

One option would be allow a CO to call NodePublish multiple times with a different target_path (and potentially with different parameters) if the access_mode of the volume is MULTI_NODE_xxx. This is essentially the same workflow as publishing the volume to a different node. In the teardown path, the CO will be responsible for calling NodeUnpublish multiple times, one for each target_path. The question is: whether ControllerPublishVolume should be called multiple times or not in that case. My hunch is probably not, because when dealing with Node gone scenarios, it probably does not make sense to call ControllerUnpublishVolume multiple times. Also, I imagine for NFS (or fuse) like plugins, most likely the plugin won't set PUBLISH_UNPUBLISH_VOLUME in the controller service (i.e., a no-op).

To solve the user/group issue, we still need to introduce parameters that a CO can set differently for each NodePublish. Instead of first class user and group here, I would actually prefer a parameters field. The definition of user and group is very specific to a plugin.

@cpuguy83, to your comment: Unlike DVDI, the CSI spec asks the CO to specify the target_path, rather than the plugin returns the mounted_path. This allows us to achieve idempotency for NodePublish.

@oritnm
Copy link
Author

oritnm commented Jun 6, 2017

IMHO, NodePublish is required for every container that will be using this volume, different target_path and user credential should be able to be provided for every container independently

@jieyu
Copy link
Member

jieyu commented Jun 6, 2017

@oritnm The reason we don't want NodePublish to be per-container initially is that for backends like iscsi, the 'attach' should happen only once, and the logic of attach has to be in NodePublish because iscsi does not support remote attach. If the CO invokes NodePublish multiple times, we'll end up calling 'attach' multiple times.

@oritnm
Copy link
Author

oritnm commented Jun 7, 2017

@jieyu understood but since the nodepublish request needs to be idempotent and the CO can chose to call it again in a case it didn't get the response then the Storage Plugin needs to handle this situation any way (even in the case of not having a specific access mode for it)

@quolix
Copy link

quolix commented Jun 9, 2017

@jieyu I understand that NodePublish is currently meant to be per volume and not per container, but this would preclude plug-ins to implement secure/authenticated access to storage. I don't see how this can be fixed later?

Also: whall we file another issue to point out this specific shortcoming and decouple its discussion. This issue is more about how to implement secure access.

@jieyu
Copy link
Member

jieyu commented Jun 9, 2017

@oritnm that's a good point. We were thinking that the idempotency of NodePublish can be achieved by just looking at target_path. But sounds like in some case, this is not possible. I agree with you that for the iscsi case, the plugin anyway has to have some logic to detect if a volume has been attached or not to achieve idempotency, even if we intend to just publish the volume at one location. For instance, it needs to handle the case where the plugin crashes after the volume has been attached, but before it is able to mount to target_path.

If we want to go with the per-container publish route, I think we need to figure out what exactly are the per-container information that CO will pass to the Plugin, how that can be generalized so that it's meaningful to all COs, and how that information can be validated by the CO.

@oritnm
Copy link
Author

oritnm commented Jun 13, 2017

@jieyu ,@quolix, Opened a separate issue for multiple invocation of NodePublish : under #40

@jieyu
Copy link
Member

jieyu commented Jun 14, 2017

Thank you @oritnm. Will follow up with the working group on this asap.

@casusbelli
Copy link

FYI: I squashed the range of commits in PR #30 to single concise commit, see there for details.

@saad-ali
Copy link
Member

There are a few different levels of authentication/authorization.

  1. Volume plugin auth to storage backend.
    • These are the credentials that the volume plugin will use to auth it self with the storage control plane.
    • This is not defined in spec (since the spec does not define packaging).
    • This will be defined in packaging by COs, and will most likely be a file or env variable in the volume plugin container that contains the credentials (for plugins that need it).
  2. End user auth per CSI RPC.
    • Just because the plugin has the ability to create/attach/mount a volume does not mean that it should do on behalf of any user.
    • We must modify the CSI spec to be able to pass through end user credentials on create/publish calls so that each call can be auth'd (for plugins that need it).
    • I sent out PR https://github.com/container-storage-interface/spec/pull/83/files to do this.
  3. OS level file/directory permissions.

@gpaul
Copy link

gpaul commented Aug 17, 2017

Reproducing my comment on PR 83 here:

I'm not clear on how the CO will know what credentials to pass to a given plugin.

Some plugins will require username/password others a JWT and others nothing at all.

I'd prefer a set of official strategies like UsernamePassword, Username, NoAuth, GenericSecret, etc. then have the plugin respond to the CO's initial Get*Capabilities() request with it's preferred authentication mechanism.

That way the CO only needs to support a small set of authentication strategies that are clearly specified by this spec.

This let's CO integrate with new plugins without changing code or leaking SP-specific details through the CSI plugin or around it (through SP-specific documentation around authentication).

One of the best things about the CSI is that the CO doesn't need to write special code for every SP.

Having to read the SP documentation to figure out what keys are expected and how to format the values seems counterproductive.

@cpuguy83
Copy link
Contributor

I think we can use a google.protobuf.Any (https://github.com/google/protobuf/blob/master/src/google/protobuf/any.proto#L112) we can pass around both the type information and the details and leave it opaque to the CO.

@gpaul
Copy link

gpaul commented Aug 17, 2017

@cpuguy83 Interesting concept, I've never seen that before. That may be useful although (if I take your meaning) it would appear to require a working internet connection from the cluster to the SP - which given that the actual data resides at the SP sounds quite reasonable to me. But still, how will the CO know how to populate whichever fields are declared in the response?

I'm not clear how authentication can be opaque to the CO. It seems that fundamentally the CO needs to get details about the user to the SP through the CSI plugin?

Depending on which SP, the exact details will be different.

I imagine a CO cluster configured with two SPs (SP-1 and SP-2).

Both SPs require the end-user to be authenticated to determine whether the volume may be published RW vs. RO vs. not at all as well as for auditing that user's actions performed on the data once published.

SP-1 requires that the end-user authenticate using a username+password combination. This information is likely stored as a secret by the CO and should accompany any Create/Destroy/*Publish/etc. RPCs to the CSI plugin for SP-1. Perhaps there's some mapping between secret name and SP credential which let's the CO determine which secret is the SP-1 username and which is the SP-1 password?

SP-2 is integrated with the specific CO cluster through some Single Sign-On component and expects the CO to pass it a valid authentication token that represents the end-user.

It seems to me that the CO will need to perform quite a lot of authentication-specific logic from when the user launches the application until some volume is published to a Node.

If the spec provides canonical messages defining credential types for a few common authentication strategies the CO can build generic authentication flows regardless of which CSI plugin / SP the 'CreateVolume' call gets scheduled to. At least, I hope so.

@cpuguy83
Copy link
Contributor

cpuguy83 commented Aug 17, 2017 via email

@gpaul
Copy link

gpaul commented Aug 17, 2017

I like the direction this is going. It would appear that this way the user could specify a SP for a given application (ie., specify the Any URL) and pass a list of key/value pairs appropriate for the SP where the keys would be username, password or token, etc. and the values would be plain text (username), the name of a secret (password) some blob (token) to be obtained by the CO by sending the user through some SSO flow. Is this what you have in mind?

I would rather not force the user to specify the SP for a given application. I hope to present the user with abstract 'storage profiles' like archive and fast instead of a specific list of SPs. Of course, that indirection comes at the cost of additional complexity.

@gpaul
Copy link

gpaul commented Aug 17, 2017

Perhaps users can be required to enter their SP-specific credentials once (as secrets) for every SP instead of once per action. These could be opaque sets of k/v pairs as specified by the SP authentication documentation for static credentials or the user could select "Share Auth token" for an SP that requires authn via SSO.

It sounds like the CO could build such flows given PR 83.

@saad-ali
Copy link
Member

@gpaul Let's carry on the conversation in #83 (comment) because it is directly related to that PR. I'll reproduce your comments there and add my response.

@quolix
Copy link

quolix commented Sep 6, 2017

I think this issue has now at least two topics that I would argue are orthogonal:

  1. Which system user to bind to the volume mapping to. This user (and potentially its groups) will be later the acting entity on any filesystem API calls and is the subject of file system access control.
  2. SP-specific context needed for establishing the volume mapping.

If we solve 2), we are able to make a file system available on the host and establish a volume mapping, which is when 1) applies.

Originally this issue was only about 1) and I still hope that we can conclude on this topic soon, as I don't really see any design alternatives and a solution is mandatory for being able to do file system access control (as presumably any production setup would like to).

Ultimately CSI is about establishing file system namespace mappings, and on at least both Linux and Windows, file systems are accessed by users and potentially groups, so I'd claim we don't introduce any OS dependencies.

We have have updated our proposal and submitted it as #99. Looking forward to feedback and comments.

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

No branches or pull requests

9 participants