OfflineIMAP / offlineimap3

Read/sync your IMAP mailboxes (python3)
Other
455 stars 64 forks source link

imap: fail with error if cannot honor proxy config #162

Closed radfish closed 1 year ago

radfish commented 1 year ago

Problem: The user might configure a proxy, but this this proxy is silently bypassed in the cases listed below. This breaks the user's privacy, since the connection to the IMAP server is leaked with the user's IP (confirmed leak with netstat showing connection to IMAP server, no connection to the proxy at all). The proxy is pypassed when:

  1. the user leaves the ipv6 config option unset
  2. the user makes an error in the proxy/authproxy config option
  3. the PySocks dependency is not installed

Cause for case 1: The code in imaplibutil.py in open_socket() (overriden imaplib2.IMPA4) that follows two different paths depending on the protocol (AF_UNSET vs AF_INIT or AF_INET6). In case of AF_UNSET, the code calls rfc6555.create_connection() which creates a new socket instead of using the existing proxied socket that was passed by the caller, thus bypassing the proxy.

Fix: raise an error in all cases when the proxy would not be honored, for Case 1 check the AF_UNSET, for Cases 2 and 3 change the warnings into errors fatal to that account processing.

Also, explain the requirement to set ipv6 option in the documentation for the proxy option in the config template file. Also, add some extra information about remote DNS request via proxy and that only IPv4 supported for proxy address. Also, change option values from True/False to yes/no for consistency with other options. Also, change example port to 1080, which is the standard port for a SOCKS proxy.

This commit does not fix the DNS request leaks from getaddrinfo() calls, tracked in Issue #189.

This PR

Add character x [x].

References

Related Issue #189

Additional information

Tested cases:

  1. no ipv6 in config:
ERROR: Account 'xxx': cannot use proxy without protocol choice ('ipv6' config must be set to yes or no)
Traceback:
  ...
  File "/usr/lib/python3.11/site-packages/offlineimap/imapserver.py", line 133, in __init__
    self.proxied_socket = self._get_proxy('proxy', socket.socket)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/offlineimap/imapserver.py", line 154, in _get_proxy
    raise OfflineImapError("Account '%s': "
  1. PySocks dependency not installed
ERROR: Account 'xxx': cannot use proxy: PySocks not found: No module named 'socks'
Traceback:
  ...
  File "/usr/lib/python3.11/site-packages/offlineimap/imapserver.py", line 133, in __init__
    self.proxied_socket = self._get_proxy('proxy', socket.socket)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/offlineimap/imapserver.py", line 175, in _get_proxy
    six.reraise(OfflineImapError,
  ...
  File "/usr/lib/python3.11/site-packages/offlineimap/imapserver.py", line 167, in _get_proxy
    import socks
  1. syntax error in config value:
ERROR: Exceptions occurred during the run!
ERROR: Account 'xxx': invalid 'proxy' config: 'SOCKS5:10.0.0.1:1080:44': too many values to unpack (expected 3
)
Traceback:
 ...
  File "/usr/lib/python3.11/site-packages/offlineimap/imapserver.py", line 181, in _get_proxy
    six.reraise(OfflineImapError,  
  ...
  File "/usr/lib/python3.11/site-packages/offlineimap/imapserver.py", line 168, in _get_proxy
    proxy_type, host, port = proxy.split(":")

Note: next branch is broken for me for unrelated reasons. I was able to test these specific error code paths on that branch, but the sync does not complete due to unrelated exceptions.

thekix commented 1 year ago

Hi @radfish

Thanks a lot for your patch. I am checking the pull-requests and this patch is for the next branch. This branch is too old (python 2 code), but this patch probably could be used in the master branch. We should include it in the master branch?

I am closing the next branch, and this patch will be closed. We can continue commenting here of if you think we should include in master, please, open a new pull-request.

Best regards, kix