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

If TLS 1.2 is not used, then AuthenticateWith fails without an error #76

Open
infiniteloopltd opened this issue Aug 1, 2019 · 4 comments

Comments

@infiniteloopltd
Copy link

If you use an older version of .NET, and TLS 1.1 is being used, then AuthenticateWith will fail silently, without any error, but nothing will work.

The solution is to use

ServicePointManager.SecurityProtocol = SecurityProtocolType.Tls12;

Perhaps that can be added into the project?

@wilz05
Copy link

wilz05 commented Aug 6, 2019

Try the below in web.config, might solve the problem
<system.web> <compilation targetFramework="4.6"/> <httpRuntime targetFramework="4.6" /> </system.web>

@collinsauve
Copy link
Collaborator

collinsauve commented Aug 7, 2019

Since any changes to ServicePointManager affects all http clients in the process, we should be very careful modifying these settings inside this package's code. What happens if a consumer wants to use TLS v1.3?

At minimum the requirement for TLS >= 1.2 should either throw an exception or be documented though.

@collinsauve
Copy link
Collaborator

For the record here's the announcement from Twitter that they were removing support for TLS < 1.2:

https://twittercommunity.com/t/removing-support-for-legacy-tls-versions-1-0-1-1-on-twitter/126648
https://twitter.com/TwitterAPI/status/1138569964032385025

Starting July 15, 2019, all connections to the Twitter API (and all other Twitter domains) will require TLS 1.2.

@Yortw
Copy link
Owner

Yortw commented Jan 31, 2020

I looked into this. AuthenticateWith doesn't make any calls to Twitter, it only records the token details for when calls are made. That means it has no real idea if the supported protocols are correct or not. I tried assuming anything greater than 1.2 would be ok, but in my testing calling Twitter with only 1.3 enabled also failed, so that logic wasn't correct. Checking specifically for 1.2 isn't very future proof and would break people fixing the protocol version outside the library.

The next best plan I had was to track the error for this when ever a call was made, which ever endpoint was called first. I spent a bunch of time trying to get the exception to bubble all the way back through the library layers, and then realised this is a break change. While I'm not saying TweetSharp never throws exception, the design is such that exceptions from calling the API are not supposed to be re-thrown from the client. Instead they're stored on a property of the Hammock Response object, which is accessible from the TwitterService.Response property. I'm not personally a fan of this plan and wouldn't have designed it this way (but then possibly the original authors also wouldn't now it's 2019 and not 2009). Despite that, I'm loathe to implement such a breaking change given I don't know who's using the library and if 'silent' failures might be better than introducing unhandled errors to their code paths. Another little problem is the .Net error that occurs when the TLS protocol wrong is wrong is a bit vague, it just tells you the remote end killed the connection, so we have to assume the reason is the protocol isn't right.

Given that, I'm not sure what a good plan is in terms of changing the code. I never finished the exception throwing and I don't think it's a good idea at this point, even if it was it would need to be a new major release.

My best advice to clients would be to make a test call after AuthenticateWith, perhaps to get the user profile or retrieve the Twitter configuration data, and then check the response property on the TwitterService instance used. If that contains the relevant error, then you know it's a protocol problem. Shame the library can't automatically do that for you, but not too onerous.

In the meantime, updating the documentation to explain this problem and the solution is probably a good idea but I haven't got around to that either.

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

No branches or pull requests

4 participants