Add global site passthrough to ignore pinning

[trifle]

Hi, thanks for this wonderful project.
Following several recent issues, it seems that the current passthrough implementation cannot work with apps that implement pinning, since it only operates once the connection has already been initiated/spoofed.
The sister project sslproxy already implemented a per-domain whitelist that prevents spoofing for defined sites.
I've backported their code to sslsplit.

Note that I don't write C code and do not understand any of the details involved.
This patch compiles and works, and I've tried to retain the code, naming and formatting style.
But it should definitely be reviewed by someone who knows what they are doing (which you obviously are!) before merging.

By the way, if this is inconvenient or does not fit the project's scope, feel free to discard the PR.
Thanks & Regards!

[0x501D]

ps != NULL

[0x501D]

ps->ip != NULL check

[0x501D]

ps->site != NULL check

[trifle]

Many thanks for looking at this, @0x501D !
As mentioned in the PR, I barely know that null pointer dereferencing and the like exist, so I can't help, as much as I'd want.
But if you stumble upon serious issues, it might be worth looking at the implementation of https://github.com/sonertari/SSLproxy as well, which is where I lifted the code. Could be interesting to @sonertari as well (author of sslproxy and co-author here)

[sonertari]

Yes, I should check possible memory allocation issues in such cases too. But I think @droe may have a more important objection to this code being merged to sslsplit. I have implemented it using a linked list, but there may be more appropriate data structures in this case, such as a type of tree, perhaps tries. Because, the data structure remains constant after we populate it at the start. All we do afterwards is search for a matching string, the target site in current connection.

[trifle]

That makes sense; if it's possible to reduce the per-connection overhead then that would obviously be a good thing. I haven't tested performance with hundreds or thousands of passsites (so there could be pathologies if lookups are log(n) ), but overall sslsplit is so fast that it doesn't seem to matter much, even on rather slow machines.

[trifle]

Just a small update on this.
I've since switched from a simple lookup to suffix matching similar to this code. This allows the whitelisting to work on TLD level if desired, which makes more sense when dealing with pinning. It also has the side effect of reducing the number of entries, since multiple subdomains can now be collated into one single suffix. Performance is still good, although I have yet to run a dedicated benchmark.

[sonertari]

Hi @trifle, when you say you want to whitelist at TLD level I guess for example you mean a suffix like .tr, so that all tr domains will be passed through, right? For example, *.google.com.tr will be passed through as well as example.com.tr, right?

Would a system/network admin really want to whitelist all of the tr domain? Or would you whitelist .com.tr instead, but isn't it still too large? If you want to whitelist all of google.com.tr, then doesn't *.google.com.tr handle this case, which is supported by my code too? (The Common Name in the google.com.tr certificate is *.google.com.tr, but I haven't checked the other subdomains for it.) Or do you want to handle www.example.com.tr and mail.example.com.tr subdomains in one passsite configuration for .example.com.tr? (But I recall that Google uses very long Comman Names to handle such subdomains in one certificate.)

Important Note: All whitelists in your solution should have a leading ., otherwise you could have false positives, as far as I understand that's how the code at your stackexchange link works (e.g. example.com.tr would match 1example.com.tr and 2example.com.tr too). And in my experience, users are never careful enough.

I am just trying to understand which is better, especially for my SSLproxy project. I guess I should look into many different certificates to figure this out, but it depends on what system/network admins need too.

[trifle]

Hi @sonertari,

(The usual preamble: This is not an "issue", I don't have an issue with the project and am instead hevily indebted, as all users are, to your continuous work on this. We owe you and Daniel thanks!)

I am indeed primarily interested in second-level suffixes (i.e. *.google.com in your case), which are typically the highest level at which one would apply whitelisting of pinned services. Other examples include *.gvt.com, *.googleapis.com and *.googleusercontent.com for essential google/android services, *.icloud.com and *.mzstatic.com for iOS and so on.

DNS is never this simple, of course, with two-part TLDs such as *.ac.uk, *.co.uk and so on. People might also be interested to specifically whitelist individual third- or fourth-level domains. That's why I think it's best to have a flexible approach that simply compares arbitrary suffixes and lets users pass through entry TLDs if they so choose. By the way, a special use case might be if you have an internal DNS set up with a custom (non-existant) domain (such as .myproduction) - in this case you might want to in fact whitelist the entire TLD.

I have to admit that I didn't try to use wildcard-based matching with your code and instead jumped straight into writing and taking apart the simpler (or stupider) approach in an attempt to get a thorough understanding of what I'm running. So excuse if the suffix matching is merely a re-implementation :)

[sonertari]

Thanks @trifle for your comments. No, you are right, my code cannot handle wildcard asterisk. But I was trying to say I have seen very long Common Names in Google (or other) certificates which simply list many subdomains, so that my implementation can match one of them, as if it supports *.google.com. I agree, it would help write fewer rules if we support wildcards in passsite configuration.

(Note on SSLproxy and UTMFW: I am still concerned about user errors that may pass unintended sites through, which would effectively disable deep ssl inspection for those sites.)

[trifle]

You're absolutely right @sonertari, Google has incredibly long CNs - and they span basically all of their essential services! So you will be able to match i.e. youtube domains being used for JS hosting or similar use cases.

And yes, I'm in absolute agreement with your concern on the false positive matches. This is a fundamental issue: In a situation with a trusted IDS (such as UTMFW), you want to fail closed, i.e. disable passthrough if in doubt. My case is more concerned with the other scenario, which is that I want to fail open (i.e. with passthrough) in the case of pinning, because mobile apps otherwise become unusable. It's ultimately a different threat model.