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 TLS encryption support for TSD connections #186

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

broeng
Copy link
Contributor

@broeng broeng commented Dec 1, 2014

  • Enable TLS encryption on TSD connections with command line
    toggle --tls or --ssl. Since OpenTSDB does not support SSL,
    this requires a SSL proxy in front of OpenTSDB, such as
    stunnel or similar
  • Prefers TLS v1.2 if available (since python 2.7.9), uses
    TLS v1 otherwise
  • Add _valid_certificate_name method to SenderThread, for
    verifying certificate name against hostname. Allows use of
    wildcard (*) in subdomains, but not in TLD or HOST parts.
    I.e. *.example.tld allowed
  • Add command line option --ca-certs for specifying the path
    to the system ca-certificates file. Checks existence on
    start up. Defaults to /etc/ssl/certs/ca-certificates.crt
  • Add EXTRA_ARGS option to init scripts, for specifying extra
    options like --tls and --ca-certs

@broeng
Copy link
Contributor Author

broeng commented Dec 8, 2014

By the way, we have been using this in production, since I submitted the PR.

@vasiliyk
Copy link
Member

vasiliyk commented Dec 8, 2014

@tsuna I have tested pull req, it works fine. Please, check command line names, I know you are serious about naming, and I will do the rest. Thanks.

- Enable TLS encryption on TSD connections with command line
  toggle --tls or --ssl. Since OpenTSDB does not support SSL,
  this requires a SSL proxy in front of OpenTSDB, such as
  stunnel or similar

- Prefers TLS v1.2 if available (since python 2.7.9), uses
  TLS v1 otherwise

- Add _valid_certificate_name method to SenderThread, for
  verifying certificate name against hostname. Allows use of
  wildcard (*) in subdomains, but not in TLD or HOST parts.
  I.e. *.example.tld allowed

- Add command line option --ca-certs for specifying the path
  to the system ca-certificates file. Checks existence on
  start up. Defaults to /etc/ssl/certs/ca-certificates.crt

- Add EXTRA_ARGS option to init scripts, for specifying extra
  options like --tls and --ca-certs
@broeng
Copy link
Contributor Author

broeng commented Dec 8, 2014

I just fixed a few spelling errors and incorrect details in the commit message (and PR description). The implementation is still as vasiliyk tested (thanks, btw).

@broeng
Copy link
Contributor Author

broeng commented Nov 6, 2015

It's been almost a year since @vasiliyk tested the PR. Anything needed on the PR for a merge?

@johann8384
Copy link
Member

I'm sorry, let me pull this down and rebase it. I'll give it a test off the recent additions we've made in that area. Hopefully we'll be a little more active in our support of the project! Thank you for your contribution!!

@johann8384 johann8384 added this to the 1.3.1 milestone Feb 16, 2016
@johann8384 johann8384 modified the milestones: 1.3.1, 1.3.2 Mar 16, 2016
@johann8384 johann8384 modified the milestones: 1.3.3, 1.3.2 Jul 6, 2016
@johann8384
Copy link
Member

johann8384 commented Dec 5, 2018

Yep, and now it's been 2 years, sorry @broeng totally not fair. I pulled the PR down and rebased it, I have to make a few tweaks to make sure the linting is still good and that it works in 2.7 through 3.6 but looks like it will be pretty good.

edit: 4 years....

@johann8384 johann8384 self-assigned this Dec 5, 2018
@johann8384
Copy link
Member

I pulled it down and the reason it wasn't merged is that it conflicted with the other TLS work done at the same time. I don't think they are replacements of each other though so I'll need to spend some time sorting it out. The other support was added to allow it to submit data to OVH's metrics platform. https://www.ovh.com/fr/data-platforms/metrics/

@johann8384
Copy link
Member

Sorry this never got merged :-/ It's still on my list of things I should test out and validate! It's only been 6 years...

@johann8384 johann8384 modified the milestones: 1.3.3, 1.3.4 Sep 18, 2020
@vasiliyk
Copy link
Member

vasiliyk commented Apr 26, 2024

Simon, first of all, thank you!
Can you make a fresh PR based on your current version if you use it?

@vasiliyk vasiliyk modified the milestones: 1.3.4, 1.4.0 Apr 27, 2024
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.

3 participants