ome / omero-py

Python project containing Ice remoting code for OMERO
https://www.openmicroscopy.org/omero
GNU General Public License v2.0
22 stars 32 forks source link

cli: don't unset ICE_CONFIG (fix #191) #193

Closed joshmoore closed 4 years ago

joshmoore commented 4 years ago

The unsetting of ICE_CONFIG fails on Windows and may no longer be needed. It was added in 445940543a0469fe0476d0a1d6b5f4846e7349b8 while working on omero shell.

see: https://github.com/ome/omero-py/issues/191

manics commented 4 years ago

Should omero login take the contents of ICE_CONFIG into account (it doesn't seem to)?

edit: It takes some e.g. IceSSL.Trace.Security=1 but not all properties into account

dominikl commented 4 years ago

Looks ok 👍 Didn't notice any change of behaviour on Linux. And on Windows I seem to get one step further now with this PR. Without PR: AttributeError: module 'os' has no attribute 'unsetenv' With PR: ModuleNotFoundError: No module named 'win32con' etc. (whatever this means)

dominikl commented 4 years ago

Noticed that too. With and without PR the ICE_CONFIG doesn't seem to have any effect.

joshmoore commented 4 years ago

Should omero login take the contents of ICE_CONFIG into account (it doesn't seem to)?

I didn't do anything to make that work yet. I assume some processes will make use of it, but that itself could be confusing. Do you guys feel that having it fully integrated is needed at this point? Or more of a follow-up?

It takes some e.g. IceSSL.Trace.Security=1 but not all properties into account

What does? Login?

on Windows I seem to get one step further now with this PR.

:+1:

dominikl commented 4 years ago

Definitely ok for follow-up.

manics commented 4 years ago
$ cat ice.config
IceSSL.Trace.Security=1
omero.host=localhost
omero.pass=password

$ ICE_CONFIG=ice.config omero login
-- 03/16/20 13:35:43.860 Security: enabling SSL ciphersuites:
    ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
    ECDHE_ECDSA_WITH_AES_256_CBC_SHA384
    ECDHE_ECDSA_WITH_AES_256_CBC_SHA
    ECDHE_RSA_WITH_AES_256_GCM_SHA384
    ECDHE_RSA_WITH_AES_256_CBC_SHA384
    ECDHE_RSA_WITH_AES_256_CBC_SHA
    ECDH_ECDSA_WITH_AES_256_GCM_SHA384
    ECDH_ECDSA_WITH_AES_256_CBC_SHA384
    ECDH_ECDSA_WITH_AES_256_CBC_SHA
    ECDH_RSA_WITH_AES_256_GCM_SHA384
    ECDH_RSA_WITH_AES_256_CBC_SHA384
    ECDH_RSA_WITH_AES_256_CBC_SHA
    DHE_RSA_WITH_AES_256_GCM_SHA384
    DHE_RSA_WITH_AES_256_CBC_SHA256
    DHE_RSA_WITH_AES_256_CBC_SHA
    RSA_WITH_AES_256_GCM_SHA384
    RSA_WITH_AES_256_CBC_SHA256
    RSA_WITH_AES_256_CBC_SHA
    TLS_AES_256_GCM_SHA384
    DH_anon_WITH_AES_256_GCM_SHA384
    DH_anon_WITH_AES_256_CBC_SHA256
    DH_anon_WITH_AES_256_CBC_SHA
    PSK_WITH_AES_256_CBC_SHA384
    PSK_WITH_AES_256_CBC_SHA
    DH_anon_WITH_AES_128_GCM_SHA256
    DH_anon_WITH_AES_128_CBC_SHA256
    DH_anon_WITH_AES_128_CBC_SHA
-- 03/16/20 13:35:43.874 Network: SSL summary for outgoing connection
   cipher = DH_anon_WITH_AES_256_CBC_SHA
   protocol = TLS 1.0
   local address = 10.50.24.100:52761
   remote address = 10.0.48.9:4064
Previous session expired for USER on EXAMPLE.openmicroscopy.org:4064
Server: [EXAMPLE.openmicroscopy.org:4064]
Username: [USER]
Password:
-- 03/16/20 13:35:48.540 Security: enabling SSL ciphersuites:
    ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
    ECDHE_ECDSA_WITH_AES_256_CBC_SHA384
    ECDHE_ECDSA_WITH_AES_256_CBC_SHA
    ECDHE_RSA_WITH_AES_256_GCM_SHA384
    ECDHE_RSA_WITH_AES_256_CBC_SHA384
    ECDHE_RSA_WITH_AES_256_CBC_SHA
    ECDH_ECDSA_WITH_AES_256_GCM_SHA384
    ECDH_ECDSA_WITH_AES_256_CBC_SHA384
    ECDH_ECDSA_WITH_AES_256_CBC_SHA
    ECDH_RSA_WITH_AES_256_GCM_SHA384
    ECDH_RSA_WITH_AES_256_CBC_SHA384
    ECDH_RSA_WITH_AES_256_CBC_SHA
    DHE_RSA_WITH_AES_256_GCM_SHA384
    DHE_RSA_WITH_AES_256_CBC_SHA256
    DHE_RSA_WITH_AES_256_CBC_SHA
    RSA_WITH_AES_256_GCM_SHA384
    RSA_WITH_AES_256_CBC_SHA256
    RSA_WITH_AES_256_CBC_SHA
    TLS_AES_256_GCM_SHA384
    DH_anon_WITH_AES_256_GCM_SHA384
    DH_anon_WITH_AES_256_CBC_SHA256
    DH_anon_WITH_AES_256_CBC_SHA
    PSK_WITH_AES_256_CBC_SHA384
    PSK_WITH_AES_256_CBC_SHA
    DH_anon_WITH_AES_128_GCM_SHA256
    DH_anon_WITH_AES_128_CBC_SHA256
    DH_anon_WITH_AES_128_CBC_SHA
-- 03/16/20 13:35:48.554 Network: SSL summary for outgoing connection
   cipher = DH_anon_WITH_AES_256_CBC_SHA
   protocol = TLS 1.0
   local address = 10.50.24.100:52762
   remote address = 10.0.48.9:4064
Created session for USER@EXAMPLE.openmicroscopy.org:4064. Idle timeout: 10 min. Current group: read-only-1
joshmoore commented 4 years ago

This has been in for a while. With more testing happening on Windows, I'd propose getting it in & released and then handling the propagation to login separately.

joshmoore commented 4 years ago

General thumbs up today on getting this in. As there are no other merged PRs that need releasing, I'd propose only releasing this as a dev release at most.