search

ring-clojure / ring

Clojure HTTP server abstraction
MIT License
3.77k stars 520 forks source link

feat: allow overriding cipher exclusions #405

Closed MIJOTHY closed 4 years ago

MIJOTHY commented 4 years ago

jetty maintains an internal default set of cipher suites and protocols to exclude. There are use-cases for completely replacing this set rather than adding to it. In particular, IBM JVM implementations prefix their cipher suites with SSL_, and these are excluded by default by jetty's internal list.

a91d874 added the ability to extend the list of excluded cipher suites and protocols by implementing :exclude-{ciphers,protocols}, which provide an interface to the underlying SslContextFactory.addExclude{CipherSuites,Protocols} methods. However, this operation is purely additive. In order to replace the list of excluded cipher suites, SslContextFactory.setExcludeCipherSuites needs to be called. This commit adds this functionality.

Resolves: #404

weavejester commented 4 years ago

Thanks. Can you ensure the commit adheres to the contributing guidelines?

MIJOTHY commented 4 years ago

Thanks. Can you ensure the commit adheres to the contributing guidelines?

I take it this was referring to the commit subject line? If so, updated

weavejester commented 4 years ago

Thanks! Could you also ensure that the line length is udner 80 characters in the code, and can you also remove the markdown formatting in the commit message. Commit messages are plaintext, rather than markdown.

weavejester commented 4 years ago

Thanks. Can you capitalize "jetty" in the commit message?

I'm also wondering if it would be better to have a boolean :replace-exclude-ciphers? and :replace-exclude-protocols?, which if set would use set... rather than add.... What do you think?

MIJOTHY commented 4 years ago

Thanks. Can you capitalize "jetty" in the commit message?

Sure

I'm also wondering if it would be better to have a boolean :replace-exclude-ciphers? and :replace-exclude-protocols?, which if set would use set... rather than add.... What do you think?

I'm not opposed to it, it certainly seems more intuitive given it doesn't make sense to call both add and set. My main motivation for doing the way I have it that is it keeps the distance between the Jetty implementation and the adapter interface relatively small, but given the existing differences anyway, I think your suggestion makes for a nicer UX.