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

protocol should default to tls if security is set #496

Closed
riedel opened this issue May 11, 2021 · 7 comments · Fixed by #526
Closed

protocol should default to tls if security is set #496

riedel opened this issue May 11, 2021 · 7 comments · Fixed by #526
Labels
all job schedulers enhancement New feature or request

Comments

@riedel
Copy link
Member

riedel commented May 11, 2021

If there is nothing that speaks against that I would file a PR for this.

Should I only do this if encryption is required by the security object?

Another question: does it make sense to add a feature to automatically generate certificates for worker, scheduler and client in the jobque case?

Currently you will get an runtime error:
RuntimeError: encryption required by Dask configuration, refusing communication from/to 'tcp://'

(Actually I then accidentially set tls via the scheduler options which made things worse: the workers silently failed to connect)

@guillaumeeb
Copy link
Member

I've never looked into Security into Dask, but what you propose makes sense, especially if encryption option is set in security object.

Another question: does it make sense to add a feature to automatically generate certificates for worker, scheduler and client in the jobque case?

If it's simple enough yes I guess, I don't know if there are other solutions to obtain certificates when IPs are kind of random, or at least can vary on a high range.

@jacobtomlinson
Copy link
Member

We did this in dask/dask-cloudprovider#222 for the cloud VM based cluster managers.

It took a little work to remove some of the hard coded assumptions we had made around protocol and there was a little effort to handle the certificate generation correctly. But now by default certificates are generated by default and updated into the Dask config dynamically. In dask-cloudprovider we serlialize and ship the in memory config to the workers and scheduler so everything works nicely on that end.

One issue we did run into was there is a limit on AWS on how much config we can pass, and certs fills that up quickly!

@guillaumeeb
Copy link
Member

@riedel, feel free to propose a PR for this!

@riedel
Copy link
Member Author

riedel commented Sep 15, 2021

#519 fixes this, however the PR is not as minimal as I wanted.

@guillaumeeb
Copy link
Member

But now by default certificates are generated by default and updated into the Dask config dynamically. In dask-cloudprovider we serlialize and ship the in memory config to the workers and scheduler so everything works nicely on that end.

@jacobtomlinson if you have time to look at #519, and also explain a bit more the part of serializing and shipping the in memory config to the workers. I think this is the hard part for dak-jobqueue currently, so @riedel used a workaround by writing the in memory security keys to a file... I'm not really fond of this solution.

@riedel
Copy link
Member Author

riedel commented Oct 8, 2021

I commented on why I did this in #520, because one could actually close this issue by only supporting file certificates in the first place. TL;DR: I think using dask.config.set alternatively could have an bigger impact than creating some tempfiles, but maybe I am not completely getting the trick here.

@jacobtomlinson
Copy link
Member

I think only supporting file certificates in dask-jobqueue would be reasonable. In places like dask-cloudprovider we are limited to passing the certs in the Dask config because we can't guarantee there will be a shared filesystem. But for HPC I think most cases there will be one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
all job schedulers enhancement New feature or request
Projects
None yet
4 participants