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

[WFCORE-180] / [WFCORE-182] Allow the supported protocols and cipher suites to be set. #251

Merged
merged 5 commits into from
Oct 28, 2014

Conversation

darranl
Copy link
Contributor

@darranl darranl commented Oct 17, 2014

This change allows the configuration of the cipher suites and protocols for the supplied SSLContext.

This supplied SSLContext is used both for the domain management HTTP server and for Remoting connections (both inbound and outbound).

@dmlloyd
Copy link
Member

dmlloyd commented Oct 17, 2014

Looks good to me iff someone reviews the model stuff and the pull player comes back from its holiday and approves.

@wildfly-ci
Copy link

Windows Build 325 is now running using a merge of 9343313

@wildfly-ci
Copy link

Linux Build 599 is now running using a merge of 9343313

@wildfly-ci
Copy link

Windows Build 325 outcome was SUCCESS using a merge of 9343313
Summary: Tests passed: 2688, ignored: 56 Build time: 0:15:20

@wildfly-ci
Copy link

Linux Build 599 outcome was SUCCESS using a merge of 9343313
Summary: Tests passed: 2688, ignored: 56 Build time: 0:17:41

@darranl
Copy link
Contributor Author

darranl commented Oct 20, 2014

Before merging I have a follow up change to add for the default values.

…abled-protocols and enabled-cipher-suites for the SSL definition within the security realm.
…of the schema and update the parser to support enabled-protocols and enabled-cipher-suites.
… the protocol and cipher suite values are set whenever it is used.
…1.2 for the enabled protocols if no others are specified.
@wildfly-ci
Copy link

Windows Build 327 is now running using a merge of c453dc8

@wildfly-ci
Copy link

Linux Build 602 is now running using a merge of c453dc8

@darranl
Copy link
Contributor Author

darranl commented Oct 20, 2014

Added a minor correction and also set default values for the protocols.

These default values are not backwards compatible but in reality this means that the preferred protocols are automatically used by default which is the desired behaviour, users that really want SSLv3 can still go back and enable it themselves.

Subject to testing and review this is ready to merge.

@wildfly-ci
Copy link

Windows Build 327 outcome was SUCCESS using a merge of c453dc8
Summary: Tests passed: 2688, ignored: 56 Build time: 0:14:04

@wildfly-ci
Copy link

Linux Build 602 outcome was SUCCESS using a merge of c453dc8
Summary: Tests passed: 2688, ignored: 56 Build time: 0:17:12

.build();

public static final StringListAttributeDefinition ENABLED_PROTOCOLS = new StringListAttributeDefinition.Builder(ModelDescriptionConstants.ENABLED_PROTOCOLS)
.setDefaultValue(new ModelNode().add(TLSV1).add(TLSV1_1).add(TLSV1_2))
Copy link
Contributor

Choose a reason for hiding this comment

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

The only issue with this is that when TLS 1.3 comes out and the JVM starts to support it Wildfly will not be able to use it out of the box.

I can't help thinking that a blacklist of insecure protocols is more future proof that a white list of known good ones.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of this is to be moved into Elytron anyway where I do want to fully support a notion of enabled and disabled for both protocols and cipher suites.

kabir added a commit that referenced this pull request Oct 28, 2014
[WFCORE-180] / [WFCORE-182] Allow the supported protocols and cipher suites to be set.
@kabir kabir merged commit 379cd69 into wildfly:master Oct 28, 2014
@darranl darranl deleted the WFCORE-180 branch October 28, 2014 10:29
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