-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Ubuntu18.04 net5.0 HttpClient SendAsync ignores timeout (via token), hangs for some time and eventually crash the app #45010
Comments
Tagging subscribers to this area: @dotnet/ncl Issue DetailsDescriptionConcrete site with this behavior: https://www.goszakup.gov.kz Example: static async Task Main(string[] args)
{
var sw = new Stopwatch();
sw.Start();
try {
using var cl = new HttpClient();
using var cts = new CancellationTokenSource(7000);
var res = await cl.GetStringAsync("https://www.goszakup.gov.kz", cts.Token);
Console.WriteLine($"[{sw.Elapsed}] Res: {res}");
} catch (Exception e) {
sw.Stop(); //Never here
Console.WriteLine($"[{sw.Elapsed}] Err: {e}.");
}
} Configuration
OpenSSL 1.1.1 11 Sep 2018 Other informationSame behavior is for SendAsync or GetStringAsync.
crash is probably due to StackOverflow. If there is no fix but workaround exists (some global settings (like env) etc) - please share.
|
This is interesting case. I would agree that the cancelation should kick in but it does not. It boils to X509Chain and the way how this server set up. On the get, server presents only it's certificate:
So the X509Chain tries to fetch the CA cert from
that sends redirect to HTTPS. The chain follow and eventually it hits http://root.gov.kz/cert/root_rsa.cer and that gets redirect to https://pki.gov.kz/cert/nca_rsa.cer. X509Chain unfortunately does not take Cancellation token so there is no way to propagate the desire to give up. cc: @bartonjs and @vcsjones if it would make sense to detect loops like this and simply fail. |
I see. That is... interesting. The root certificate is available to download over HTTPS and the root is serving it self. It's rather uncommon for AIA to happen over HTTPS. I don't know how exactly how Windows handles this situation. Failing seems right (especially if that is what Windows is doing.) The intention in the current AIA implementation is to not use HTTPS at all if it is specified in the certificate as HTTPS: Lines 1131 to 1132 in 58667e7
But that doesn't help the situation where HTTP -> HTTPS redirection is happening. So maybe a solution is to disable |
Hm, yeah, we definitely want to block an http -> https redirection. Following a http -> http redirection seems good, but perhaps with a limit of 2 redirects (3 hops). And, we probably want to service this when we have a solution. |
It is possible this used to work as the links point to http:// but perhaps somebody touched the nginx to do wholesale redirects ;( It also seems the X509Chain is based on timeout X509ChainPolicy.UrlRetrievalTimeout but HttpClient is based on Cancellation token so it is hard to stick them together in consistency way. It feels like opportunity for better API... |
I'll give it a shot over the weekend. |
@wfurt so can I set up somehow (through public API) X509ChainPolicy.UrlRetrievalTimeout as temp workaround? And does it take into account redirects of cert downloads? |
unfortunately no @dvitel. This is hidden down in SslStream and there is no public API to manipulate this. |
@wfurt A bit unrelated from the issue, my understanding of the CA is that this is the certificate authority that Kazakhstan has set up to perform mass HTTPS decryption, I don't believe it is expected this certificate chain will work and has been put on a disallow list by many certificate trust stores. https://bugzilla.mozilla.org/show_bug.cgi?id=1232689#c14 provides some additional context. |
Yeah, this seems bad. Is there some reason we don't support CancellationToken in these APIs? Is it just a matter of lack of API surface or is there some underlying issue involved? Which particular API are we talking about here? Edit: It would certainly be nice to detect loops here, but (a) that's not trivial and (b) that doesn't handle a lot of cases, like slow retrieval or weird chains that go on for ever without actually looping. Seems like we should be robust against those scenarios. We don't necessarily need to have perfect cancellation capabilities (though that would be ideal), but we need to do enough to not block on this forever, whatever the underlying cause is. |
I would presume this is because only on Linux do we actually do AIA fetches using HttpClient. On Windows and MacOS the chain building and fetching process is opaque and handled by the OS. I don’t know what or how a CancellationToken would do on MacOS or Windows. |
If there is any chance of asynchronocy, we could at least release control back to caller with partial result. |
Not sure what you mean, but I wouldn't know of a great way do this operation partially. In the case of macOS, Windows' API shape is similar to macOS, with the exception that |
I wrote a failing test for this over at master...vcsjones:45010-no-follow-https. That leaves me with the question of the best way to fix it. Setting Perhaps something new on Alternatively, we could "best attempt" to implement the redirect logic since I think some of it doesn't really apply to AIA, like worrying about a POST -> GET method change. |
Is the reflection only to get hands on the handler? I'm wondering if we can. add some private.internal flag to control the redirection here: runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs Line 126 in d73c65e
instead of duplicating the logic. |
The whole thing is done with reflection due to cyclic dependencies. Could certainly add something that is internal and set with reflection since there is a bunch of reflection going on anyway. Lines 136 to 146 in d73c65e
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsDescriptionConcrete site with this behavior: https://www.goszakup.gov.kz Example: static async Task Main(string[] args)
{
var sw = new Stopwatch();
sw.Start();
try {
using var cl = new HttpClient();
using var cts = new CancellationTokenSource(7000);
var res = await cl.GetStringAsync("https://www.goszakup.gov.kz", cts.Token);
Console.WriteLine($"[{sw.Elapsed}] Res: {res}");
} catch (Exception e) {
sw.Stop(); //Never here
Console.WriteLine($"[{sw.Elapsed}] Err: {e}.");
}
} Configuration
OpenSSL 1.1.1 11 Sep 2018 Other informationSame behavior is for SendAsync or GetStringAsync.
crash is probably due to StackOverflow. If there is no fix but workaround exists (some global settings (like env) etc) - please share.
|
This bug is related to dotnet/announcements#175 |
Description
Concrete site with this behavior: https://www.goszakup.gov.kz
Expected behavior (as tested on Windows) -
The remote certificate is invalid because of errors in the certificate chain: PartialChain
or at least timeout
Example:
Configuration
OpenSSL 1.1.1 11 Sep 2018
Other information
Same behavior is for SendAsync or GetStringAsync.
Played with SocketsHttpHandler Ssl settings - did not help
On run, app hangs around for 30mins on my machine and then crashes with Aborted (core dumped)
The crash dump contains 10K lines of exception call stack - seems that there is a infinite recursion somewhere
here is two cycles of loop from exception:
crash is probably due to StackOverflow. If there is no fix but workaround exists (some global settings (like env) etc) - please share.
The text was updated successfully, but these errors were encountered: