Support ZMQ Curve for transport encryption#1515
Support ZMQ Curve for transport encryption#1515krassowski wants to merge 15 commits intoipython:mainfrom
Conversation
| self.log.warning( | ||
| "Kernel is running over TCP without encryption." | ||
| " All communication (including code and outputs) is sent in plain text" | ||
| " and is susceptible to eavesdropping." | ||
| " Use IPC transport or set IPKernelApp.enable_curve=True to enable" | ||
| " CurveZMQ encryption." | ||
| ) |
There was a problem hiding this comment.
nbclient downstream tests are failing due to addition of this warning, see:
E AssertionError: assert '[IPKernelApp] WARNING | Kernel is running over TCP without encryption. All communication (including code and outputs) is sent in plain text and is susceptible to eavesdropping. Use IPC transport or set IPKernelApp.enable_curve=True to enable CurveZMQ encryption.\n[IPKernelApp] WARNING | Kernel is running over TCP without encryption. All communication (including code and outputs) is sent in plain text and is susceptible to eavesdropping. Use IPC transport or set IPKernelApp.enable_curve=True to enable CurveZMQ encryption.' == ''
I believe we should keep it and update nbclient tests, any objections?
There was a problem hiding this comment.
yeah, nbclient tests shouldn't fail if warnings are logged from another package, that's a problem in the test suite
| ).tag(config=True) | ||
|
|
||
| enable_curve = Bool( | ||
| bool(int(os.environ.get("JUPYTER_ENABLE_CURVE", "0"))), |
There was a problem hiding this comment.
I don't like the name it's non obvious, i think it's ok to have "curve" in inner variable names, maybe not in env-var and options. ; should we also trim(), the value of environ.get, or be stricter on it's format ?
There was a problem hiding this comment.
I agree, I think we could hide "curve" here as an implementation detail; an argument for keeping it as-is would be alignment with ipyparallel (ipython/ipyparallel#553).
I think an obvious choice would be enable_transport_encryption?
Same change would need to follow in jupyter-server/jupyter_server#1638.
CC @minrk in case if you have an opinion here
Co-authored-by: M Bussonnier <bussonniermatthias@gmail.com>
| "comm>=0.1.1", | ||
| "traitlets>=5.4.0", | ||
| "jupyter_client>=8.8.0", | ||
| "jupyter_client @ git+https://github.com/krassowski/jupyter_client.git@add-curve-encryption", |
There was a problem hiding this comment.
(needs reverting once jupyter-client PR is merged and released)
| [tool.hatch.metadata] | ||
| allow-direct-references = true |
3 participants
References
Code changes