Skip to content

[FEATURE] SSL verification + custom proxies #3844

Closed

Description

@codereptile

What kind of feature are you missing? Where do you notice a shortcoming of PTB?

There is no way to disable ssl verification for proxies and pass a custom httpx.Proxy, instead of just a string.

Describe the solution you'd like

Some proxies work as a man-in-the-middle and may lead to errors, like: ssl.SSLCertVerificationError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1006).

Due to this reason, I think it's necessary to allow control over ssl verification in HTTPXRequest. Just add an argument like ssl_verify: bool = True and add verify=ssl_verify to self._client_kwargs.

Another thing I'd love to see, is to change the type of:
proxy_url: Optional[str] = None,
to
proxy_url: Optional[Union[str, httpx.Proxy] = None,

This is due to some proxies requiring authorization through headers, which could be passed as:

httpx.Proxy(
    url=proxy_url,
    headers={
        "x-secret-header": "secret value"
    },
)

(moreover it might be used for passing logging headers to the proxy and so on)

Describe alternatives you've considered

No response

Additional context

No response

Activity

  1. changed the title [-][FEATURE][/-] [+][FEATURE] SSL verification + custom proxies[/+] on Aug 14, 2023
  2. tsnoam commented on Aug 16, 2023

    @tsnoam
    Member
    1. I really dislike verify=False. It is better to add a custom CA cert, if and only if the user trusts the entity performing ssl mitm.
      I would argue that we shouldn't easily allow disabling ssl verification as it compromises the security guarantees of Telegram. If someone really really want it then they should inherit from HTTPXRequest (or monkey patch it) and mangle whatever they want, but then the responsibility and the implications are on them.

    2. The suggested approach to setting a proxy seems like a solid solution. We should do it.

  3. codereptile commented on Aug 17, 2023

    @codereptile
    Author

    @tsnoam

    1 - In my specific case, there is simply no way to provide a custom CA cert, but I can agree that this might be a one-of-a-kind problem, so it shouldn't be implemented. However, I'd ask you to review the patch I'm currently using:

    def _build_client(self) -> httpx.AsyncClient:
        self._client_kwargs['verify'] = False
        return httpx.AsyncClient(**self._client_kwargs)
    

    2 - If I could be of any assistance, I'd be glad to help. I can make a pull request, but since this seems like a minor change, I'd probably spend more time reading guidelines.

  4. Bibo-Joshi commented on Aug 19, 2023

    @Bibo-Joshi
    Member

    1: Your snippet looks okay to me. Just be aware that we make no guarantees about the stability of protected methods :)

    2: if we allow httpx.Proxie input, then the name "proxy_url" is no longer the best. I suggest deprecating that argument in favor of a new proxy argument that accepts both a string or such an object.
    One more comment: httpx accepts multiple proxies, e.f.

    proxies = {
        # Route all traffic through a proxy by default...
        "all://": "http://localhost:8030",
        # But don't use proxies for HTTPS requests to "domain.io"...
        "https://domain.io": None,
        # And use another proxy for requests to "example.com" and its subdomains...
        "all://*example.com": "http://localhost:8031",
        # And yet another proxy if HTTP is used,
        # and the "internal" subdomain on port 5550 is requested...
        "http://internal.example.com:5550": "http://localhost:8032",
    }

    Since in our case we only have one target to query, I see no need to allow for such functionality. Cc @tsnoam

    @codereptile you are ofc very welcome to contribute if you are interested :)

  5. Bibo-Joshi commented on Sep 25, 2023

    @Bibo-Joshi
    Member

    v0.25.0 of httpx adds support for https proxies is encode/httpx#2845. Would this feature resolve the verification problem mentioned in the original comment (possibly after exposing this feature appropriately via PTBs interface)? CC @tsnoam

  6. codereptile commented on Sep 28, 2023

    @codereptile
    Author

    @Bibo-Joshi As far as I'm concerned, yes, you could set up the https proxy correctly, if PTB would expose the https proxy feature.
    Seems like both tickets could be solved with a single PR.

  7. tsnoam commented on Oct 21, 2023

    @tsnoam
    Member

    @Bibo-Joshi I think that ptb already supports getting a httpx.Proxy object over proxy_url as there's no enforcement to it being str.

    So if I read the code right, the problem is semantics & documentation. Nothing on the code itself.

    So basically, in ideal world what could have been changed is:

    1. rename proxy_url to proxy
    2. change proxy type from str to httpx.Proxy.
      and that's it.

    again, I hope I got things right here...

  8. codereptile commented on Oct 21, 2023

    @codereptile
    Author

    @tsnoam Yes, you can actually just throw in the proxy and it'll work (and that's how I did), but the type/name change would be nice to avoid IDE/mypy errors

  9. locked and limited conversation to collaborators on Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Fields

    Priority

    None yet

    Effort

    None yet

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions