Workaround for incompatibility with HTTPS Upgrade

[ss3git]

-- The description below is obsolete; on_ssl_bump_error directive is to be changed to on_error. --

Background

• Recent Chromium browsers introduced the HTTPS Upgrade (Automatic HTTPS) function by default:
  https://blog.chromium.org/2023/08/towards-https-by-default.html

• HTTPS Upgrade:
  The browsers (may) first try to connect to HTTPS port even if the user directed to connect to HTTP port.

Issue

• If an accessing HTTP site has both HTTP/HTTPS ports opened while an inappropriate certificate is set to the HTTPS port, the browsers with HTTPS Upgrade function first try to connect to HTTPS port and fall back to the HTTP port if directly connecting to the site, however, the browsers only show the ERR_SECURE_CONNECT_FAIL page if connecting to the site via Squid with SSL Bump.

• A similar result (ERR_CONNECT_FAIL) may occur for HTTP sites whose HTTPS port is not open.

• ERR_ZERO_SIZE_OBJECT is another known case where the origin HTTPS port does not reply anything after successful CONNECT.

This PR

• introduces an option by on_ssl_bump_error directive (maybe as a transitory measure until the fallback algorithm of the browsers will be more sophisticated or standardized).

• With the option set for ERR_CONNECT_FAIL and ERR_SECURE_CONNECT_FAIL, if CONNECT (including certificate validation) to a site failed, Squid will terminate (or reset) the client session without sending any header and body. This enables the affected browsers to fall back to HTTP as they do so for direct connection, instead of just showing the error page generated by Squid.

  • To be precise, Squid returns the fake "HTTP/1.1 200" to the client for CONNECT to affected sites even if this option is set. The option differentiates the response after the first HTTP request from the client is received.

  • Similarly, if the option is set for ERR_ZERO_SIZE_OBJECT, Squid will terminate the client session when it does not receive any data from the connected origin port.

Note

• Chrome has a setting option "Always use secure connections" and turning it on is required to reproduce this issue. In other words, turning it off is an alternative workaround at client side.

• Chromium Edge has a slimilar switch but needs a registry modification: HKEY_LOCAL_MACHINE\SOFTWARE\Policies\Microsoft\Edge
  HttpsUpgradesEnabled

  • Some web sources suggest that edge://flags/#edge-automatic-https is the one for this issue, however, it does not actually control the behavior as of 121.0.2277.112.

Miscellaneous references

• https://groups.google.com/a/chromium.org/g/blink-dev/c/cAS525en8XE
• https://issues.chromium.org/issues/40248833
• HTTPS Upgrades w3ctag/design-reviews#853 (comment)

[squid-prbot]

Can one of the admins verify this patch?

[yadij]

FTR;

1. Squid current behaviour of doing a TLS "success" and presenting 5xx HTTP error page - is to avoid bugs in Chrome and Firefox handling of non-HTTP/1.1 200 OK responses to CONNECT tunnel attempts.

2. The behaviour you are suggesting we add is in a way the correct behaviour for TLS issues of the type that will correctly trigger the Chrome auto-HTTPS behaviour, but also wrong for many other TLS issues.

So, to do this right without breaking a lot of traffic beyond Chrome we need to be a lot more specific about which action is taken and when. Checking details from whether we have a CONNECT (by which User-Agent, with which Upgrade: negotiation, ...), to what TLS error occured (its TLS alert type, by which downstream hop generated it, ...), and whether we have alternate destinations remaining to try.

[rousskov]

Thank you for working on addressing this annoying problem!

Amos: we need to be a lot more specific about which action is taken and when. Checking details from whether we have a CONNECT (by which User-Agent, with which Upgrade: negotiation, ...), to what TLS error occured (its TLS alert type, by which downstream hop generated it, ...), and whether we have alternate destinations remaining to try.

I agree that a different fix is needed. I also agree that bypassing the "retry" part of retryOrBail() logic is wrong. I disagree that Squid should (automatically) check User-Agent, Upgrade, TLS error, etc. to (automatically) enable the desired behavior.

