Conversation
Contributor
|
LGTM; Will merge along with other PRs shortly |
EXTERNALCOREWORKS
pushed a commit
to EXTERNALCOREWORKS/winrm4j
that referenced
this pull request
May 9, 2019
…uncycastle (in addition to the common JKS) as the provider. On winrm4j 0.6.1, new changes were introduced by your team: Due to this issue: cloudsoft#80 Commits: cloudsoft#92 cloudsoft#93 Which allowed us to propagate the: * SSLContext (which mostly contains the protocols and as well as the security provider being defined, either JKS or bouncycastle or whatever, vital for FIPS compliance) and the * SSLSocketFactory (which contains the cipher suites to use, vital for FIPS compliance). So when we tried to do it by just passing over both the SSLContext and the SSLSocketFactory, we encountered the following open issue which we had already reported to your team: cloudsoft#97 So in order to fix it: -Based on the SSLSocketFactory (if present), we are just propagating out of it, the cipher suites to the Apache cxf TLSClientParameters class which later on will use such ciphers along with the provider for the secure communication. - Based on the SSLContext, we are passing over the whole SSLContext to the TLSClientParameters, but we are also passing the protocols in the Apache CXF TLSClientParameters, so that the communication can be established successfully and FIPS compliant. There could be better ways to propagate only those parameters (like allowing to pass the ciphers and protocols as new params besides the SSLContext and the SSLSocketFactory), but the fact that how it got implement in 0.6.1 was not stable enough is true, meaning that by incorporating this changes, the client application should only care to propagate both the SSLContext and the SSLSocketFactory and the winrm4j library will use only what's really needed :). Hope you can absorb this changes that will benefit a lot your API, there is no single API similar like yours (like overthere or so) that actually supports FIPS, so this will be a great WIN for your software.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.