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

Use OpenSSL DTLS_method & TLS_server_method when available #1832

Merged
merged 4 commits into from
Jul 8, 2019

Conversation

MSP-Greg
Copy link
Member

@MSP-Greg MSP-Greg commented Jul 1, 2019

2nd commit changes a require in Rakefile to require_relative...

@MSP-Greg
Copy link
Member Author

MSP-Greg commented Jul 2, 2019

This PR replaces some of the code in #1788 & #1799.

Sometime soon, I'll review the code in them and generate a new PR.

@MSP-Greg MSP-Greg changed the title Add extconf test for DTLS_method & use in mini_ssl.c Use OpenSSL DTLS_method & TLS_server_method when available Jul 7, 2019
@nateberkopec
Copy link
Member

  • Needs a changelog entry
  • I'm going to say the trusty CI build covers this testing-wise

@MSP-Greg
Copy link
Member Author

MSP-Greg commented Jul 8, 2019

Needs a changelog entry

Done.

I'm going to say the trusty CI build covers this testing-wise

Agreed. Fixes issue #1833

@nateberkopec nateberkopec merged commit 1814008 into puma:master Jul 8, 2019
@MSP-Greg MSP-Greg deleted the dtls-check branch July 8, 2019 21:50
@MSP-Greg
Copy link
Member Author

MSP-Greg commented Jul 8, 2019

@nateberkopec

Thanks. Since there's some overlap (my mistake), I'll now combine #1788 & #1799 with another PR. Hopefully, SSL tests will then pass for any OpenSSL build/configuration and be a good indicator of functionality.

I agree that a MiniSSL::Context#min_version= method/attr is best left for later...

@MSP-Greg
Copy link
Member Author

MSP-Greg commented Jul 9, 2019

@nateberkopec, et al

I saw the Friday release mention.

Question: Don't know future plans re releases, but I think one thing people might want is to also have a no_tlsv1_1= method to lock Puma to TLSv1.2 or higher.

Make it so #no_tlsv1_1 = true overrides #no_tlsv1 = false.

Ideally, MiniSSL::Context#min_version= is probably a better solution, but given that new TLS versions are really uncommon, having two 'no_' methods isn't cumbersome. Maybe when TLSv1.4 or TLSv2.0 is released five years from now...

Thoughts? Mention because it is much easier than MiniSSL::Context#min_version=...

@nateberkopec
Copy link
Member

I think one thing people might want is to also have a no_tlsv1_1= method to lock Puma to TLSv1.2 or higher.

That's fine in the near-term I think. Probably for 4.1.

@MSP-Greg
Copy link
Member Author

MSP-Greg commented Jul 9, 2019

Ok, so not for the next release?

It's relatively easy to add, so I could add it when I combine #1788 & #1799

IOW, generate the new PR (and include no_tlsv1_1=) within the next few days...

@nateberkopec
Copy link
Member

not for the next release?

Correct. I'd like to keep new methods out of 4.0.1.

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

Successfully merging this pull request may close these issues.

None yet

2 participants