Using SHA256 and PSS padding #722
Conversation
| @@ -75,9 +75,10 @@ def _parse_pfx(pfx_path, passphrase_bytes): | |||
| x5c = [ | |||
| '\n'.join(cert_pem.splitlines()[1:-1]) # Strip the "--- header ---" and "--- footer ---" | |||
| ] | |||
| sha256_thumbprint = cert.fingerprint(hashes.SHA256()).hex() # cryptography 0.7+ | |||
There was a problem hiding this comment.
Can you add the below checks to this function so that you're only returning one thumbprint?
There was a problem hiding this comment.
You mean a test case? We do not currently have such a test case, but its utilization is in this part of the PR. You can see that we pass in only one of the thumbprints, not both.
| } | ||
| else: # Fall back | ||
| if not sha1_thumbprint: | ||
| raise ValueError("You shall provide a thumbprint in SHA1.") |
There was a problem hiding this comment.
Are the SHA thumbprints something that the developer is meant to provide, and would they see this error message if something went wrong? If so, you may want something more informative like "A SHA1 thumbprint is needed for ADFS" or similar to let them know which SHA version is needed for each flow.
There was a problem hiding this comment.
When an app developer encounters this error message, he or she is already in the ADFS code path, so the "you shall provide a thumbprint in SHA1" is good enough as an actionable item.
I incline to avoid mentioning too-specific info such as "ADFS" in this case, because that kind of wording tends to become outdated when/if we will add or remove some scenarios into this SHA1 group.
No new API is added in this PR. The SHA256 behavior will automatically kick in when using the recent .pfx support AND when the authority is not ADFS.
Manually tested and saw the
x5t#S256and the"alg": "PS256"visible in the header. All the existing E2E test cases are now using this new SHA256 behavior, and they still pass.Also manually tested the SHA1 code path and it still works. (Did not add it as automated test case because currently there is no public API to control the opt-in and opt-out.)