search

mixpanel / mixpanel-java

Other
49 stars 37 forks source link

fix: Use TLS for all API calls #26

Closed JensRantil closed 8 years ago

JensRantil commented 9 years ago

Based [1] it looks like TLS can be used interchangably against unencrypted connections. I have sent a support e-mail to MixPanel to verify this, but I'm pretty sure this is the case.

This commit fixes #18.

[1] https://mixpanel.com/help/reference/http

JensRantil commented 9 years ago

I have sent a support e-mail to MixPanel to verify this, but I'm pretty sure this is the case.

I'll make sure to post the MixPanel support response here as soon as it arrives.

JensRantil commented 9 years ago

Looks like the iOS SDK is only using https://api.mixpanel.com (while the Android supports some kind of fallback to non-TLS). I therefor conclude that TLS can be used interchangably for the API.

JensRantil commented 9 years ago

I'll make sure to post the MixPanel support response here as soon as it arrives.

Got a response:

Hi Jens,

Maddy here, from Mixpanel Support.

We officially support both HTTP and HTTPS requests to the api.mixpanel.com endpoint. As a note, while we support TLS, we recently disabled SSLv3 to protect against POODLE attacks. I would be happy to further chat about this change if you are interested.

I apologize that this is not made more explicit in our HTTP spec. I have already gone ahead and synced with our knowledge share/docs team to improve documentation here. Thank you for the valuable feedback!

Please do let me know if there is anything else that I can help you with.

Best,

Maddy

In other words, I see no reason to not merge this pull request.

ryanseams commented 9 years ago

@yinfeiru mind taking a look at the PRs @JensRantil has been so kind to provide? there are a few around connection type, timeout, etc. -- thanks in advance!

JensRantil commented 8 years ago

Hey, how is it going with review of this? This a rather apparent security related issue. If you took privacy seriously, you should have merged and published a new Maven artifact within at least a week of submitting this. This has been laying dormant for the past 2 months. Shame on you.

Suhail commented 8 years ago

paging @jbwyme

JensRantil commented 8 years ago

Two months have now gone by..... :clap: :clap: :clap: :laughing:

patedit commented 8 years ago

Thanks @JensRantil !

JensRantil commented 8 years ago

Thanks for merging! Next time, I think you should respond faster to security related pull requests like this. It's a bit embarrassing for you that it's taken this long...