squid icon indicating copy to clipboard operation
squid copied to clipboard

Workaround for incompatibility with HTTPS Upgrade

Open ss3git opened this issue 1 year ago • 11 comments

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

Background

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://github.com/w3ctag/design-reviews/issues/853#issuecomment-1622149334

ss3git avatar Feb 12 '24 05:02 ss3git

Can one of the admins verify this patch?

squid-prbot avatar Feb 12 '24 05:02 squid-prbot

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.

yadij avatar Feb 12 '24 09:02 yadij

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.

ss3git avatar Apr 03 '24 18:04 ss3git

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.

rousskov avatar Apr 03 '24 19:04 rousskov

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: image

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

image

ss3git avatar Apr 04 '24 11:04 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.

ss3git avatar Apr 04 '24 12:04 ss3git

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.

rousskov avatar Apr 04 '24 17:04 rousskov

@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 avatar Apr 05 '24 14:04 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 avatar Apr 06 '24 02:04 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.

image

ss3git avatar Apr 06 '24 02:04 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

  1. 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:

image

image

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

@yadij It might be a topic to be discussed here?: https://github.com/squid-cache/squid/pull/893

ss3git avatar Apr 11 '24 10:04 ss3git