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

tls backends using connection filter IO #9962

Closed
wants to merge 1 commit into from
Closed

Conversation

icing
Copy link
Contributor

@icing icing commented Nov 22, 2022

  TLS backends using connection filters for IO, enabling HTTPS-proxy.

    - OpenSSL (and compatible)
    - BearSSL
    - gnutls
    - mbedtls
    - rustls
    - schannel
    - secure-transport
    - wolfSSL (v5.0.0 and newer)

    This leaves the only the following without HTTPS-proxy support:
    - gskit
    - nss
    - wolfSSL (versions earlier than v5.0.0)

(void)mbedtls_ssl_read(&backend->ssl, (unsigned char *)buf, sizeof(buf));
backend->io_data = NULL;
Copy link
Member

@jay jay Nov 23, 2022

Choose a reason for hiding this comment

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

Though I'm loathe to bring up conn->data (which was eliminated about 2 years ago) is there a better way then than the association of data like this at a bunch of seemingly arbitrary points? Yes I'm sure they're not arbitrary but I'm struggling to follow how this works. Is there a more central location where data could be assigned, like vtls for example?

Copy link
Contributor Author

@icing icing Nov 23, 2022

Choose a reason for hiding this comment

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

I agree that this is looking for a more elegant solution. The sprinkling of those assignments in the code is not to my liking really, either.

That we got to have data everywhere is not the right design, since we now have multi-use of connections. In the connection filter chain, bytes handled often to not belong to a specific data.

So, in my view, we should get rid of data in connection IO, at least on the filter level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, changes now introduce a standard vtls way to save the current Curl_easy handle in a filter call and backends that need it use it from there.

The "standard" way now also NULLs at the end of a call, so UAFs should not happen.

@icing icing changed the title first batch of tls backends with filter chain IO tls backends using connection filter IO Nov 23, 2022
@icing icing force-pushed the cf-tls-chain branch 5 times, most recently from ba6eaaa to 94ddf80 Compare November 25, 2022 13:07
            - OpenSSL (and compatible)
            - BearSSL
            - gnutls
            - mbedtls
            - rustls
            - schannel
            - secure-transport
            - wolfSSL (v5.0.0 and newer)

            This leaves the only the following without HTTPS-proxy support:
            - gskit
            - nss
            - wolfSSL (versions earlier than v5.0.0)
@bagder
Copy link
Member

bagder commented Nov 28, 2022

@icing ready for merge?

@bagder
Copy link
Member

bagder commented Nov 28, 2022

Thanks!

@bagder bagder closed this in 55807e6 Nov 28, 2022
@vszakats vszakats mentioned this pull request Jun 30, 2024
1 task
vszakats added a commit to vszakats/curl that referenced this pull request Jul 1, 2024
- support detecting wolfSSL via pkg-config (like autotools.)

- detect wolfSSL version.

- add `HTTPS-proxy` to feature list for fitting wolfSSL versions.
  (completing this TODO)

Ref: 55807e6 curl#9962
Closes #xxxxx
vszakats added a commit that referenced this pull request Jul 1, 2024
- support detecting wolfSSL via pkg-config (like autotools.)

- detect wolfSSL version.

- detect `HAVE_WOLFSSL_DES_ECB_ENCRYPT`.
  (needs e.g. `--enable-curl` when building wolfSSL)

- detect `HAVE_WOLFSSL_FULL_BIO` and enable HTTPS-proxy feature.
  (needs e.g. `--enable-opensslall` when building wolfSSL)

- fix to show `HTTPS-proxy` in cmake feature list.
  Ref: 55807e6 #9962

- fix to show `NTLM` in cmake feature list.

- fix to show `smb` and `smbs` in cmake protocol list.

- add wolfSSL CMake job to GHA (for macOS).

- fix mqtt and wolfSSL symbol clash.
  ```
  ./curl/lib/mqtt.c: In function 'mqtt_doing':
  ./curl/lib/mqtt.c:746:17: error: declaration of 'byte' shadows a global declaration [-Werror=shadow]
    746 |   unsigned char byte;
        |                 ^~~~
  /opt/homebrew/Cellar/wolfssl/5.7.0_1/include/wolfssl/wolfcrypt/types.h:85:36: note: shadowed declaration is here
     85 |             typedef unsigned char  byte;
        |                                    ^~~~
  ```

- format `FindWolfSSL.cmake` closer to neighbours.

Closes #14064
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants