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!

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.

None yet

3 participants