-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Verify MD5 is available #807
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
On some systems, such as a FIPS enabled system, MD5 hashing is unavailable. This commit will detect when that is the case, avoiding it where possible and raising a better error otherwise.
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -237,3 +237,17 @@ def total_seconds(delta): | |
day_in_seconds = delta.days * 24 * 3600.0 | ||
micro_in_seconds = delta.microseconds / 10.0**6 | ||
return day_in_seconds + delta.seconds + micro_in_seconds | ||
|
||
|
||
def md5_available(): | ||
""" | ||
Checks to see if md5 is available on this system. A given system might not | ||
have access to it for various reasons, such as FIPS mode being enabled. | ||
:return: True if md5 is available. False if not. | ||
""" | ||
try: | ||
import hashlib | ||
hashlib.md5() | ||
return True | ||
except: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer to check the specific Exception here ( |
||
return False |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,13 +25,13 @@ | |
import warnings | ||
|
||
from botocore.compat import urlsplit, urlunsplit, unquote, \ | ||
json, quote, six, unquote_str, ensure_bytes | ||
json, quote, six, unquote_str, ensure_bytes, md5_available | ||
from botocore.docs.utils import AutoPopulatedParam | ||
from botocore.docs.utils import HideParamFromOperations | ||
from botocore.docs.utils import AppendParamDocumentation | ||
from botocore.signers import add_generate_presigned_url | ||
from botocore.signers import add_generate_presigned_post | ||
from botocore.exceptions import ParamValidationError | ||
from botocore.exceptions import ParamValidationError, MD5UnavailableError | ||
from botocore.exceptions import UnsupportedTLSVersionWarning | ||
from botocore.utils import percent_encode, SAFE_CHARS | ||
|
||
|
@@ -122,6 +122,9 @@ def json_decode_template_body(parsed, **kwargs): | |
|
||
|
||
def calculate_md5(params, **kwargs): | ||
if not md5_available(): | ||
raise MD5UnavailableError() | ||
|
||
request_dict = params | ||
if request_dict['body'] and 'Content-MD5' not in params['headers']: | ||
body = request_dict['body'] | ||
|
@@ -150,7 +153,7 @@ def _calculate_md5_from_file(fileobj): | |
def conditionally_calculate_md5(params, **kwargs): | ||
"""Only add a Content-MD5 when not using sigv4""" | ||
signer = kwargs['request_signer'] | ||
if signer.signature_version not in ['v4', 's3v4']: | ||
if signer.signature_version not in ['v4', 's3v4'] and md5_available(): | ||
calculate_md5(params, **kwargs) | ||
|
||
|
||
|
@@ -188,6 +191,9 @@ def copy_source_sse_md5(params, **kwargs): | |
def _sse_md5(params, sse_member_prefix='SSECustomer'): | ||
if not _needs_s3_sse_customization(params, sse_member_prefix): | ||
return | ||
if not md5_available(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than require each call site to check if md5 is available before creating checksums, can we just have a compat function that either calculate the md5 if possible or raises an exception if not available? I'd rather have this logic once rather than throughout the code base. It'll be easier to maintain. |
||
raise MD5UnavailableError() | ||
|
||
sse_key_member = sse_member_prefix + 'Key' | ||
sse_md5_member = sse_member_prefix + 'KeyMD5' | ||
key_as_bytes = params[sse_key_member] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to check this each time. It's either available on the system or not. I'd prefer to just move this to a constant in this module.