-
Notifications
You must be signed in to change notification settings - Fork 301
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
Port support urgently needed (e.g. 8443) #424
Comments
In other scenarios, things I've written myself. I simply opt-in to TLS 1.2 prior to making a request. Also not sure how that works when using this library. For example, prior to requests, I would do this:
I'm using v5.6.2 of this library. Does it gracefully attempt 1.2?
Or no?
Update: I think .NET 4.6 uses Tls 1.2 by default. Looking forward to updates on this thread. |
The SecurityProtcol setting is not done in the library and should not be, because that would affect connections for all target hosts in the application. Yes, you can do it outside the library, but the library will hit port 443, not port 8443. Port 443 allows other forms of connections aside from Tls 1.2, but port 8443 allows only TLS 1.2. So without the ability to override the library URL, there is not an obvious and conclusive way of testing TLS 1.2 using the library before it goes live on June 20. |
And to add: .Net 4.6 targeted apps will indeed work without coding changes as long as the OS the app is running in supports TLS 1.2 (Server 2008 SP2, Server 2008 R2, later server OSes, etc). But who wants to be forced to move to 4.6 just to deal with this? |
@lellis1936 Good point, you're right. It should not. I suppose I didn't think about it because my paranoia about my application breaking destroyed common sense. |
@lellis1936 you can use the example code provided here to test using the library's underlying HTTP client on your current setup: |
imthepitts/ModestGenius, yes, I have done so but a conclusive test in my mind is one that uses the library methods. The provided test does not. |
@imthepitts I just did this as well, but I have to opt in to TLS 1.2 explicitly, as my application is on .net 4.5. I suppose I need to upgrade. |
ModestGenius: or make the coding change. Here's the one we're evaluating: ServicePointManager.SecurityProtocol = This works for me back to .Net 3.5. But I cannot test this with the library! |
@lellis1936 Right, this was my first example and I've done this in other applications. I still suppose I should upgrade. I've been putting it off for a while. What version of framework are you on? I have 1.2 in the enum. Wouldn't it also be better to climb the tree the other way?
|
One app is 4.5.1, another is 3.5. The sequence the protocols are listed is not relevant. Of course it is better to use official enum values if they are present, but they are not in base 3.5 |
@lellis1936 Well, I think I'll opt into 1.2 in my endpoints and deal with upgrading later. This should solve the problem, although yes, it would be nice to simulate this in a staging environment with 8443 on same code used in production. |
Yes the code provided has "fixed" our 3.5 app, but that app does not use the library. Would still like an answer from someone if it is possible to test 8443 using library methods. For example, would using the proxy capability in the library work? Not ideal, a URI or port override would be preferable. |
As far as the sample code we provide for testing port 8443 is concerned, I understand your apprehension as a solid end-to-end test is always better. We believe that since TLS connectivity is negotiated well before any other methods are executed, it truly is sufficient just to successfully negotiate the connection. That said, there may be a way to provide the confidence you'd like. The most thorough method would be to provide a custom HTTP Client which just wraps the existing one with the only change being to use port 8443. I've not tested it in our C# library, but some Java library users have used just the Request object itself (which, I grant, isn't using the library code paths) to test things by cleverly adding the port into the URL:
I hope one of these solutions works for you as we continue to work to improve the flexibly of our helper libraries. |
@jmctwilio What about the debug console or log history of requests. Is it possible that you could include connection information in this so we can confirm the request was made with Tls1.2? That way I could just run my stating site, send a test message, and confirm, without changing the port. |
MG: I don't know how to obtain that information at the app level, moreover if there is a way it would have to be done in the library I imagine (if using the library). I guess WireShark might work... jcmtwilio thanks for your response and the code but I suspect I am as comfortable as I can be without using actual library code paths. Too bad there's not a way, TLS 1.2-only is a serious change and I'll bet a lot of customers are unknowingly NOT ready for this. I would have preferred much more notice of this, a way to continue using earlier TLS if I wanted, or both. |
@lellis1936 My fault, my reply was to @jmctwilio but I typed your name by mistake. I'm referring to signing into Twilio and looking at the request history. You can do that now and see the logs. I was thinking it would be nice if the connection details were there on the requests so we could just plainly see, which would satisfy end-to-end testing. |
@ModestGenius |
I'm also interested in this. It would be nice if we could get a one-off build of the 4.7.2 version lib where the base url is |
@awwong1 just clone the repo and try it... ;) |
Hello @lellis1936, It appears that this issue should be resolved based on the checks on PR 492. If not, please let us know and we will investigate further. Thanks! With best regards, Elmer |
No longer an issue for me. Thanks. |
Within a month, Twilio will withdraw support for TLS versions < 1.2
Client applications which use this library cannot adequately test this transition using the new temporary API port 8443 because the library does not allow the API URI to be overridden.
Perhaps there is a workaround for testing with the library but not one that is obvious to me.
The text was updated successfully, but these errors were encountered: