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

Secure communication with RE Manager using 0MQ API #147

Merged
merged 35 commits into from
Apr 7, 2021

Conversation

dmgav
Copy link
Contributor

@dmgav dmgav commented Mar 30, 2021

Implementation of options to enable secure communication between clients and RE Manager via 0MQ channel. Security is enabled by turning on Curve-based encryption, which is built in 0MQ library.

A key pair may be generated by using CLI command qserver-zmq-keys to generate new keys:

$ qserver-zmq-keys
====================================================================================
     ZMQ SECURITY: New public-private key pair.
====================================================================================
 Private key (RE Manager):                 kHAe2@EY5g>:w}OIYl5q.3Iry-8ew0cvr*h<EDWH
 Public key (CLI client or HTTP server):   NUXaxz1LO-h1TI>Bt^JJK9uYDhx&$:QJpjWt:e>k
====================================================================================

or a public key based on an existing private key:

$ qserver-zmq-keys --zmq-private-key 'kHAe2@EY5g>:w}OIYl5q.3Iry-8ew0cvr*h<EDWH'
====================================================================================
     ZMQ SECURITY: Private key generated based on provided private key.
====================================================================================
 Private key (RE Manager):                 kHAe2@EY5g>:w}OIYl5q.3Iry-8ew0cvr*h<EDWH
 Public key (CLI client or HTTP server):   NUXaxz1LO-h1TI>Bt^JJK9uYDhx&$:QJpjWt:e>k
====================================================================================

Once the keys are generated, encryption may be enabled for RE Manager by setting the environment variable:

export QSERVER_ZMQ_PRIVATE_KEY='kHAe2@EY5g>:w}OIYl5q.3Iry-8ew0cvr*h<EDWH'
start-re-manager

Encryption should be also enabled at the client side before client can communicate with RE Manager. All 0MQ API are now accepting the server public key as a parameter: constructors of classes ZMQCommSendAsync and ZMQCommSendThreads and function zmq_single_request accept additional server_public_key parameter. The developer using API is responsible for passing the public key to API classes and functions.

HTTP Server and qserver CLI clients are part of bluesky-queueserver package. Encryption for both clients can be enabled by setting QSERVER_ZMQ_PUBLIC_KEY environment variable before starting HTTP server or running qserver CLI. For example, HTTP Server can be started as

export QSERVER_ZMQ_PUBLIC_KEY='NUXaxz1LO-h1TI>Bt^JJK9uYDhx&$:QJpjWt:e>k'
uvicorn bluesky_queueserver.server.server:app --host localhost --port 60610

and qserver commands may be executed as

export QSERVER_ZMQ_PUBLIC_KEY='NUXaxz1LO-h1TI>Bt^JJK9uYDhx&$:QJpjWt:e>k'
qserver environment open
qserver queue start
qserver environment close

The PR addresses issue #146

@dmgav dmgav changed the title Options for secure communication using 0MQ API Secure communication with RE Manager using 0MQ API Mar 30, 2021
@mrakitin
Copy link
Member

mrakitin commented Apr 6, 2021

Any ideas on how to fix the linter?

Error: Unable to resolve action `psf/black@stable`, unable to find version `stable`

@dmgav
Copy link
Contributor Author

dmgav commented Apr 6, 2021

@danielballan looked at this issue. His opinion was that it will be fixed at certain point and we should just run 'black' manually.

@dmgav
Copy link
Contributor Author

dmgav commented Apr 6, 2021

It happens to all our projects that use 'black'.

@mrakitin
Copy link
Member

mrakitin commented Apr 6, 2021

xref psf/black#2079

@mrakitin
Copy link
Member

mrakitin commented Apr 6, 2021

How about trying this solution meanwhile before the stable branch is reverted?

@dmgav
Copy link
Contributor Author

dmgav commented Apr 6, 2021

I agree with the majority of participants of the discussion at psf/black#2079 who are not planning to do any changes. There are no external contributors to this project yet.

Copy link
Member

@mrakitin mrakitin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of using a native encryption feature of 0MQ. I have concerns about passing the private key via CLI options but otherwise looks good.

bluesky_queueserver/manager/comms.py Outdated Show resolved Hide resolved
bluesky_queueserver/manager/comms.py Show resolved Hide resolved
bluesky_queueserver/manager/comms.py Show resolved Hide resolved
bluesky_queueserver/manager/comms.py Show resolved Hide resolved
bluesky_queueserver/manager/qserver_cli.py Show resolved Hide resolved
bluesky_queueserver/manager/start_manager.py Outdated Show resolved Hide resolved
bluesky_queueserver/manager/start_manager.py Outdated Show resolved Hide resolved
bluesky_queueserver/server/server.py Show resolved Hide resolved
bluesky_queueserver/server/server.py Show resolved Hide resolved
dmgav and others added 2 commits April 6, 2021 16:39
Co-authored-by: Maksim Rakitin <mrakitin@users.noreply.github.com>
@dmgav
Copy link
Contributor Author

dmgav commented Apr 7, 2021

I think I applied all the recent suggestions in the latest commit. They are marked as outdated, but they are applied to the code.

@dmgav
Copy link
Contributor Author

dmgav commented Apr 7, 2021

I also removed command-line options of setting encryption keys as potentially unsafe and also inconvenient. I also removed the description of the command-line options from the PR description.

Copy link
Member

@mrakitin mrakitin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Suggested a few corrections, mostly in docstrings.

bluesky_queueserver/manager/comms.py Outdated Show resolved Hide resolved
bluesky_queueserver/manager/comms.py Outdated Show resolved Hide resolved
bluesky_queueserver/manager/qserver_cli.py Show resolved Hide resolved
bluesky_queueserver/manager/tests/test_qserver_cli.py Outdated Show resolved Hide resolved
bluesky_queueserver/manager/tests/test_zmq_api.py Outdated Show resolved Hide resolved
dmgav and others added 4 commits April 7, 2021 13:19
Co-authored-by: Maksim Rakitin <mrakitin@users.noreply.github.com>
Co-authored-by: Maksim Rakitin <mrakitin@users.noreply.github.com>
Co-authored-by: Maksim Rakitin <mrakitin@users.noreply.github.com>
@dmgav dmgav merged commit 47fe56d into bluesky:main Apr 7, 2021
@dmgav dmgav deleted the encryption branch May 19, 2021 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants