Skip to content

Conversation

@adietish
Copy link
Member

@adietish adietish commented Oct 1, 2014

No description provided.

@openshift-bot
Copy link

Java Client Action Required: Only pull request(s) from ["master"] are handled by the openshift-bot

@adietish
Copy link
Member Author

adietish commented Oct 2, 2014

we miss a system property (to force DHE cipher exclusion) and tests before we can merge this.

Copy link
Contributor

Choose a reason for hiding this comment

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

this seem to change api - should it not be 2.7.0 then ?

@maxandersen
Copy link
Contributor

added comments.

I would say the system property should be something like .excludeciphers=yes|no|auto and default is auto that will decided based on the bruteforce detection (which should be cached imo)

@adietish
Copy link
Member Author

adietish commented Oct 2, 2014

we are not excluding all ciphers, we're currently just skipping DHE ciphers. Shouldnt we call it .exclude_dhe_ciphers=yes|no|auto

@maxandersen
Copy link
Contributor

is it important that it is dhe ciphers? would we introduce another property for excluding other ciphers?

actually how about call it .filter_bad_ciphers=yes|no|auto

@adietish
Copy link
Member Author

adietish commented Oct 2, 2014

+1 better naming but somehow opaque on what's being removed?
disable_bad_sslciphers=yes|no|auto?

@adietish adietish force-pushed the OSJC-125 branch 3 times, most recently from dd0ac28 to d3ee8ad Compare October 2, 2014 18:39
Copy link
Contributor

Choose a reason for hiding this comment

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

does this really need to be public api ?

@adietish adietish force-pushed the OSJC-125 branch 4 times, most recently from c395e92 to 97012d4 Compare October 2, 2014 22:41
@adietish adietish merged commit 361de63 into openshift:2.6.x Oct 3, 2014
@adietish adietish deleted the OSJC-125 branch October 3, 2014 00:49
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.

3 participants