open_socket() leaks DNS requests when proxying

[nerdy-sam]

socks.setdefaultproxy() has a fourth arg for sending DNS requests through the proxy https://github.com/OfflineIMAP/offlineimap/blob/master/offlineimap/imapserver.py#L119

However, even with that fourth arg set to True, using getaddrinfo() bypasses SOCKS5 proxy for dns resolution. This also breaks .onion Tor hidden service resolution. https://github.com/OfflineIMAP/offlineimap/blob/master/offlineimap/imaplibutil.py#L77 When proxying using this method to connect is recommended. s = self.socket() s.connect((self.host, self.port))

Might consider socks.wrapmodule(). https://github.com/Anorov/PySocks/search?utf8=%E2%9C%93&q=wrap

ping @xiaket

[nicolas33]

@xiabet: we are currently late in the release cycle. I won't take patches if they are not trivial. Once the stable will be out, I'll freeze the code so I can do a deep rewrite of the codebase.

[nicolas33]

Document lack of DNS support: 5f0915d06ee3597b

Also, forgot to say that I expect next stable to come soon. Something like a week or two.

[xiaket]

ok, I'll see what I can do asap.

[nicolas33]

I'm updating the plan. I did more more progress and it appears that the refactoring I intend would be too much large and ambitious regarding the codebase. So, I won't do it like what I had in mind and I won't freeze the code.

IOW, there's no more dead line for your contributions. ,-)

[chris001]

Ping @xiaket Preventing DNS leakage could help maintain privacy and connection for users behind the china GFW. Maybe now is the time to add support for RFC8484 secure encrypted DNS lookup (DOH/DOT) . https://github.com/curl/curl/wiki/DNS-over-HTTPS See recent issue #654

[Gu1nness]

This will not allow to use .onion names. The gethostname should just never happen when you are proxying in SOCKS5, it is against the RFC iirc.

[tubaman]

Here's a temporary hack

(offlineimap) user@host:~/sandboxes/offlineimap3 [tubaman]$ git diff master 
diff --git a/offlineimap/imaplibutil.py b/offlineimap/imaplibutil.py
index 13b2299..0a80d21 100644
--- a/offlineimap/imaplibutil.py
+++ b/offlineimap/imaplibutil.py
@@ -94,6 +94,8 @@ class UsefulIMAPMixIn:
             try:
                 # use socket of our own, possibly SOCKS socket.
                 s = self.socket(af, socktype, proto)
+                if hasattr(s, 'proxy'):
+                    sa = (self.host, self.port)
             except socket.error:
                 continue
             try:

[radfish]

@tubaman wait, the DNS leak happens inside socket.getaddrinfo call, so your patch wouldn't fix it

        for res in socket.getaddrinfo(self.host, self.port, af, socket.SOCK_STREAM):
            af, socktype, proto, canonname, sa = res
            try:
                # use socket of our own, possiblly socksified socket.
                s = self.socket(af, socktype, proto)
                if hasattr(s, 'proxy'):
                    sa = (self.host, self.port)

The fix would have to get rid of getaddrinfo call entirely when proxy is enabled, and pass the hostname to the connect method as is.