Skip to content

Commit

Permalink
Factorize the region detection inner request and stop retrying reques…
Browse files Browse the repository at this point in the history
…ts that fail the ssl certificate checks
  • Loading branch information
fviard committed Mar 20, 2020
1 parent 4caa653 commit e4b18b1
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 56 deletions.
18 changes: 8 additions & 10 deletions S3/ConnMan.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,12 @@
from urllib.parse import urlparse

from .Config import Config
from .Exceptions import ParameterError
from .Exceptions import ParameterError, S3SSLCertificateError
from .Utils import getBucketFromHostname

if not 'CertificateError' in ssl.__dict__:
class CertificateError(Exception):
pass
else:
CertificateError = ssl.CertificateError

__all__ = [ "ConnMan" ]

__all__ = ["ConnMan"]


class http_connection(object):
Expand Down Expand Up @@ -128,11 +124,13 @@ def match_hostname(self):
cert = self.c.sock.getpeercert()
try:
ssl.match_hostname(cert, self.hostname)
except AttributeError: # old ssl module doesn't have this function
except AttributeError:
# old ssl module doesn't have this function
return
except ValueError: # empty SSL cert means underlying SSL library didn't validate it, we don't either.
except ValueError:
# empty SSL cert means underlying SSL library didn't validate it, we don't either.
return
except CertificateError as e:
except S3CertificateError as e:
if not self.forgive_wildcard_cert(cert, self.hostname):
raise e

Expand Down
13 changes: 13 additions & 0 deletions S3/Exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,26 @@
else:
PY3 = False

## External exceptions

from ssl import SSLError as S3SSLError

try:
from ssl import CertificateError as S3SSLCertificateError
except ImportError:
class S3SSLCertificateError(Exception):
pass


try:
from xml.etree.ElementTree import ParseError as XmlParseError
except ImportError:
# ParseError was only added in python2.7, before ET was raising ExpatError
from xml.parsers.expat import ExpatError as XmlParseError


## s3cmd exceptions

class S3Exception(Exception):
def __init__(self, message = ""):
self.message = S3.Utils.unicodise(message)
Expand Down
73 changes: 29 additions & 44 deletions S3/S3.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
from .S3Uri import S3Uri
from .ConnMan import ConnMan
from .Crypto import (sign_request_v2, sign_request_v4, checksum_sha256_file,
checksum_sha256_buffer, s3_quote, format_param_str)
checksum_sha256_buffer, s3_quote, format_param_str)

try:
from ctypes import ArgumentError
Expand Down Expand Up @@ -1225,24 +1225,37 @@ def _http_403_handler(self, request, response, fn, *args, **kwargs):

raise S3Error(response)

def send_request(self, request, retries = _max_retries):
if request.resource.get('bucket') \
and not request.use_signature_v2() \
and S3Request.region_map.get(request.resource['bucket'],
Config().bucket_location) == "US":
debug("===== Send_request inner request to determine the bucket region =====")
def update_region_inner_request(self, request):
"""Get and update region for the request if needed.
Signature v4 needs the region of the bucket or the request will fail
with the indication of the correct region.
We are trying to avoid this failure by pre-emptively getting the
correct region to use, if not provided by the user.
"""
if request.resource.get('bucket') and not request.use_signature_v2() \
and S3Request.region_map.get(
request.resource['bucket'], Config().bucket_location
) == "US":
debug("===== SEND Inner request to determine the bucket region "
"=====")
try:
s3_uri = S3Uri(u's3://' + request.resource['bucket'])
# "force_us_default" should prevent infinite recursivity because
# it will set the region_map dict.
region = self.get_bucket_location(s3_uri, force_us_default=True)
if region is not None:
S3Request.region_map[request.resource['bucket']] = region
debug("===== END send_request inner request to determine the bucket region (%r) =====",
region)
debug("===== SUCCESS Inner request to determine the bucket "
"region (%r) =====", region)
except Exception as exc:
# Ignore errors, it is just an optimisation, so nothing critical
debug("Error getlocation inner request: %s", exc)
debug("getlocation inner request failure reason: %s", exc)
debug("===== FAILED Inner request to determine the bucket "
"region =====")

