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

have Java clients remove "anon" from disabled algorithms #5947

Merged
merged 3 commits into from
Jan 24, 2019

Conversation

mtbc
Copy link
Member

@mtbc mtbc commented Jan 24, 2019

What this PR does

On initialization has clients remove "anon" from among the value of the jdk.tls.disabledAlgorithms security property.

Testing this PR

Insight should be able to connect to servers even with current Java installed: version 8u201 or later.

bin/omero import ... should work similarly.

Related reading

https://trello.com/c/q7we5yYn/68-insight-ssl-algorithms
#5943 (comment)

@joshmoore
Copy link
Member

joshmoore commented Jan 24, 2019

Only quick thought based on ongoing conversations is whether or not omero.security.anonymous=true should disable the new workaround. (A more complicated version would be omero.security.allow_ciphers=a,b,c) Another option would be to do that outside of ICE_CONFIG and use a specialized environment variable.

@mtbc
Copy link
Member Author

mtbc commented Jan 24, 2019

It's a good idea but for an emergency patch I'd keep things real simple so I'd suggest targeting that for 5.5.

@joshmoore
Copy link
Member

Understood. Someone can always downgrade back to 5.4.9 if this has any issues. Just waiting on the doc push, then let's merge and build.

@sbesson
Copy link
Member

sbesson commented Jan 24, 2019

c3d8ba7 cherry-picked the single commit from #5944 to get at least Travis passing on the newly created dev_5_4 branch, prior to merging this and building a 5.4.10 release candidate

Copy link
Member

@joshmoore joshmoore left a comment

Choose a reason for hiding this comment

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

The same code worked well in insight and the importer, so 👍 for moving it to all Java clients. "Disabling" the workaround will be equivalent to not using 5.4.10 which seems fair enough.

@sbesson
Copy link
Member

sbesson commented Jan 24, 2019

Thanks all. Merging for the creation of first 5.4.10 release candidate.

@sbesson sbesson merged commit cad1883 into ome:dev_5_4 Jan 24, 2019
@mtbc mtbc deleted the client-SSL branch January 24, 2019 12:44
@sbesson sbesson removed the develop label Jan 28, 2019
@sbesson sbesson added this to the 5.4.10 milestone Jan 28, 2019
@chris-allan
Copy link
Member

Just as a bit of feedback after the fact here and with #5949 on the go these changes result in there being a Guava dependency now for clients. While using com.google.common.base.Splitter I'm sure made this implementation a tad easier, it does seem a bit excessive to be pulling in that entire dependency for those purposes.

Anyone who has been using exclude lists to keep their dependency tree in check as an OMERO client will need to take this into account now.

@mtbc
Copy link
Member Author

mtbc commented Mar 5, 2019

@jburel jburel modified the milestones: 5.4.10, 5.5.0 Jun 13, 2019
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