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

Add a note about supporting TLS in the README.md #217

Merged
merged 1 commit into from
Mar 23, 2016
Merged

Conversation

rafiyagi
Copy link

I think it would be helpful for users of this library to know they should utilize an OpenSSL version to one that is using TLS v1.1+ since Recurly will slowly be deprecating any protocols that do not use TLS v1.1.

```

Please ensure that your OpenSSL version supports TLS v1.1 or higher. You can view the change log for OpenSSL here: https://www.openssl.org/news/changelog.html.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That link doesn't work with the . on the end. I pulled it up and I've got no idea how I'd determine which versions support it. Can you dig through there and come up with a list?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@drewish does the markdown file include the period . as part the URL? When I view the branch on github, it properly renders the link without the period. Did you simply copy/past the link with the period?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to account for both people looking at it on GitHub and people looking at it in their editor.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@drewish period removed.

@bhelx
Copy link
Contributor

bhelx commented Mar 23, 2016

It might be good to add the minimum OpenSSL version too so people don't have to dig. I think it's 1.0.1 but I will need to check.

@rafiyagi
Copy link
Author

@bhelx it's 1.0.1g - or at least that's the one where they fixed heartbleed. Should that be the minimum version?

@rafiyagi
Copy link
Author

Ok - so initial TLSv1.1 support starts with v1.0.1.

This was cataloged in "Changes between 1.0.0h and 1.0.1": https://www.openssl.org/news/changelog.html#x21 -- note: it's described in the last bullet point:

Initial TLSv1.1 support. Since TLSv1.1 is very similar to TLS v1.0 only a few changes are required: ...

However, since 1.0.1 also introduced heartbleed, I think we should recommend versions 1.0.1g and up -- 1.0.1g was the version that fixed heartbleed: https://www.openssl.org/news/changelog.html#x14

A missing bounds check in the handling of the TLS heartbeat extension can be used to reveal up to 64k of memory to a connected client or server.

As far as the "best" version, I think it's 1.1.0 since it "prefers" TLS v1.2:
https://www.openssl.org/news/changelog.html#x0

It's a number of bullet points down:

Changes to the DEFAULT cipherlist:
- Prefer (EC)DHE handshakes over plain RSA.
- Prefer AEAD ciphers over legacy ciphers.
- Prefer ECDSA over RSA when both certificates are available.
- Prefer TLSv1.2 ciphers/PRF.
- Remove DSS, SEED, IDEA, CAMELLIA, and AES-CCM from the
default cipherlist.
[Emilia Käsper]

@bhelx @drewish, thoughts?

@rafiyagi
Copy link
Author

Ok, based on the info above, I changed the wording to:

Please ensure that your OpenSSL version supports TLS v1.1 or higher. At a minimum use v1.0.1g, however we recommend v1.1.0 and up.

@drewish
Copy link

drewish commented Mar 23, 2016

👍

@bhelx
Copy link
Contributor

bhelx commented Mar 23, 2016

@rafdizzle86 agreed, thoughtful consideration

@bhelx
Copy link
Contributor

bhelx commented Mar 23, 2016

👍

@bhelx bhelx merged commit ec89653 into master Mar 23, 2016
@drewish drewish deleted the ry-add-tls-readme branch August 5, 2016 20:36
@bhelx bhelx added the V2 V2 Client label Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
V2 V2 Client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants