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

Jump to bottom

Make SimpleSSLContextFactory#configuration public and return unmodifiable map #1085

Closed
sleberknight opened this issue Nov 25, 2023 · 0 comments · Fixed by #1086
Closed

Make SimpleSSLContextFactory#configuration public and return unmodifiable map #1085

sleberknight opened this issue Nov 25, 2023 · 0 comments · Fixed by #1086
Assignees
@sleberknight
Labels
enhancement A request for change or improvement to an existing feature
Milestone
3.2.0

Comments

@sleberknight
Copy link
Member

sleberknight commented Nov 25, 2023 •
edited
Loading

Change SimpleSSLContextFactory#configuration visibility to public. The reason is that I wanted to access this while implementing #1080 from a test in a different package.

After some consideration, making it public adds some value, since it permits inspection of the properties used to generate SSLContext instances. The main caveat, which should be included in the javadoc, is that the returned map contains the
key and trust store passwords.

Of course, in order to even create an instance of SimpleSSLContextFactory , the calling code needs to have that information anyway, so allowing it to be returned here, while not ideal, isn't the end of the world, especially when you consider that callers could use reflection to inspect the internal properties. We could only fix this by changing the design such that SimpleSSLContextFactory does not store properties internally, which would be a complete redesign that we're not interested in doing at present

Plus, callers can choose to use KiwiSecurity directly instead of using SimpleSSLContextFactory, since the latter is just an abstraction over the former.
@sleberknight sleberknight added the enhancement A request for change or improvement to an existing feature label Nov 25, 2023
@sleberknight sleberknight added this to the 3.2.0 milestone Nov 25, 2023
@sleberknight sleberknight self-assigned this Nov 25, 2023
sleberknight added a commit that referenced this issue Nov 25, 2023
@sleberknight
Add disableSniHostCheck property to TLS and SSL configuration classes
c99dc62 
* Add disableSniHostCheck to TlsContextConfiguration,
  SSLContextConfiguration, and SimpleSSLContextFactory
* Update SSLContextConfiguration#toSimpleSSLContextFactory factory
  method to provide disableSniHostCheck to SimpleSSLContextFactory
* Update SSLContextConfiguration#toTlsContextConfiguration factory
  method to provide disableSniHostCheck to TlsContextConfiguration
* Update javadoc of the conversion functions in TlsContextConfiguration
  to explain how disableSniHostCheck is handled (since it does
  not exist in Dropwizard TlsConfiguration)
* Update TlsContextConfiguration#toSslContextConfiguration to
  provide disableSniHostCheck to SSLContextConfiguration
* Add new all-args constructor to SimpleSSLContextFactory
* Clean up duplicative code in SimpleSSLContextFactory by extracting
  several private helper methods
* Change SimpleSSLContextFactory#configuration method to be
  public, and to return unmodifiable map.
* Change tests with lots of assertions to use assertAll
* Minor grammatical fixes in javadocs and comments

Closes #1080
Closes #1085
@sleberknight sleberknight mentioned this issue Nov 25, 2023
Merged
@sleberknight sleberknight added the in progress A task that is actively being worked on label Nov 25, 2023
terezivy pushed a commit that referenced this issue Nov 25, 2023
@sleberknight
Add disableSniHostCheck property to TLS and SSL configuration classes (…
a133c0f 
…#1086)

* Add disableSniHostCheck to TlsContextConfiguration,
  SSLContextConfiguration, and SimpleSSLContextFactory
* Update SSLContextConfiguration#toSimpleSSLContextFactory factory
  method to provide disableSniHostCheck to SimpleSSLContextFactory
* Update SSLContextConfiguration#toTlsContextConfiguration factory
  method to provide disableSniHostCheck to TlsContextConfiguration
* Update javadoc of the conversion functions in TlsContextConfiguration
  to explain how disableSniHostCheck is handled (since it does
  not exist in Dropwizard TlsConfiguration)
* Update TlsContextConfiguration#toSslContextConfiguration to
  provide disableSniHostCheck to SSLContextConfiguration
* Add new all-args constructor to SimpleSSLContextFactory
* Clean up duplicative code in SimpleSSLContextFactory by extracting
  several private helper methods
* Change SimpleSSLContextFactory#configuration method to be
  public, and to return unmodifiable map.
* Change tests with lots of assertions to use assertAll
* Minor grammatical fixes in javadocs and comments

Closes #1080
Closes #1085
@sleberknight sleberknight removed the in progress A task that is actively being worked on label Nov 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sleberknight sleberknight

Labels
enhancement A request for change or improvement to an existing feature
Projects
None yet
Milestone
3.2.0
Development

Successfully merging a pull request may close this issue.

Add disableSniHostCheck property to TLS and SSL configuration classes kiwiproject/kiwi
1 participant
@sleberknight