Instead, I suggest adding an on_ssl_bump_error directive, similar to the existing on_unsupported_protocol directive. The directive should tell Squid whether to close the connection, respond with a CONNECT error, or bump and respond with a CONNECT error (at least). The initial implementation does not need to support all these actions if just closing connections (without bumping the client and without sending an error response) is enough to support "try HTTPS first" problem this PR focuses on. Two actions (default and new) would be enough.

Squid ACLs can be used to make the choice conditional. If there are not enough ACL types, new ones can be added, of course.

If the problem is not specific to SslBump deployments, then the new directive should not be SslBump-specific either, of course. I have only heard about this problem in SslBump context, but that does not mean the problem is specific to SslBump.

---

Amos: 1. Squid current behaviour of doing a TLS "success" and presenting 5xx HTTP error page - is to avoid bugs in Chrome and Firefox handling of non-HTTP/1.1 200 OK responses to CONNECT tunnel attempts.

Just to clarify: On many SslBump errors, Squid bumps TLS client connection and waits for the first HTTP request on it to send the error response because some popular browsers do not show CONNECT error responses to users. AFAIK from HTTP WG discussions, browser folks do not consider this behavior to be a browser bug; they just do not want to spend (non-trivial) resources on properly relaying the error to the user. Thus, this aspect of browser behavior is unlikely to change in the foreseeable future.

The corresponding Squid behavior breaks "try HTTPS first" feature in recent Chrome releases (when the user types in the domain name without the URI scheme), as documented in this PR description. This PR idea to make that behavior conditional is correct, but PR implementation needs to be refactored as discussed above.

[ss3git]

My explanation in this PR might have an unclear point, so I leave a note first here just in case:

Squid returns "HTTP/1.1 200" to the client for CONNECT to affected sites even if this draft option is set.

After receiving the actual request (the first HTTP request as you mention) from the client, Squid without the option returns the 503 error page (ERR_SECURE_CONNECT_FAIL/ERR_CONNECT_FAIL), and Squid with the option set returns nothing (terminates TCP with no more header/body).

The browser shows ERR_EMPTY_RESPONSE (or ERR_CONNECTION_RESET if the commit c575e8d is applied) message for the latter case (if intentionally tried to access the URL of wrong HTTPS, not by HTTPS Upgrade).

[rousskov]

FWIW, you comment matches my overall understanding of this PR. The only difference I am aware of is that Squid does not close the connection to the browser in my tests and "terminates TCP" in yours. We are discussing that aspect in another change request.

[rousskov]

After testing this PR a little, I would like to add the following to my blocking review: The current PR code does result in "HTTPS to HTTP" downgrade in Chrome, as intended, but the desired Chrome behavior appears to be triggered by a timeout in Chrome (i.e. by Squid inaction): In my quick-and-dirty tests, the original CONNECT tunnels opened by Chrome remain open when Chrome (after a ~6 second pause) sends a plain text request to Squid. If that observation is not a side effect of my poor testing, then I hope Squid behavior can be changed when PR code is refactored to address earlier review concerns: If it all possible, we want Squid to close doomed CONNECT tunnels; we do not want these tunnels to wait for browser timeouts.

[ss3git]

I am not sure what is actually causing the difference though, the browser does not seem to (generally) wait the timeout on my environment.

I am aware of that the timeout is one of the triggers of HTTPS to HTTP downgrade. If the upstream HTTPS port is unreachable (TCP SYN is silently dropped), Squid wait TCP ACK for a certain amount of time then that perhaps results in the same behaviour of browser like you observe. Does it match to your test case?

[ss3git]

Note: if waiting timeout generally occurs depending on the platform, sending TCP RST (setsockopt) at the disconnection may be an idea to try. I prototyped before posting this PR but did not include it there because I did not find any benefit on my environment.

[rousskov]

I am not sure what is actually causing the difference though, the browser does not seem to (generally) wait the timeout on my environment.

What signal/information do you think your browser is acting on if it is not the timeout? AFAICT, Squid sends nothing (but a TCP ACK packet) to the browser after responding to CONNECT... In your tests, does Squid close the browser-to-Squid connection before browser downgrades? If yes, I can think of several explanations for that difference, including these three:

