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

Security per Listener #3339

Open
mrocklin opened this issue Dec 23, 2019 · 7 comments
Open

Security per Listener #3339

mrocklin opened this issue Dec 23, 2019 · 7 comments
Labels
discussion Discussing a topic with no specific actions yet

Comments

@mrocklin
Copy link
Member

In #3288 we allowed the scheduler to listen to multiple Listeners at the same time. This allowed for a single scheduler to accept connections from two different networks at the same time, such as a set of local inproc:// workers, and a remote set of tcp:// workers. This seems to have been a relatively painless change, and works smoothly.

However, when we start rolling security into this, we run into issues because we now accept only one security object, which gets uniformly applied to all of our listeners. This is troublesome in the inproc/tls case, and in theory you could imagine having a tls/tls case that would also be unpleasant.

So what is the right way to handle this? We could accept a list of Security objects (including None in that list for no-security) and apply those objects appropriately to each of the listeners. Even if we did that, it becomes unclear which security object we should use when making new connections. For example if we have workers on two different tls:// networks and we're asked to make a connection to tls://alice, which security object should we use?

In this case should we have two ConnectionPools each with its own security and some sort of function to help dispatch between the two? Or should we have a single ConnectionPool with a set of Security objects that it tries, one after the other?

cc @jcrist @sodre @mariusvniekerk

@mrocklin
Copy link
Member Author

It would probably be simple to have a security per protocol

Scheduler(..., security={"tls://": my_security, "inproc://": None})

But I'm not sure how to handle the multiple tls case.

I may also be thinking about this situation wrong. I think that everyone cc'ed on this issue knows more about security than I do.

@sodre
Copy link
Contributor

sodre commented Dec 23, 2019

For the use-case I have in mind, it would be sufficient/necessary to have one security object per listener, just like there is support for multiple ports.

Here is an example of how we could call it:

python -m distributed.cli.dask_spec  --spec \
  '{"cls": "dask.distributed.Scheduler",
    "opts": {"port": [8785, 8786],
                  "protocol": ["tls", "gssapi"],
                  "security": ["distributed.security.TLSSecurity", 
                               "dask_gssapi.security.GSSAPISecurity}}'  

@mrocklin
Copy link
Member Author

When the scheduler makes an outgoing connection which security object should it use?

@sodre
Copy link
Contributor

sodre commented Dec 23, 2019

I see the issue better now. Some additional here, we can assume that "gssapi" is used for talking to clients only, and the "tls" is used for talking to the workers.

@mrocklin
Copy link
Member Author

mrocklin commented Dec 23, 2019 via email

mrocklin added a commit to mrocklin/distributed that referenced this issue Dec 23, 2019
Previously we used the security object to create connection_args and listen_args
This was some unnecessary redundant state and makes it a bit harder to
change things in the future
(see dask#3339)

Now we remove excess calls to connect and instead try to centralize
everything to the `ConnectionPool`.  Hopefully we can isolate changes to
that in the future.
mrocklin added a commit to mrocklin/distributed that referenced this issue Dec 23, 2019
Previously we used the security object to create connection_args and listen_args
This was some unnecessary redundant state and makes it a bit harder to
change things in the future
(see dask#3339)

Now we remove excess calls to connect and instead try to centralize
everything to the `ConnectionPool`.  Hopefully we can isolate changes to
that in the future.
mrocklin added a commit to mrocklin/distributed that referenced this issue Dec 23, 2019
Previously we used the security object to create connection_args and listen_args
This was some unnecessary redundant state and makes it a bit harder to
change things in the future
(see dask#3339)

Now we remove excess calls to connect and instead try to centralize
everything to the `ConnectionPool`.  Hopefully we can isolate changes to
that in the future.
mrocklin added a commit to mrocklin/distributed that referenced this issue Dec 23, 2019
Previously we used the security object to create connection_args and listen_args
This was some unnecessary redundant state and makes it a bit harder to
change things in the future
(see dask#3339)

Now we remove excess calls to connect and instead try to centralize
everything to the `ConnectionPool`.  Hopefully we can isolate changes to
that in the future.
@mrocklin
Copy link
Member Author

Some small cleanup here: #3340

@sodre
Copy link
Contributor

sodre commented Dec 24, 2019

The idea of a SecurityContext is not new and maybe referring to the notes on the python-gssapi SecurityContext implementation might help shed some light.

The key thing I would like to point out is the first sentence in that section:

Security contexts represent active sessions between two different entities.

In dask's terms instead of creating a Security object per role, we would create/define a security context for each pair of comms, client-scheduler, scheduler-worker, scheduler-nanny, worker-worker, etc...

@GenevieveBuckley GenevieveBuckley added the discussion Discussing a topic with no specific actions yet label Oct 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussing a topic with no specific actions yet
Projects
None yet
Development

No branches or pull requests

3 participants