getsentry / symbolicator

Native Symbolication as a Service
https://getsentry.github.io/symbolicator/
MIT License
352 stars 45 forks source link

feat: Add no_ssl client to the HTTP downloader #1404

Closed loewenheim closed 3 months ago

loewenheim commented 3 months ago

Based on #1403.

This adds a reqwest::Client to HttpDownloader ~and GcsDownloader~ which doesn't reject invalid SSL certs. For now, this client is unused.

Question: do we actually need/want this for GCS? Answer: no.

anonrig commented 3 months ago

What's the goal of having a client like this? Why do we want to not throw an error for an invalid SSL?

loewenheim commented 3 months ago

What's the goal of having a client like this? Why do we want to not throw an error for an invalid SSL?

Basically we can't fetch from a particular symbol source (NVidia) because (to my understanding) their SSL certs are broken. Obviously they need to fix this, but in the meantime we want to still be able to access the source.

Swatinem commented 3 months ago

the certs are not really "broken". AFAIK, they just use newer features that are simply not supported by openssl / ca-certificates which come with the debian image we are using. Also latest ubuntu curl can’t access that server.

However browsers can access it just fine, because they support this newer certificate feature.

loewenheim commented 3 months ago

Thanks for the clarification!

anonrig commented 3 months ago

Accepting invalid certificates result in not having any integrity checks and makes us vulnerable to man in the middle attacks. I understand the product wise reasoning behind this, but we should know that this is extremely risky.

loewenheim commented 3 months ago

Accepting invalid certificates result in not having any integrity checks and makes us vulnerable to man in the middle attacks. I understand the product wise reasoning behind this, but we should know that this is extremely risky.

Agreed, we need to be careful with this.

Swatinem commented 3 months ago

There is no doubt about that. I might also note that I proposed this purely because it was asked for, and I advocate against doing this.

To put it into perspective: we have been failing these requests for almost 2 months by now, and I haven’t really heard anyone actively complaining about this. So the urgency here might be very exaggerated.