1. a bad side effect of my quick-and-dirty tests; this is the most likely reason
2. a side effect of a different ssl_bump configuration (I tested with ssl_bump stare all) or some other valid/reasonable but different configuration options
3. a side effect that you have not tested master-based code (because it did not compile until 297a8d0)

If the upstream HTTPS port is unreachable (TCP SYN is silently dropped), Squid wait TCP ACK for a certain amount of time then that perhaps results in the same behaviour of browser like you observe. Does it match to your test case?

Please note that what happens between Squid and the origin server is irrelevant to my concern. Said that, IIRC, in my test, Squid "immediately" receives a RST packet because nobody is listening on the HTTPS port of the test origin server.

[ss3git]

What signal/information do you think your browser is acting on if it is not the timeout? AFAICT, Squid sends nothing (but a TCP ACK packet) to the browser after responding to CONNECT... In your tests, does Squid close the browser-to-Squid connection before browser downgrades?

Edge: CONNECT tcp session is immediately (of course with slight delay to access the origin server) disconnected by FIN, and GET follows it.

Chrome: needed to push the security warning button before actually access to the downgraded HTTP, so there is a little interval between CONNECT and GET, but you will see CONNECT tcp session is immediately disconnected by FIN.

[ss3git]

My test environment is also dirty.
I'm using the version of v6.6 with KTLS patch (and some more personal patch like increasing buffer size)..
If the difference persists, we have to narrow down the cause.

[rousskov]

If the difference persists, we have to narrow down the cause.

Agreed. I am not particularly worried about kept-open browser-Squid connections at this time as long as we agree that they should not be kept open. We will retest when other (primary) concerns are addressed.

[ss3git]

This seems to be the one.

a side effect of a different ssl_bump configuration (I tested with ssl_bump stare all) or some other valid/reasonable but different configuration options

I had been using ssl_bump bump all.
Changing it to ssl_bump stare all reproduces the timeout.

[ss3git]

Changed the code to proactively terminate (reset) tcp as of 1943d24.

[rousskov]

Thank you for your continued effort to address this and similar issues with SslBump error handling! I (still) support the addition of on_ssl_bump_error or similar direcitve. Your changes have significantly improved this PR. Thank you! I wish I could find more time to devote to this issue.

There are many secondary problems in the proposed new code, but we should ignore them until the following primary issues are addressed:

1. on_ssl_bump_error should not apply to bumped tunnels

The new feature should only apply to transactions where the from-client connection has not been bumped or spliced yet. Once the connections are spliced or (at least) the client is bumped, the new ..._ssl_bump_... directive should not be consulted because SslBump scope has ended (at least for that spliced or bumped client). In other words, on_ssl_bump_error directive is not an on_error directive!

To address this concern, we could detect the right from-client connection SslBump state in FwdState::bumpOnError(), but that approach is flawed because FwdState is not the code that determines what happens to the client: FwdState (and friends) generate errors when things go wrong while talking to the server, but some errors may happen before FwdState gets control, and the (unwanted) reaction to all errors does not actually happen in FwdState! The decision to bump the client and serve the error response to that client happens on the other side (I do not recall the exact code, but you will find it if you follow debugging in the problematic cases; looking for "Force bump action on error" may also help). That is the code where the new directive should be consulted.

2. on_ssl_bump_error should apply to ssl_bump errors encountered when talking to the client

This application is in addition to reacting to errors encountered when talking to the origin server, of course. It is possible that you will "automatically" add this support while addressing the first concern above. You can stop reading this section if you do. Otherwise, please keep reading.

The initial implementation does not have to support both sides, but, if it does not support both sides, its documentation must explicitly state this temporary limitation while making sure that we can enhance support in the future without renaming the directive, adding a new one, or warning the admins.

This concern can be addressed by adjusting cf.data.pre text AFAICT. We need to be very clear about this new directive (intended) scope, so that reviewers know what feature they are approving even if they do not review implementation details. I will help polish cf.data.pre text, but please make the initial edits, so that I do not forget to disclose this important limitation/plans when polishing.

[ss3git]

Hi, @rousskov