def send_request(self, request, retries = _max_retries):
self.update_region_inner_request(request)

request.body = encode_to_s3(request.body)
headers = request.headers
Expand All @@ -1268,6 +1281,10 @@ def send_request(self, request, retries = _max_retries):
attrs = parse_attrs_header(response["headers"]["x-amz-meta-s3cmd-attrs"])
response["s3cmd-attrs"] = attrs
ConnMan.put(conn)
except (S3SSLError, S3SSLCertificateError):
# In case of failure to validate the certificate for a ssl
# connection,no need to retry, abort immediately
raise
except (IOError, Exception) as e:
debug("Response:\n" + pprint.pformat(response))
if ((hasattr(e, 'errno') and e.errno
Expand Down Expand Up @@ -1343,23 +1360,7 @@ def send_request(self, request, retries = _max_retries):
def send_file(self, request, stream, labels, buffer = '', throttle = 0,
retries = _max_retries, offset = 0, chunk_size = -1,
use_expect_continue = None):
if request.resource.get('bucket') \
and not request.use_signature_v2() \
and S3Request.region_map.get(request.resource['bucket'],
Config().bucket_location) == "US":
debug("===== Send_file inner request to determine the bucket region =====")
try:
s3_uri = S3Uri(u's3://' + request.resource['bucket'])
# "force_us_default" should prevent infinite recursivity because
# it will set the region_map dict.
region = self.get_bucket_location(s3_uri, force_us_default=True)
if region is not None:
S3Request.region_map[request.resource['bucket']] = region
debug("===== END Send_file inner request to determine the bucket region (%r) =====",
region)
except Exception as exc:
# Ignore errors, it is just an optimisation, so nothing critical
debug("Error getlocation inner request: %s", exc)
self.update_region_inner_request(request)

if use_expect_continue is None:
use_expect_continue = self.config.use_http_expect
Expand Down Expand Up @@ -1612,23 +1613,7 @@ def send_file_multipart(self, stream, headers, uri, size, extra_label = ""):
return response

def recv_file(self, request, stream, labels, start_position = 0, retries = _max_retries):
if request.resource.get('bucket') \
and not request.use_signature_v2() \
and S3Request.region_map.get(request.resource['bucket'],
Config().bucket_location) == "US":
debug("===== Recv_file inner request to determine the bucket region =====")
try:
s3_uri = S3Uri(u's3://' + request.resource['bucket'])
# "force_us_default" should prevent infinite recursivity because
# it will set the region_map dict.
region = self.get_bucket_location(s3_uri, force_us_default=True)
if region is not None:
S3Request.region_map[request.resource['bucket']] = region
debug("===== END recv_file Inner request to determine the bucket region (%r) =====",
region)
except Exception as exc:
# Ignore errors, it is just an optimisation, so nothing critical
debug("Error getlocation inner request: %s", exc)
self.update_region_inner_request(request)

method_string, resource, headers = request.get_triplet()
filename = stream.stream_name
Expand Down
3 changes: 1 addition & 2 deletions s3cmd
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ except ImportError:
# python2 fallback code
from distutils.spawn import find_executable as which

from ssl import SSLError
import io

def output(message):
Expand Down Expand Up @@ -3131,7 +3130,7 @@ if __name__ == '__main__':
sys.stderr.write("See ya!\n")
sys.exit(EX_BREAK)

except SSLError as e:
except (S3SSLError, S3SSLCertificateError) as e:
# SSLError is a subtype of IOError
error("SSL certificate verification failure: %s" % e)
sys.exit(EX_ACCESSDENIED)
Expand Down

0 comments on commit e4b18b1

Please sign in to comment.