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

Port to Python cryptography module #27

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

chris-allan
Copy link
Member

Ports the plugin to use the Python cryptography module rather than calling out to the OpenSSL command line tools which can be error prone, is not cross platform, and for which error conditions are hard to control for.

Note that this commit also changes the format of
`omero.certificates.owner` from the OpenSSL command line `/` separated
format to RFC 4514 (which supercedes RFC 2253) `,` separated format.  As
this plugin saves the owner in OMERO server configuration the upgrade
user experience will have to be considered in a follow up commit.
Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

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

Two immediate comments from my initial testing in a OSX 12 / Python 3.8 (server) / OS X 12 / Python 3.10 (client) context. I'll expand my testing on Linux environments.

omero_certificates/certificates.py Show resolved Hide resolved
omero_certificates/certificates.py Outdated Show resolved Hide resolved
@sbesson
Copy link
Member

sbesson commented Oct 18, 2022

Successfully tested in an Ubuntu 20.04 environment with a deployed OMERO.server.

After stopping the server, moving the existing certs and installing this new version of the omero-certificates plugin

(OMERO.venv) root@omero-certificates:/opt/omero# pip install git+https://github.com/ome/omero-certificates@refs/pull/27/merge
...
Successfully built omero-certificates
Installing collected packages: omero-certificates
  Attempting uninstall: omero-certificates
    Found existing installation: omero-certificates 0.2.0
    Uninstalling omero-certificates-0.2.0:
      Successfully uninstalled omero-certificates-0.2.0
Successfully installed omero-certificates-0.2.1.dev59+ga697857
(OMERO.venv) root@omero-certificates:/opt/omero# exit
(OMERO.venv) omero@omero-certificates:~$ pip freeze | grep omero
omero-certificates @ git+https://github.com/ome/omero-certificates@a697857f61bd090dcce591e6beaf8b0cacdc528e
omero-figure==4.4.3
omero-marshal==0.7.0
omero-parade==0.2.3
omero-plus==0.2.2
omero-py==5.12.1
omero-web==5.13.0
(OMERO.venv) omero@omero-certificates:~$ omero certificates -v
INFO:omero_certificates.certificates:Using existing key: /OMERO/certs/server.key
INFO:omero_certificates.certificates:Creating self-signed certificate: /OMERO/certs/server.pem
WARNING:omero_certificates.certificates:'omero.certificates.owner' configuration setting '/L=OMERO/O=OMERO.server' not a valid RFC 4514 string!  Attempting to convert.
INFO:omero_certificates.certificates:Creating PKCS12 bundle: /OMERO/certs/server.p12
certificates created: /OMERO/certs/server.pem /OMERO/certs/server.p12
(OMERO.venv) omero@omero-certificates:~$ sudo service omero-server start

Both local connections and imports worked as expected

(OMERO.venv) omero@omero-certificates:~$ omero login root@localhost 
Previously logged in to localhost:4064 as root
Password:
Created session for root@localhost:4064. Idle timeout: 10 min. Current group: system
(OMERO.venv) omero@omero-certificates:~$ omero import test.fake --errs test.err
Using session for root@localhost:4064. Idle timeout: 10 min. Current group: system
Image:1902

@jburel
Copy link
Member

jburel commented Oct 20, 2022

I have now modified the OMERO-test-integration job on merge-cito install this PR instead of the release version of omero-certificates.

@jburel
Copy link
Member

jburel commented Oct 20, 2022

ome/omero-install#269 tests the installation and import of image on Ubuntu 20.04, Debian 10 and Centos7 with scl Python 3.8

@jburel
Copy link
Member

jburel commented Oct 21, 2022

@jburel
Copy link
Member

jburel commented Oct 21, 2022

List of actions derived from this change

README.md Outdated
For full information see the output of:
## Upgrading