I have not wholly understood your comment yet (with a concrete image of codes), and it may take time for me, so for the meanwhile, I have added some notes regarding ERR_ZERO_SIZE_OBJECT in this PR description that I did not realize when initially submitting this PR.

Necessity of supporting ERR_ZERO_SIZE_OBJECT (which occurs after successful CONNECT, and if I’m understanding the terms correctly, in the bumped tunnel) might affect the appropriate directive (name) and/or code changes.

[rousskov]

I have not wholly understood your comment yet (with a concrete image of codes), and it may take time

Needless to say, please feel free to ask questions, especially if something I wrote is not clear or appears to be self-contradictory.

meanwhile, I have added some notes regarding ERR_ZERO_SIZE_OBJECT in this PR description ... Necessity of supporting ERR_ZERO_SIZE_OBJECT (which occurs after successful CONNECT, and if I’m understanding the terms correctly, in the bumped tunnel) might affect the appropriate directive (name) and/or code changes.

Bugs notwithstanding, ERR_ZERO_SIZE_OBJECT error may only occur after active SslBump processing is finished. In other words, it should have no effect on SslBump code in general and the new directive in particular.

[ss3git]

For the record before further discussion:

here is an example of public site where Squid with bump configuration (as of today) generates Zero Sized Reply for HTTPS-upgraded Edge: tempesta-tech.com/wp-content/uploads/2022/09/5_tempesta_tls.pdf

Without the (draft) directive:

With the (draft) directive enabled:

• acl terminate_on_bump_error squid_error ERR_ZERO_SIZE_OBJECT
• on_ssl_bump_error terminate terminate_on_bump_error

Edge falls back to HTTP and shows a PDF

[ss3git]

@rousskov

So, reading your comment, my current understanding is that your suggested directive “on_ssl_bump_error” or the term SslBump processing seems to have targeted only narrower phase of session establishment than I initially imagined.

As previously commented, in addition to ERR_CONNECT_FAIL and ERR_SECURE_CONNECT_FAIL, ERR_ZERO_SIZE_OBJECT has newly been found as a target to work around the HTTPS Upgrade issue. At the same time, I do not think ERR_ZERO_SIZE_OBJECT during non-ssl-bumped session (I hope this term is ok..) should be targeted.

Supposing the proper directive “on_ssl_bump_error” is not expected to apply to bumped tunnels, and if that means the directive does not cover the ERR_ZERO_SIZE_OBJECT in question, since this PR’s main objective is making a workaround for the HTTPS Upgrade issue, I guess we should first rename the directive (and relative function names etc.) to some appropriate one that covers bumped tunnels then discuss how we can improve the implementation.

Please let me know if my observation is wrong.

[rousskov]

Alleged facts:

Alex: ERR_ZERO_SIZE_OBJECT error may only occur after active SslBump processing is finished. In other words, it should have no effect on SslBump code in general and the [proposed] new directive in particular.

ss3git: Squid with bump configuration generates ERR_ZERO_SIZE_OBJECT for HTTPS-upgraded Edge. ... With the (draft) directive enabled, Edge falls back to HTTP.

There are several ways to reconcile the two statements (at the specs level), but I will focus on the most viable candidate:

• on_error: The proposed new directive scope should be enlarged to apply to all final Squid-generated errors, including post-SslBump errors (e.g., ERR_ZERO_SIZE_OBJECT). If Squid (in the absence of this directive) would log %err_code=ERR_FOO for the master transaction, then the new directive is applied before the corresponding Squid-to-client communication takes place. The directive is consulted even if SslBump is not enabled or is not applied to the current master transaction. Existing or, if necessary, new ACLs should be used to distinguish errors associated with being-bumped or bumped tunnels, as needed.

we should first rename the directive (and relative function names etc.) to some appropriate one that covers bumped tunnels

To reduce the number of code rewrites, I would not rush into changing code at this point. Let's agree on the specs first. That agreement should be reflected in cf.data.pre. If you do not want to break the build, you can keep the NAME line and related metadata intact while we work on the primary text documenting the new directive.

[and only] then discuss how we can improve the implementation.

Yes, but I would move "(and relative function names etc.)" from your step one into this second step. Let's agree on basic/high-level specs first (and give others an opportunity to review the version we are happy with).

