Add support for ECDSA signed CloudFront URLs #6627
Conversation
alextwoods
added
the
no-api-surface-area-change
...cloudfront/src/main/java/software/amazon/awssdk/services/cloudfront/CloudFrontUtilities.java
Outdated
Show resolved
Hide resolved
| try { | ||
| return KeyFactory.getInstance("RSA").generatePrivate(privateKeySpec); | ||
| } catch (InvalidKeySpecException rsaFail) { | ||
| return KeyFactory.getInstance("EC").generatePrivate(privateKeySpec); |
There was a problem hiding this comment.
Should we add a debug statement mentioning falling back to EC?
There was a problem hiding this comment.
Yeah, good question - I'm a little on the fence about this code in general here. Ideally we would use the EncodedKeySpec#getAlgorithm method, but its not available until Java 9 :-(. So I'd say conceptually this is a little different than a fall back.
What I did do is add an additional catch(InvalidKeySpecException) in the privateKeyFromPkcs8 method that calls this that makes it more clear that the given private key file wasn't one of the supported formats.
What do you think? I'm happy to add logging, but I think it might be a little confusing to see "falling back to EC" when thats what you intended by passing an EC private key?
There was a problem hiding this comment.
Gotcha, makes sense. I see the new revision added improved error message, lgtm!
|
2 participants
Add support for ECDSA signed CloudFront URLs
Motivation and Context
Fixes: #6543
See also:
Modifications
Q: Why not make this more generic and support possible future algorithms?
A: The signing mechanism for Cloudfront signed urls requires both knowing the key type (RSA vs EC) and the hash function to use. We cannot derive the required hash function from the PrivateKey. If additional algorithms are supported in the future, its possible/likely that they will choose to use more modern/secure hashes like SHA256 - however, we don't have that knowledge now. The code in this PR attempts to make the code more generic to handle possible future algorithms more easily, but doesn't add support for anything that isn't supported in CloudFront today.
Testing
New and existing unit and integration tests. (Note: Integration tests confirm usage of
SHA1withECDSAis supported service side).Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith yourLicense