Since version 0.3.0 this plugin uses portable RFC 4514 (supercedes RFC 2253) formatted strings for the `omero.certificates.owner` configuration option. If you have ran `omero certificates` before you may have OpenSSL command line formatted strings in your configuration that should be updated before you can run `omero certificates` again. In most cases this means taking a string such as `/L=OMERO/O=OMERO.server` and reformatting it to `L=OMERO,O=OMERO.server`; remove the leading `/` and replace separator `/`'s with `,`'s.
Copy link
Member

Choose a reason for hiding this comment

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

Quick note before I need to head out: there is a place in omero-py for upgrading properties in case we want to make use of it. (And this is a really long line...)

@jburel
Copy link
Member

jburel commented Nov 22, 2022

This PR has been included in daily build for several weeks.
Proposing to merge and tag as 0.3.0
The installation doc should be updated accordingly

@sbesson
Copy link
Member

sbesson commented Nov 22, 2022

As briefly mentioned at the weekly meeting today, my main caveat is that the testing so far has taken place on several operating systems/environment but has been restricted to client/server connections within the server environment itself.

I will work on cross-environment testing and report in the upcoming days.

setup.py Outdated Show resolved Hide resolved
Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

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

Set-up two servers with different versions of Python/OpenSSL with omero-certificates including this PR:

Ubuntu 20.04 Ubuntu 22.04
Python 3.8.10 3.10.6
Java 11.0.17 11.0.17
OpenSSL 1.1.1 3.0.215
omero-py 5.13.1 5.13.1

Tested the import against both servers using the same systems as well as a CentOS 7 as client. All connections worked as expected.

Client / server Ubuntu 20.04 Ubuntu 22.04
Ubuntu 20.04 pass pass
Ubuntu 22.04 pass pass
CentOS 7 pass pass

The CentOS 7 / Python 3.6 / OpenSSL 1.0.x client/server should have been tested by the latest builds of the OME CI infrastructure. Deferring to @jburel on whether we want to some cross-environment testing using this environment.

The only remaining issue that was highlighted from my testing is the minimum cryptography requirement. Importantly, this came up when running an upgrade against a deployed system with an older version of cryptography so there is probably a case for testing the upgrade scenario if we cut a new release of omero-certificates.

@sbesson
Copy link
Member

sbesson commented Mar 21, 2023

@jburel with OMERO.server 5.6.7 behind us, I propose we come back to this. What are the next steps to get this released as 0.3.0?

@jburel
Copy link
Member

jburel commented Mar 29, 2023

Several warning in the merge-ci build

/home/omero/workspace/OMERO-test-integration/.venv3/lib64/python3.6/site-packages/omero_certificates/certificates.py:11: CryptographyDeprecationWarning: Python 3.6 is no longer supported by the Python core team. Therefore, support for it is deprecated in cryptography. The next release of cryptography will remove support for Python 3.6.
02:17:28   from cryptography import x509

@sbesson
Copy link
Member

sbesson commented Mar 29, 2023

This warning communicates the upstream decide to drop Python 3.6 support with the current 40.x series being the last one where this version is supported.
From my side, this is inline with OME's intent to set Python 3.8+ as the recommended minimum Python version for OMERO deployments although I understand the documentation and the CI infrastructure still need to be adjusted accordingly. Independently, Python 3.6 deployments should still be able to install and use the last compatible release of cryptography.

@imagesc-bot
Copy link

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/ezomero-on-windows/73163/12

@sbesson
Copy link
Member

sbesson commented Jun 9, 2023

Coming back to this as the underlying migration of OME CI infrastructure is getting upgraded to Python 3.8+. This means the concerns echoed in #27 (comment) should be lifted soon. Are there any other outstanding blockers to moving forward with this migration?

Update (2023-06-16): from a discussion with @chris-allan, the current proposal would be to:
1- hold off on this PR for now
2- develop and release a version omero-certificates using openssl generating SSL certificates fixing the Cipher issues seen when newer openssl systems are connecting to CentOS 7/RHEL7
3- once support for CentOS 7/ RHEL7 has been removed, switch to pure Python cryptography as per this PR and remove the OpenSSL 1.0.x workarounds

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.

5 participants