Remaining fixes to allow BouncyCastle to be swapped out with other implementations. #131
Conversation
*Issue #, if available:* #41 *Description of changes:* Removes explicit use of BouncyCastle for deriving an HMAC in `CryptoAlgorithm`. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. # Check any applicable: - [ ] Were any files moved? Moving files changes their URL, which breaks all hyperlinks to the files.
*Description of changes:* Remaining fixes to allow BouncyCastle to be swapped out with other implementations. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. # Check any applicable: - [ ] Were any files moved? Moving files changes their URL, which breaks all hyperlinks to the files.
… into nobc � Conflicts: � src/main/java/com/amazonaws/encryptionsdk/CryptoAlgorithm.java
| @@ -268,7 +268,7 @@ public static ByteBuffer limit(final ByteBuffer buff, final int newLimit) { | |||
| * @return decoded data as a byte array | |||
| */ | |||
| public static byte[] decodeBase64String(final String encoded) { | |||
| return Base64.decode(encoded); | |||
| return encoded.isEmpty() ? new byte[0] : Base64.decode(encoded); | |||
There was a problem hiding this comment.
We should already have a constant defined for a zero-byte array. Return that instead. Otherwise you create needless memory pressure.
There was a problem hiding this comment.
I didn't see a constant defined in Constants.java, so I just used the one in Apache Commons Lang ArrayUtils. If you prefer I can add the constant to Constants.java
| import sun.security.x509.AlgorithmId; | ||
| import sun.security.x509.CertificateAlgorithmId; | ||
| import sun.security.x509.CertificateSerialNumber; | ||
| import sun.security.x509.CertificateValidity; | ||
| import sun.security.x509.CertificateX509Key; | ||
| import sun.security.x509.X500Name; | ||
| import sun.security.x509.X509CertImpl; | ||
| import sun.security.x509.X509CertInfo; |
There was a problem hiding this comment.
Is there any way to do this without importing sun. packages. This may be test code but it's a pretty severe anti-pattern.
There was a problem hiding this comment.
Standard BC and FIPS BC have different APIs for certificate generation, and the two packages are not compatible with each other so we can't just add a new test dependency. I added a comment about the usage of the sun packages.
This method passes the compatibility tests using either standard BC or FIPS BC
Issue #, if available: #41
Description of changes:
Remaining fixes to allow BouncyCastle to be swapped out with other implementations.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Check any applicable: