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

Switch cert and peer validation to false by default #1855

Merged
merged 1 commit into from Dec 12, 2016

Conversation

nickbabcock
Copy link
Contributor

If any either of these two options are enabled, a HTTPs enabled Dropwizard
app will most likely fail to start with cryptic error messages. Since
Jetty has these options disabled by default, we should follow their lead
and disable these options as well.

Future work includes adding in additional certificate features as
Certificate Revocation List (CRL), CRL Distribution Points Protocol
(CRLDP), On-Line Certificate Status Protocol (OCSP)

This may be considered a bugfix and is a candidate for either 1.0.x or 1.1

If any either of these two options are enabled, a HTTPs enabled Dropwizard
app will most likely fail to start with cryptic error messages. Since
Jetty has these options disabled by default, we should follow their lead
and disable these options as well.

Future work includes adding in additional certificate features as
Certificate Revocation List (CRL),  CRL Distribution Points Protocol
(CRLDP), On-Line Certificate Status Protocol (OCSP)
@coveralls
Copy link

Coverage Status

Coverage remained the same at 81.721% when pulling 43371be on nickbabcock:false-cert-validation into 7d9df2f on dropwizard:master.

@evnm
Copy link
Member

evnm commented Dec 11, 2016

Makes sense to me. We should be noisy when releasing this in the next stable versions, given the possibility that it changes behavior out from under folks.

@evnm evnm added the bug label Dec 11, 2016
@evnm evnm modified the milestones: 1.0.6, 1.1.0 Dec 11, 2016
@arteam arteam merged commit 5e78b95 into dropwizard:master Dec 12, 2016
@arteam
Copy link
Member

arteam commented Dec 12, 2016

Makes sense to me as well. Merged to upstream and applied to the 1.0.x branch as 7be0112.

@nickbabcock
Copy link
Contributor Author

Updated release notes in 31068e5 and 94fd90f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants