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.

Sign up for GitHub

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

Update getSupportedSSLParameters() #215

Closed
wants to merge 1 commit into from
Closed

Update getSupportedSSLParameters() #215

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/java/com/wolfssl/provider/jsse/WolfSSLContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,16 @@ protected SSLParameters engineGetSupportedSSLParameters() {
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
"entered engineGetSupportedSSLParameters()");

return WolfSSLParametersHelper.decoupleParams(this.params);
SSLParameters supportedParams =
WolfSSLParametersHelper.decoupleParams(this.params);

/* set cipher suites and protocols to all supported cipher suites and
protocols */
supportedParams.setCipherSuites(WolfSSLUtil.sanitizeSuites(
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this cipher suite and protocol list already set when the WolfSSLContext is created, inside createCtx()? Around these lines:

161         /* Auto-populate enabled ciphersuites with supported ones. If suites
162          * have been restricted with wolfjsse.enabledCipherSuites system
163          * security property, the suite list will be filtered in
164          * WolfSSLEngineHelper.sanitizeSuites() to adhere to any
165          * set restrictions */
166         if (WolfSSLUtil.isSecurityPropertyStringSet(
167             "wolfjsse.enabledCipherSuites")) {
168             /* User is overriding cipher suites, set CTX list */
169             this.setCtxCiphers(WolfSSLUtil.sanitizeSuites(ciphersIana));
170         }
171         params.setCipherSuites(WolfSSLUtil.sanitizeSuites(ciphersIana));
172
173         /* Auto-populate enabled protocols with supported ones. Protocols
174          * which have been disabled via system property get filtered in
175          * WolfSSLEngineHelper.sanitizeProtocols() */
176         params.setProtocols(WolfSSLUtil.sanitizeProtocols(
177             this.getProtocolsMask(ctxAttr.noOptions)));

I would think since that is set into the object-level "params", that would also come through here when we call decoupleParams(), no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like at line 76 ciphersIana is set to WolfSSL.getCiphersAvailableIana(), returning the list of "enabled ciphers":

        /* Get available wolfSSL cipher suites in IANA format */
        ciphersIana = WolfSSL.getCiphersAvailableIana(this.currentVersion);

Then, at line 81, ciphersIana is passed into WolfSSLCustomUser and is set as the list variable:

        WolfSSLCustomUser ctxAttr = WolfSSLCustomUser.GetCtxAttributes
                          (this.currentVersion, ciphersIana);

At line 149, because the length of ctxAttr.list is greater than 0, ciphersIana is set to WolfSSL.getCiphersAvailableIana(this.currentVersion) from above:

        if(ctxAttr.list != null && ctxAttr.list.length > 0) {
            ciphersIana = ctxAttr.list;
        } else {
            ciphersIana = WolfSSL.getCiphersIana();
        }

Last, at line 167, the SSLParameters cipher suites are set to ciphersIana:
params.setCipherSuites(WolfSSLUtil.sanitizeSuites(ciphersIana));
I think that this is why calling getSupportedSSLParameters() then returns an SSLParameters object with only the list of "enabled" cipher suites and why the cipher list needed to be set again in getSupportedSSLParameters() in order to give the full list. Given all that, do you think something else should be changed in order to return the full list of ciphers in getSupportedSSLParameters()?
It does seem like setting the protocols again in supportedParams like I have is redundant though and I can delete that part.

Copy link
Member

Choose a reason for hiding this comment

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

Looking at the Javadocs for SSLContext.getSupportedSSLParameters(), it says:

Returns a copy of the SSLParameters indicating the supported settings for this SSL context.

My interpretation of "supported settings for this SSL context" means only cipher suites that are supported by this SSLContext object, which has been created with a specific TLS protocol version. Not all cipher suites compiled into native wolfSSL are supported by all protocol versions. For example, TLS 1.3 cipher suites are not supported in a TLS 1.2 SSLContext.

WolfSSL.getCiphersAvailableIana() returns the cipher suite list "supported" by the protocol version given to it. WolfSSL.getCiphersIana() returns the list of ALL cipher suites enabled at compile time in native wolfSSL. We want to return a filtered version of WolfSSL.getCiphersAvailableIana() as the "supported" cipher suites by this SSLContext.

More specifically, "supported" cipher suites start with the value of WolfSSL.getCiphersAvailableIana() but then that list may be filtered / restricted down to a subset by the following steps:

  1. We get full "supported" list of cipher suites from wolfSSL for the protocol version our SSLContext was created from (WolfSSL.getCiphersAvailableIana()).
  2. Some customers want to force restrict that list by hard-coding changes inside the WolfSSLCustomUser class. By default, we pass the list from step 1 into our default implementation of WolfSSLCustomUser. It does nothing to the list and returns it as the list to use.
  3. Users may further restrict the supported/enabled cipher suites by setting the wolfjsse.enabledCipherSuites Security property in their java.security file.
  4. We finally set the resulting cipher suite list into the SSLParameters:
171         params.setCipherSuites(WolfSSLUtil.sanitizeSuites(ciphersIana));

All that to say, I think we are setting the SSLParameters cipher suites to the correct "supported" list for a SSLContext object. And, the params we return from getSupportedSSLParameters() should be accurate already.

Let me know if you think this is still incorrect, thanks.

WolfSSL.getCiphersIana()));
supportedParams.setProtocols(WolfSSLUtil.sanitizeProtocols(
WolfSSL.getProtocols()));
return supportedParams;
}

/**
Expand Down