Do you agree with the new directive scope (i.e. the on_error bullet above)? If yes, would you be able to adjust cf.data.pre text to match it (while keeping my earlier concerns in mind) or do you prefer for me to do that? Again, this text will document what we want Squid to do (when this PR code is adjusted and merged), and not what the current PR code does right now. In other words, the current PR code may be ignored while we work on the specs during this round.

[ss3git]

@rousskov

Do you agree with the new directive scope (i.e. the on_error bullet above)? If yes, would you be able to adjust cf.data.pre text to match it (while keeping my earlier concerns in mind) or do you prefer for me to do that?

I have no specific objection to the scope so updated cf.data.pre.

Although I kept the option reset (RST) now, it does not seem to be mandatory to work around the HTTPS Upgrade issue.
Eliminating the option can be considered if supporting it is difficult for every case.

Existing or, if necessary, new ACLs should be used to distinguish errors associated with being-bumped or bumped tunnels, as needed.

Yes, but I would move "(and relative function names etc.)" from your step one into this second step. Let's agree on basic/high-level specs first (and give others an opportunity to review the version we are happy with).

To check if "annotate_client bumped=true" works, I have changed a part of the code and some function names along the way.
I have pushed the code just in case, but I would not request your review for that now.

[ss3git]

Regarding the specific ERR_ZERO_SIZE_OBJECT error case (tempesta-tech.com/wp-content/uploads/2022/09/5_tempesta_tls.pdf), another factor may be affecting the result.

Unlike other (HTTPS-Upgrade) error cases I’m aware of, direct access (or access via Squid splice) to the https url works normally, and pdf is shown correctly.

The cause is yet unknown.

[ss3git]

Regarding the specific ERR_ZERO_SIZE_OBJECT error case (tempesta-tech.com/wp-content/uploads/2022/09/5_tempesta_tls.pdf), another factor may be affecting the result.

Unlike other (HTTPS-Upgrade) error cases I’m aware of, direct access (or access via Squid splice) to the https url works normally, and pdf is shown correctly.

The cause is yet unknown.

Looks like this is not a Squid (only) problem.

% wget https://tempesta-tech.com/wp-content/uploads/2022/09/5_tempesta_tls.pdf
Resolving tempesta-tech.com (tempesta-tech.com)... 185.8.106.163
Connecting to tempesta-tech.com (tempesta-tech.com)|185.8.106.163|:443... connected.
HTTP request sent, awaiting response... No data received.
Retrying.

% wget http://tempesta-tech.com/wp-content/uploads/2022/09/5_tempesta_tls.pdf
Resolving tempesta-tech.com (tempesta-tech.com)... 185.8.106.163
Connecting to tempesta-tech.com (tempesta-tech.com)|185.8.106.163|:80... connected.
HTTP request sent, awaiting response... 200 OK
Length: 104384 (102K) [application/pdf]
Saving to: ‘5_tempesta_tls.pdf’


% fetch https://tempesta-tech.com/wp-content/uploads/2022/09/5_tempesta_tls.pdf
fetch: https://tempesta-tech.com/wp-content/uploads/2022/09/5_tempesta_tls.pdf: No error: 0

% fetch http://tempesta-tech.com/wp-content/uploads/2022/09/5_tempesta_tls.pdf
5_tempesta_tls.pdf                                     101 kB  222 kBps    00s

The same result on FreeBSD 13.3, 12.4, and Ubuntu2310.

Google's search result url (not https but http) reinforces it.

[ss3git]

Regarding the specific ERR_ZERO_SIZE_OBJECT error case (tempesta-tech.com/wp-content/uploads/2022/09/5_tempesta_tls.pdf), another factor may be affecting the result.

1. The site (HTTPS port) in question only supports HTTP/2.

% curl -vI --http1.1 https://tempesta-tech.com/wp-content/uploads/2022/09/5_tempesta_tls.pdf
* Host tempesta-tech.com:443 was resolved.
* IPv6: (none)
* IPv4: 185.8.106.163
*   Trying 185.8.106.163:443...
* Connected to tempesta-tech.com (185.8.106.163) port 443
* ALPN: curl offers http/1.1
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
* TLSv1.3 (IN), TLS alert, no application protocol (632):
* OpenSSL/1.1.1w: error:14094460:SSL routines:ssl3_read_bytes:reason(1120)
* Closing connection
curl: (35) OpenSSL/1.1.1w: error:14094460:SSL routines:ssl3_read_bytes:reason(1120)

