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

Add support for CONNECT method on HTTP/2 connection #9616

Merged
merged 2 commits into from
May 1, 2023

Conversation

maskit
Copy link
Member

@maskit maskit commented Apr 16, 2023

This blog post motivated me to implement CONNECT method support.
https://daniel.haxx.se/blog/2023/04/14/curl-speaks-http-2-with-proxy/

$ ~/opt/curl/bin/curl -vk --proxy-insecure --proxy-http2 -x https://localhost:8443/ https://www.apache.org/ > /dev/null
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0*   Trying 127.0.0.1:8443...
* Connected to localhost (127.0.0.1) port 8443 (#0)
* ALPN: offers h2,http/1.1
} [5 bytes data]
* TLSv1.2 (OUT), TLS handshake, Client hello (1):
} [512 bytes data]
* TLSv1.2 (IN), TLS handshake, Server hello (2):
{ [122 bytes data]
* TLSv1.3 (OUT), TLS change cipher, Change cipher spec (1):
} [1 bytes data]
* TLSv1.3 (IN), TLS handshake, Encrypted Extensions (8):
{ [15 bytes data]
* TLSv1.3 (IN), TLS handshake, Certificate (11):
{ [1444 bytes data]
* TLSv1.3 (IN), TLS handshake, CERT verify (15):
{ [520 bytes data]
* TLSv1.3 (IN), TLS handshake, Finished (20):
{ [36 bytes data]
* TLSv1.3 (OUT), TLS handshake, Finished (20):
} [36 bytes data]
* SSL connection using TLSv1.3 / TLS_AES_128_GCM_SHA256
* ALPN: server accepted h2
* Proxy certificate:
*  subject: C=AU; ST=Some-State; O=Internet Widgits Pty Ltd; CN=localhost
*  start date: Apr 16 01:33:15 2023 GMT
*  expire date: Apr 15 01:33:15 2024 GMT
*  issuer: C=AU; ST=Some-State; O=Internet Widgits Pty Ltd; CN=localhost
*  SSL certificate verify result: self signed certificate (18), continuing anyway.
* CONNECT tunnel: HTTP/2 negotiated
* Establish HTTP/2 proxy tunnel to www.apache.org:443
} [5 bytes data]
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
{ [214 bytes data]
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
{ [214 bytes data]
* CONNECT tunnel established, response 200
* CONNECT phase completed
* ALPN: offers h2,http/1.1
} [5 bytes data]
* TLSv1.2 (OUT), TLS handshake, Client hello (1):
} [512 bytes data]
* TLSv1.2 (IN), TLS handshake, Server hello (2):
{ [122 bytes data]
* TLSv1.3 (OUT), TLS change cipher, Change cipher spec (1):
} [1 bytes data]
* TLSv1.3 (IN), TLS handshake, Encrypted Extensions (8):
{ [19 bytes data]
* TLSv1.3 (IN), TLS handshake, Certificate (11):
{ [4022 bytes data]
* TLSv1.3 (IN), TLS handshake, CERT verify (15):
{ [264 bytes data]
* TLSv1.3 (IN), TLS handshake, Finished (20):
{ [36 bytes data]
* TLSv1.3 (OUT), TLS handshake, Finished (20):
} [36 bytes data]
* SSL connection using TLSv1.3 / TLS_AES_128_GCM_SHA256
* ALPN: server accepted h2
* Server certificate:
*  subject: CN=*.apache.org
*  start date: Apr 13 16:56:32 2023 GMT
*  expire date: Jul 12 16:56:31 2023 GMT
*  issuer: C=US; O=Let's Encrypt; CN=R3
*  SSL certificate verify result: unable to get local issuer certificate (20), continuing anyway.
{ [5 bytes data]
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
{ [161 bytes data]
* using HTTP/2
* h2h3 [:method: GET]
* h2h3 [:path: /]
* h2h3 [:scheme: https]
* h2h3 [:authority: www.apache.org]
* h2h3 [user-agent: curl/8.1.0-DEV]
* h2h3 [accept: */*]
* Using Stream ID: 1 (easy handle 0x55fb74068380)
} [5 bytes data]
> GET / HTTP/2
> Host: www.apache.org
> user-agent: curl/8.1.0-DEV
> accept: */*
>
{ [5 bytes data]
< HTTP/2 200
< server: Apache
< last-modified: Sun, 16 Apr 2023 05:23:20 GMT
< etag: "1195a-5f96d45bc947c"
< cache-control: max-age=3600
< expires: Sun, 16 Apr 2023 06:23:21 GMT
< content-security-policy: default-src 'self' 'unsafe-inline' https://www.apachecon.com/ https://www.google.com/cse/ https://cse.google.com/ https://www.googleapis.com/generate_204 http://*.google.com/generate_204 https://afs.googlesyndication.com/ https://csp.withgoogle.com/ https://www.google.com/images/ https://ssl.gstatic.com/ui/ https://docs.google.com/forms/ https://www.youtube.com/embed/; script-src 'self' 'unsafe-inline' 'unsafe-eval' https://cse.google.com/ http://cse.google.com/adsense/search/async-ads.js https://www.google.com/cse/ https://partner.googleadservices.com/; style-src 'self' 'unsafe-inline' https://www.google.com/cse/; frame-ancestors 'none';
< strict-transport-security: max-age=31536000; preload
< content-type: text/html
< via: 1.1 varnish, 1.1 varnish
< accept-ranges: bytes
< date: Sun, 16 Apr 2023 05:50:09 GMT
< age: 1608
< x-served-by: cache-hel1410025-HEL, cache-dfw-kdfw8210029-DFW
< x-cache: HIT, HIT
< x-cache-hits: 4, 1
< x-timer: S1681624210.756756,VS0,VE1
< vary: Accept-Encoding
< content-length: 72026
<
{ [5 bytes data]
100 72026  100 72026    0     0   556k      0 --:--:-- --:--:-- --:--:--  558k
* Connection #0 to host localhost left intact

@maskit maskit added the HTTP/2 label Apr 16, 2023
@maskit maskit added this to the 10.0.0 milestone Apr 16, 2023
@maskit maskit self-assigned this Apr 16, 2023
@bryancall bryancall requested a review from bneradt April 17, 2023 22:13
Copy link
Contributor

@bneradt bneradt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to add an AuTest for this. In case it's helpful, Proxy Verifier supports the CONNECT method. Zhengxi added a test using it here:
https://github.com/apache/trafficserver/blob/master/tests/gold_tests/connect/connect.test.py#L112

I think it should work fine for HTTP/2, but of course let us know if it shows problems.

@maskit
Copy link
Member Author

maskit commented Apr 18, 2023

It doesn't seem like Proxy Verifier supports H2 CONNECT method. It doesn't allow me to only send :method and :authority (it thinks :scheme and :path are always required).

And this might be because of my limited knowledge about ProxyVerifier, but it looks like requests and responses are tightly coupled by uuid header, and that makes it tricky to send a request as a request body like below. The both CONNECT request and the real request have uuid: 1.

sessions:
  - protocol:
    - name: http
      version: 2
    - name: tls
      sni: www.example.com
    - name: tcp
    - name: ip

    transactions:
      - client-request:
          headers:
            fields:
            - [ :method, CONNECT ]
            - [ :authority, www.example.com:80 ]
            - [ :path, x1]
            - [ :scheme, x2]
            - [ uuid, 1 ]
            - [ test, connect-request ]
          content:
            encoding: plain
            data: "GET /get HTTP/1.1\r\nuuid: 1\r\ntest: real-request\r\n\r\n"

        # This is a response for the real request but not for the CONNECT request
        server-response:
          status: 200
          reason: OK

        # ATS returns a 200 responses to client when it establishes a tunnel
        # between the client and server
        # The response body should be the whole response data from the origin server
        proxy-response:
          status: 200
          content:
            verify: {value: "HTTP/1.1 200 OK", as: contains}
class ConnectViaPVTest2:
    # This test also executes the CONNECT request but using proxy verifier to
    # generate traffic
    connectReplayFile = "replays/connect_h2.replay.yaml"

    def __init__(self):
        self.setupOriginServer()
        self.setupTS()

    def setupOriginServer(self):
        self.server = Test.MakeVerifierServerProcess(
            "connect-verifier-server2",
            self.connectReplayFile)
        # Verify server output
        self.server.Streams.stdout += Testers.ExcludesExpression(
            "test: connect-request",
            "Verify the CONNECT request doesn't reach the server.")
        self.server.Streams.stdout += Testers.ContainsExpression(
            "GET /get HTTP/1.1\nuuid: 1\ntest: real-request", reflags=re.MULTILINE,
            description="Verify the server gets the second request.")

    def setupTS(self):
        self.ts = Test.MakeATSProcess("connect-ts2", enable_tls=True)

        self.ts.Disk.records_config.update({
            'proxy.config.diags.debug.enabled': 1,
            'proxy.config.diags.debug.tags': 'http|hpack',
            'proxy.config.ssl.server.cert.path': f'{self.ts.Variables.SSLDir}',
            'proxy.config.ssl.server.private_key.path': f'{self.ts.Variables.SSLDir}',
            'proxy.config.http.server_ports': f"{self.ts.Variables.ssl_port}:ssl",
            'proxy.config.http.connect_ports': f"{self.server.Variables.http_port}",
        })

        self.ts.addDefaultSSLFiles()
        self.ts.Disk.ssl_multicert_config.AddLine(
            'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key'
        )

        self.ts.Disk.remap_config.AddLines([
            f"map / http://127.0.0.1:{self.server.Variables.http_port}/",
        ])
        # Verify ts logs
        self.ts.Disk.traffic_out.Content += Testers.ContainsExpression(
            f"Proxy's Request.*\n.*\nCONNECT 127.0.0.1:{self.server.Variables.http_port} HTTP/1.1", reflags=re.MULTILINE,
            description="Verify that ATS recognizes the CONNECT request.")

    def runTraffic(self):
        tr = Test.AddTestRun("Verify correct handling of CONNECT request on HTTP/2")
        tr.AddVerifierClientProcess(
            "connect-client2",
            self.connectReplayFile,
            https_ports=[self.ts.Variables.ssl_port],
            other_args='--thread-limit 1')
        tr.Processes.Default.StartBefore(self.server)
        tr.Processes.Default.StartBefore(self.ts)
        tr.StillRunningAfter = self.server
        tr.StillRunningAfter = self.ts

    def run(self):
        self.runTraffic()


ConnectViaPVTest2().run()

@bneradt
Copy link
Contributor

bneradt commented May 1, 2023

It doesn't seem like Proxy Verifier supports H2 CONNECT method. It doesn't allow me to only send :method and :authority (it thinks :scheme and :path are always required).

Yes, you are correct. Thank you for the heads up @maskit. I filed a Proxy Verifier issue for this:

yahoo/proxy-verifier#252

I believe all other attributes of Proxy Verifier should work with this. @duke8253 added support for explicit DATA frames which may be helpful here (although your use of the content: node should work as well).

And this might be because of my limited knowledge about ProxyVerifier, but it looks like requests and responses are tightly coupled by uuid header, and that makes it tricky to send a request as a request body like below. The both CONNECT request and the real request have uuid: 1.

Yes, the Proxy Verifier server uses uuid to know what response to send. I think this will fit the HTTP/2 CONNECT use case well. In the body of the CONNECT, to keep things distinct for the reader of the test, I suggest using a different uuid than the parent one. Then add a separate transaction to tell the server how to handle that UUID request. This is similar to what is done here:

https://github.com/apache/trafficserver/blob/master/tests/gold_tests/connect/replays/connect.replay.yaml#L42

But, as you noted already, things are a bit different with HTTP/2. Rather than the whole connection being a tunnel after the CONNECT, as it is with h1, only the DATA frames of the particular stream are tunneled. So the requests will have to happen on the client side via DATA frames, and the origin will parse them as regular h1 using the replay file's uuid transactions specified in the DATA frame/content nodes.

Thanks for working on this. It's neat to get this feature working in ATS (and Proxy Verifier).

Copy link
Contributor

@bneradt bneradt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks helpful. Let's get your change in as is which the current autest suite shows didn't break anything. I'll plan to take your observations from the autest you started with Proxy Verifier and generate a future release of Proxy Verifier that supports HTTP/2 CONNECT requests. With that we can add an autest with Proxy Verifier as a separate PR.

@maskit maskit merged commit 7e699e2 into apache:master May 1, 2023
bneradt pushed a commit that referenced this pull request Jun 25, 2023
)

In #9616, @maskit wrote an H2 CONNECT Autest but couldn't include that in the final PR because of a Proxy Verifier issue.
Now that the Proxy Verifier issue is resolved, the Autest is added in this PR(with a few tweaks).

ATS crashes with the new test executing HTTP/2 tunneling traffic. This PR also includes a fix to resolve that.
JosiahWI pushed a commit to JosiahWI/trafficserver that referenced this pull request Jul 19, 2023
These are cherry-picks from asf that has the fix for crash caused by H2 CONNECT.
* Added http connect Autest with proxy verifier (apache#9315)

* Added a HTTP CONNECT test using proxy verifier

* Updated comment

* added proxy-response verification

(cherry picked from commit b5f2023)

* Added Autest for H2 CONNECT and fix a crash triggered by the test (apache#9781)

In apache#9616, @maskit wrote an H2 CONNECT Autest but couldn't include that in the final PR because of a Proxy Verifier issue.
Now that the Proxy Verifier issue is resolved, the Autest is added in this PR(with a few tweaks).

ATS crashes with the new test executing HTTP/2 tunneling traffic. This PR also includes a fix to resolve that.

(cherry picked from commit df7ccfe)

* Update to Proxy Verifier v2.8.1 (apache#9834)

Proxy Verifier v2.8.1 has fixes for the way Proxy Verifier relates to
HTTP/2 CONNECT method request pseudo header fields. This will be helpful
for testing HTTP/2 CONNECT requests.

(cherry picked from commit d5c47a7)
JosiahWI pushed a commit to JosiahWI/trafficserver that referenced this pull request Jul 19, 2023
…ache#823)

* Add support for CONNECT method on HTTP/2 connection

* Check whether the header is a request header

(cherry picked from commit 7e699e2)

Co-authored-by: Masakazu Kitajo <maskit@apache.org>
cmcfarlen pushed a commit to cmcfarlen/trafficserver that referenced this pull request Jun 3, 2024
* asf/master: (33 commits)
  Add error log for invalid OCSP response (apache#9674)
  Add new settings to specify TLS versions to use (apache#9667)
  Remove flask from tests/Pipfile (apache#9688)
  Doc: Add example of --enable-lto build option with LLVM (apache#9654)
  Added Zhengxi to the asf contributors (apache#9685)
  Don't build native QUIC implementation (apache#9670)
  Stabilize autest tls_hooks17 (apache#9671)
  Cleanup: remove ts::buffer from HostDB. (apache#9677)
  Fix leak in MultiTextMod in ControlBase. (apache#9675)
  Cleanup: remove TsBuffer.h from MIME.cc (apache#9661)
  Cleanup: remove ts::Buffer from ControlBase. (apache#9664)
  setup pre-commit hook at cmake generation time (apache#9669)
  Updated parent retry attempt logic (apache#9620)
  Try to do less work in hot function HttpHookState::getNext (apache#9660)
  cmake build, fixed warning using older openssl APIs (apache#9648)
  rename attempts to retry_attempts (apache#9655)
  OCSP: Fix unitialized variable error. (apache#9662)
  Add support for CONNECT method on HTTP/2 connection (apache#9616)
  Remove deprecated debug output functions from 10 source files. (apache#9657)
  Remove deprecated debug output functions from 14 source files. (apache#9658)
  ...
@maskit maskit mentioned this pull request Aug 15, 2024
91 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants