Skip to content

Reject blank passwords; use SSLContext for all urllib.request.urlopen requests #321

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 35 additions & 16 deletions emailproxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -1060,9 +1060,11 @@ def construct_oauth2_permission_url(permission_url, redirect_uri, client_id, sco
def start_device_authorisation_grant(permission_url):
"""Requests the device authorisation grant flow URI and user code - see https://tools.ietf.org/html/rfc8628"""
try:
ssl_context = ssl.create_default_context(purpose=ssl.Purpose.SERVER_AUTH)
ssl_context.minimum_version = ssl.TLSVersion.TLSv1_2 # GitHub CodeQL issue 2
response = urllib.request.urlopen(
urllib.request.Request(permission_url, headers={'User-Agent': APP_NAME}),
timeout=AUTHENTICATION_TIMEOUT).read()
timeout=AUTHENTICATION_TIMEOUT, context=ssl_context).read()
parsed_result = json.loads(response)
verification_uri = parsed_result.get('verification_uri_complete', parsed_result['verification_uri'])
user_code = parsed_result['user_code']
Expand Down Expand Up @@ -1187,10 +1189,14 @@ def get_oauth2_authorisation_tokens(token_url, redirect_uri, client_id, client_s
expires_at = time.time() + expires_in
while time.time() < expires_at and not EXITING:
try:
ssl_context = ssl.create_default_context(purpose=ssl.Purpose.SERVER_AUTH)
ssl_context.minimum_version = ssl.TLSVersion.TLSv1_2 # GitHub CodeQL issue 2

# in all flows except DAG, we make one attempt only
response = urllib.request.urlopen(
urllib.request.Request(token_url, data=urllib.parse.urlencode(params).encode('utf-8'),
headers={'User-Agent': APP_NAME}), timeout=AUTHENTICATION_TIMEOUT).read()
headers={'User-Agent': APP_NAME}), timeout=AUTHENTICATION_TIMEOUT,
context=ssl_context).read()
return json.loads(response)

except urllib.error.HTTPError as e:
Expand Down Expand Up @@ -1262,9 +1268,12 @@ def refresh_oauth2_access_token(token_url, client_id, client_secret, jwt_client_
params['client_assertion'] = jwt_client_assertion

try:
ssl_context = ssl.create_default_context(purpose=ssl.Purpose.SERVER_AUTH)
ssl_context.minimum_version = ssl.TLSVersion.TLSv1_2 # GitHub CodeQL issue 2
response = urllib.request.urlopen(
urllib.request.Request(token_url, data=urllib.parse.urlencode(params).encode('utf-8'),
headers={'User-Agent': APP_NAME}), timeout=AUTHENTICATION_TIMEOUT).read()
headers={'User-Agent': APP_NAME}), timeout=AUTHENTICATION_TIMEOUT,
context=ssl_context).read()
token = json.loads(response)
if 'expires_in' in token: # some servers return integer values as strings - fix expiry values (GitHub #237)
token['expires_in'] = int(token['expires_in'])
Expand Down Expand Up @@ -1678,22 +1687,32 @@ def process_data(self, byte_data, censor_server_log=False):
super().process_data(byte_data)

def authenticate_connection(self, username, password, command='login'):
success, result = OAuth2Helper.get_oauth2_credentials(username, password)
if success:
# send authentication command to server (response checked in ServerConnection)
# note: we only support single-trip authentication (SASL) without checking server capabilities - improve?
super().process_data(b'%s AUTHENTICATE XOAUTH2 ' % self.authentication_tag.encode('utf-8'))
super().process_data(b'%s\r\n' % OAuth2Helper.encode_oauth2_string(result), censor_server_log=True)

# because get_oauth2_credentials blocks, the server could have disconnected, and may no-longer exist
if self.server_connection:
self.server_connection.authenticated_username = username

self.reset_login_state()
if not success:
if not password:
# we can't actually check credentials here, but if password is missing, it's possible that
# the client want to ask for credentials and so tried without password first, in the hope
# that the server will handle it, which it won't for XOAUTH2 - intercept that here and mimic old behavior
self.reset_login_state()
result = '%s: Login failed - the password for account %s is incorrect' % (APP_NAME, username)
error_message = '%s NO %s %s\r\n' % (self.authentication_tag, command.upper(), result)
self.authentication_tag = None
self.send(error_message.encode('utf-8'))
else:
success, result = OAuth2Helper.get_oauth2_credentials(username, password)
if success:
# send authentication command to server (response checked in ServerConnection)
# note: we only support single-trip authentication (SASL) without checking server capabilities - improve?
super().process_data(b'%s AUTHENTICATE XOAUTH2 ' % self.authentication_tag.encode('utf-8'))
super().process_data(b'%s\r\n' % OAuth2Helper.encode_oauth2_string(result), censor_server_log=True)

# because get_oauth2_credentials blocks, the server could have disconnected, and may no-longer exist
if self.server_connection:
self.server_connection.authenticated_username = username

self.reset_login_state()
if not success:
error_message = '%s NO %s %s\r\n' % (self.authentication_tag, command.upper(), result)
self.authentication_tag = None
self.send(error_message.encode('utf-8'))


class POPOAuth2ClientConnection(OAuth2ClientConnection):
Expand Down