% curl -vI --http2 https://tempesta-tech.com/wp-content/uploads/2022/09/5_tempesta_tls.pdf
* Host tempesta-tech.com:443 was resolved.
* IPv6: (none)
* IPv4: 185.8.106.163
*   Trying 185.8.106.163:443...
* Connected to tempesta-tech.com (185.8.106.163) port 443
* ALPN: curl offers h2,http/1.1
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
* TLSv1.3 (IN), TLS handshake, Server hello (2):
* TLSv1.2 (IN), TLS handshake, Certificate (11):
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
* TLSv1.2 (IN), TLS handshake, Server finished (14):
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
* TLSv1.2 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (OUT), TLS handshake, Finished (20):
* TLSv1.2 (IN), TLS handshake, Finished (20):
* SSL connection using TLSv1.2 / ECDHE-ECDSA-AES128-GCM-SHA256 / [blank] / UNDEF
* ALPN: server accepted h2
* Server certificate:
*  subject: CN=tempesta-tech.com
*  start date: Apr  7 09:13:14 2024 GMT
*  expire date: Jul  6 09:13:13 2024 GMT
*  subjectAltName: host "tempesta-tech.com" matched cert's "tempesta-tech.com"
*  issuer: C=US; O=Let's Encrypt; CN=R3
*  SSL certificate verify ok.
*   Certificate level 0: Public key type ? (256/128 Bits/secBits), signed using sha256WithRSAEncryption
*   Certificate level 1: Public key type ? (2048/112 Bits/secBits), signed using sha256WithRSAEncryption
*   Certificate level 2: Public key type ? (4096/128 Bits/secBits), signed using sha256WithRSAEncryption
* using HTTP/2
* [HTTP/2] [1] OPENED stream for https://tempesta-tech.com/wp-content/uploads/2022/09/5_tempesta_tls.pdf
* [HTTP/2] [1] [:method: HEAD]
* [HTTP/2] [1] [:scheme: https]
* [HTTP/2] [1] [:authority: tempesta-tech.com]
* [HTTP/2] [1] [:path: /wp-content/uploads/2022/09/5_tempesta_tls.pdf]
* [HTTP/2] [1] [user-agent: curl/8.5.0]
* [HTTP/2] [1] [accept: */*]
> HEAD /wp-content/uploads/2022/09/5_tempesta_tls.pdf HTTP/2
> Host: tempesta-tech.com
> User-Agent: curl/8.5.0
> Accept: */*
>
< HTTP/2 200
HTTP/2 200
< date: Thu, 11 Apr 2024 08:56:59 GMT
date: Thu, 11 Apr 2024 08:56:59 GMT
< last-modified: Tue, 09 Apr 2024 21:31:43 GMT
last-modified: Tue, 09 Apr 2024 21:31:43 GMT
< etag: "197c0-615b0a60182aa"
etag: "197c0-615b0a60182aa"
< accept-ranges: bytes
accept-ranges: bytes
< content-length: 104384
content-length: 104384
< content-type: application/pdf
content-type: application/pdf
< via: 2.0 tempesta_fw (Tempesta FW 0.8.0)
via: 2.0 tempesta_fw (Tempesta FW 0.8.0)
< server: Tempesta FW/0.8.0
server: Tempesta FW/0.8.0
< strict-transport-security: max-age=31536000; includeSubDomains
strict-transport-security: max-age=31536000; includeSubDomains

<
* Connection #0 to host tempesta-tech.com left intact

2. A small hack of Squid that calls OpenSSL SSL_set_alpn_protos() to declare application_layer_protocol_negotiation in Client Hello changes the error status ERR_ZERO_SIZE_OBJECT into ERR_SECURE_CONNECT_FAIL:

So, this is technically a separate issue from this PR.

@yadij
It might be a topic to be discussed here?: #893