Add support for "old" style imap over tls#1
Conversation
| type = radicale_imap | ||
| imap_host = imap.server.tld | ||
| imap_secure = True | ||
| imaps = True # for "old"-style imap over tls |
There was a problem hiding this comment.
Change True to False. I think IMAP over SSL is better.
There was a problem hiding this comment.
I probably named this badly.
What I mean with "old-style" is that it uses a dedicated port for imap over SSL/TLS, which I consider to be more secure than upgrading with STARTTLS.
The proper imap over SSL/TLS is not even supported without this patch.
There was a problem hiding this comment.
I just wanted you to change the line to something like:
imaps = False # for "old"-style IMAP over SSL
radicale_imap/__init__.py
Outdated
| def get_secure_connection(self, host, port, imaps): | ||
| try: | ||
| if imaps: | ||
| connection = imaplib.IMAP4_SSL(host=host, port=port) |
There was a problem hiding this comment.
The hostname is not checked. This is very insecure.
Enable the check if secure is true.
radicale_imap/__init__.py
Outdated
| if imaps: | ||
| connection = imaplib.IMAP4_SSL(host=host, port=port) | ||
| else: | ||
| connection = self.get_connection(host, port) |
There was a problem hiding this comment.
All of the above must not be inside of the try-except-block. If secure is false exceptions are ignored.
radicale_imap/__init__.py
Outdated
| imaps = False | ||
| if self.configuration.has_option("auth", "imaps"): | ||
| imaps = self.configuration.getboolean("auth", "imaps") | ||
|
|
radicale_imap/__init__.py
Outdated
| if sys.version_info < (3, 4) and secure: | ||
| raise RuntimeError("Secure IMAP is not availabe in Python < 3.4") | ||
| try: | ||
| connection = imaplib.IMAP4(host=address, port=port) |
There was a problem hiding this comment.
Just do something like this, instead of creating two methods:
if imaps and secure:
connection = imaplib.IMAP4_SSL(...
elif imaps:
connection = imaplib.IMAP4_SSL(...
else:
connection = imaplib.IMAP4(host=address, port=port)
try:
...
radicale_imap/__init__.py
Outdated
| if secure or imaps: | ||
| connection = self.get_secure_connection(address, port, imaps) | ||
| else: | ||
| connection = self.get_connection(address, port) |
There was a problem hiding this comment.
If secure is false we can still try to use STARTTLS opportunistically, to protect against passive monitoring.
There was a problem hiding this comment.
I strongly disagree, this is a tool for people who know what they are doing. If I explicitly disable transport encryption I expect that it honors that, the current implementation is broken btw, it fails if STARTTLS is not available.
If at all opportunistic encryption should have it's own switch.
| type = radicale_imap | ||
| imap_host = imap.server.tld | ||
| imap_secure = True | ||
| imaps = True # for "old"-style imap over tls |
There was a problem hiding this comment.
I just wanted you to change the line to something like:
imaps = False # for "old"-style IMAP over SSL
|
|
||
| def get_connection(self, host, port, secure, imaps): | ||
| ssl_context = ssl.create_default_context() | ||
| try: |
There was a problem hiding this comment.
This try-except-block is unnecessary,
| """ | ||
|
|
||
| def get_connection(self, host, port, secure, imaps): | ||
| ssl_context = ssl.create_default_context() |
There was a problem hiding this comment.
This fails with Python < 3.4. Only create the context when secure.
| try: | ||
| if secure and imaps: | ||
| connection = imaplib.IMAP4_SSL( | ||
| host=host, port=port, ssl_context=ssl_context) |
There was a problem hiding this comment.
The case of insecure imaps (without hostname checks) is missing:
elif imaps:
connection = imaplib.IMAP4_SSL(host=host, port=port)Otherwise, it's impossible to use it with Python < 3.4 or with self-signed certificates.
| host=host, port=port, ssl_context=ssl_context) | ||
| elif secure: | ||
| connection = imaplib.IMAP4(host=host, port=port) | ||
| if sys.version_info < (3, 4): |
There was a problem hiding this comment.
This case can't happen. You can't use secure with Python < 3.4.
| else: | ||
| connection.starttls(ssl_context) | ||
| else: | ||
| connection = imaplib.IMAP4(host=host, port=port) |
There was a problem hiding this comment.
Why did you remove opportunistic encryption?
2 participants
No description provided.