From 3b44ec11e0823faa26f12e5a76ae139fb9a0d554 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Sun, 15 Sep 2013 21:16:17 -0400 Subject: [PATCH 1/4] Convert JSON dumps output to UTF-8 before sending to Software Secure --- lms/djangoapps/verify_student/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index a7f1caaca73b..485214d88baf 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -500,7 +500,7 @@ def request_message_txt(self): header_txt = "\n".join( "{}: {}".format(h, v) for h,v in sorted(headers.items()) ) - body_txt = json.dumps(body, indent=2, sort_keys=True, ensure_ascii=False) + body_txt = json.dumps(body, indent=2, sort_keys=True, ensure_ascii=False).encode('utf-8') return header_txt + "\n\n" + body_txt @@ -509,7 +509,7 @@ def send_request(self): response = requests.post( settings.VERIFY_STUDENT["SOFTWARE_SECURE"]["API_URL"], headers=headers, - data=json.dumps(body, indent=2, sort_keys=True, ensure_ascii=False) + data=json.dumps(body, indent=2, sort_keys=True, ensure_ascii=False).encode('utf-8') ) log.debug("Sent request to Software Secure for {}".format(self.receipt_id)) log.debug("Headers:\n{}\n\n".format(headers)) From b1be80b8485116e0d3dca85e2ccd655e374d7702 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Mon, 16 Sep 2013 01:59:26 -0400 Subject: [PATCH 2/4] Extend message signing code to work with dicts, lists. --- lms/djangoapps/verify_student/ssencrypt.py | 33 +++++++++++++++++----- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/lms/djangoapps/verify_student/ssencrypt.py b/lms/djangoapps/verify_student/ssencrypt.py index 862c5aa0218e..3a110b8d0463 100644 --- a/lms/djangoapps/verify_student/ssencrypt.py +++ b/lms/djangoapps/verify_student/ssencrypt.py @@ -127,9 +127,7 @@ def generate_signed_message(method, headers_dict, body_dict, access_key, secret_ """ Returns a (message, signature) pair. """ - headers_str = "{}\n\n{}".format(method, header_string(headers_dict)) - body_str = body_string(body_dict) - message = headers_str + body_str + message = signing_format_message(method, headers_dict, body_dict) # hmac needs a byte string for it's starting key, can't be unicode. hashed = hmac.new(secret_key.encode('utf-8'), message, sha256) @@ -139,6 +137,18 @@ def generate_signed_message(method, headers_dict, body_dict, access_key, secret_ message += '\n' return message, signature, authorization_header +def signing_format_message(method, headers_dict, body_dict): + """ + Given a dictionary of headers and a dictionary of the JSON for the body, + will return a str that represents the normalized version of this messsage + that will be used to generate a signature. + """ + headers_str = "{}\n\n{}".format(method, header_string(headers_dict)) + body_str = body_string(body_dict) + message = headers_str + body_str + + return message + def header_string(headers_dict): """Given a dictionary of headers, return a canonical string representation.""" header_list = [] @@ -152,7 +162,7 @@ def header_string(headers_dict): return "".join(header_list) # Note that trailing \n's are important -def body_string(body_dict): +def body_string(body_dict, prefix=""): """ This version actually doesn't support nested lists and dicts. The code for that was a little gnarly and we don't use that functionality, so there's no @@ -160,9 +170,18 @@ def body_string(body_dict): """ body_list = [] for key, value in sorted(body_dict.items()): - if value is None: - value = "null" - body_list.append(u"{}:{}\n".format(key, value).encode('utf-8')) + if isinstance(value, (list, tuple)): + for i, arr in enumerate(value): + if isinstance(arr, dict): + body_list.append(body_string(arr, u"{}.{}.".format(key, i))) + else: + body_list.append(u"{}.{}:{}\n".format(key, i, arr).encode('utf-8')) + elif isinstance(value, dict): + body_list.append(body_string(value, key + ":")) + else: + if value is None: + value = "null" + body_list.append(u"{}{}:{}\n".format(prefix, key, value).encode('utf-8')) return "".join(body_list) # Note that trailing \n's are important From ab6018a0e58bec1125fee7ac1f319cca96e312fa Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Mon, 16 Sep 2013 02:00:30 -0400 Subject: [PATCH 3/4] Replace signature validation with access-key and add logging around Software Secure callbacks. --- lms/djangoapps/verify_student/models.py | 20 +++++++++++++++ lms/djangoapps/verify_student/views.py | 34 ++++++++++++++++++++----- 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index 485214d88baf..5905a9288f24 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -356,6 +356,26 @@ def deny(self, self.status = "denied" self.save() + @status_before_must_be("must_retry", "submitted", "approved", "denied") + def system_error(self, + error_msg, + error_code="", + reviewing_user=None, + reviewing_service=""): + """ + Mark that this attempt could not be completed because of a system error. + Status should be moved to `must_retry`. + """ + if self.status in ["approved", "denied"]: + return # If we were already approved or denied, just leave it. + + self.error_msg = error_msg + self.error_code = error_code + self.reviewing_user = reviewing_user + self.reviewing_service = reviewing_service + self.status = "must_retry" + self.save() + class SoftwareSecurePhotoVerification(PhotoVerification): """ diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index f315c2136fb2..e1ec69f724f7 100644 --- a/lms/djangoapps/verify_student/views.py +++ b/lms/djangoapps/verify_student/views.py @@ -180,21 +180,43 @@ def results_callback(request): settings.VERIFY_STUDENT["SOFTWARE_SECURE"]["API_SECRET_KEY"] ) - if not sig_valid: - return HttpResponseBadRequest(_("Signature is invalid")) + _, access_key_and_sig = headers["Authorization"].split(" ") + access_key = access_key_and_sig.split(":")[0] + + # This is what we should be doing... + #if not sig_valid: + # return HttpResponseBadRequest("Signature is invalid") + + # This is what we're doing until we can figure out why we disagree on sigs + if access_key != settings.VERIFY_STUDENT["SOFTWARE_SECURE"]["API_ACCESS_KEY"]: + return HttpResponseBadRequest("Access key invalid") receipt_id = body_dict.get("EdX-ID") result = body_dict.get("Result") reason = body_dict.get("Reason", "") error_code = body_dict.get("MessageType", "") - attempt = SoftwareSecurePhotoVerification.objects.get(receipt_id=receipt_id) - if result == "PASSED": + try: + attempt = SoftwareSecurePhotoVerification.objects.get(receipt_id=receipt_id) + except SoftwareSecurePhotoVerification.DoesNotExist: + log.error("Software Secure posted back for receipt_id {}, but not found".format(receipt_id)) + return HttpResponseBadRequest("edX ID {} not found".format(receipt_id)) + + if result == "PASS": + log.debug("Approving verification for {}".format(receipt_id)) attempt.approve() - elif result == "FAILED": - attempt.deny(reason, error_code=error_code) + elif result == "FAIL": + log.debug("Denying verification for {}".format(receipt_id)) + attempt.deny(json.dumps(reason), error_code=error_code) elif result == "SYSTEM FAIL": + log.debug("System failure for {} -- resetting to must_retry".format(receipt_id)) + attempt.system_error(json.dumps(reason), error_code=error_code) log.error("Software Secure callback attempt for %s failed: %s", receipt_id, reason) + else: + log.error("Software Secure returned unknown result {}".format(result)) + return HttpResponseBadRequest( + "Result {} not understood. Known results: PASS, FAIL, SYSTEM FAIL".format(result) + ) return HttpResponse("OK!") From 99841be6044a9d6bad7c7de4be6d55811df60a2e Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Mon, 16 Sep 2013 10:23:31 -0400 Subject: [PATCH 4/4] Update docstring on ssencrypt.body_string to be more useful. --- lms/djangoapps/verify_student/ssencrypt.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/verify_student/ssencrypt.py b/lms/djangoapps/verify_student/ssencrypt.py index 3a110b8d0463..aefb4292a0c0 100644 --- a/lms/djangoapps/verify_student/ssencrypt.py +++ b/lms/djangoapps/verify_student/ssencrypt.py @@ -164,9 +164,9 @@ def header_string(headers_dict): def body_string(body_dict, prefix=""): """ - This version actually doesn't support nested lists and dicts. The code for - that was a little gnarly and we don't use that functionality, so there's no - real test for correctness. + Return a canonical string representation of the body of a JSON request or + response. This canonical representation will be used as an input to the + hashing used to generate a signature. """ body_list = [] for key, value in sorted(body_dict.items()):