Use java.security classes in preference to string-wrappers
#147
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rtyley
changed the title
java.security classes in preference to string-wrappers
rtyley
force-pushed
the
use-java.security-classes-in-preference-to-string-wrappers
branch
4 times, most recently
from
c692194
to
ff3b46f
Compare
rtyley
commented
Jun 28, 2024
Comment on lines
-28
to
+35
| val privateKey = keyFactory.generatePrivate(new PKCS8EncodedKeySpec(decodeBase64(prvKey.key))) | ||
| rsa.initSign(privateKey) | ||
| rsa.initSign(prvKey) | ||
|
|
||
| rsa.update(data) | ||
| rsa.sign() | ||
| } | ||
|
|
||
| def verifySignature(data: Array[Byte], signature: Array[Byte], pubKey: PublicKey) : Boolean = { | ||
| val rsa = Signature.getInstance(signatureAlgorithm, "BC") | ||
| val publicKey = keyFactory.generatePublic(new X509EncodedKeySpec(decodeBase64(pubKey.key))) | ||
| rsa.initVerify(publicKey) | ||
| rsa.initVerify(pubKey) |
There was a problem hiding this comment.
The signData() &verifySignature() methods on Crypto no longer need to repeatedly decode the base64-encoded strings, and so now are a bit simpler and more efficient.
rtyley
commented
Jun 28, 2024
Comment on lines
-6
to
+8
| publicKey: PublicKey, | ||
| privateKey: PrivateKey, | ||
| signingKeyPair: KeyPair, |
There was a problem hiding this comment.
The signature of the PanDomainAuthSettings case class has changed here, but thankfully this won't affect consumers of this library, as they all use the companion apply() method that accepts settingMap: Map[String, String].
This change deletes the `PublicKey` & `PrivateKey` classes in our `com.gu.pandomainauth` package, which were simple string-wrapper classes that held base64-encoded text representations of cryptographic keys. The `java.security` classes `PublicKey` & `PrivateKey` already exist and, as they represent fully-decoded keys, don't need to be _repeatedly_ decoded from strings the way we had to with our string-wrapper classes. Consequently, the `signData()` & `verifySignature()` methods on `Crypto` are now a bit simpler and more efficient. We can also use the `java.security.KeyPair` class to represent the pair of public & private keys.
rtyley
force-pushed
the
use-java.security-classes-in-preference-to-string-wrappers
branch
from
ff3b46f
to
72601f7
Compare
rtyley
added a commit
to guardian/login.gutools
that referenced
this pull request
Aug 9, 2024
This is pretty minor change, but due to guardian/pan-domain-authentication#147 and guardian/pan-domain-authentication#153, there are a couple of changes required: * imports for `PublicKey` *
rtyley
added a commit
to guardian/login.gutools
that referenced
this pull request
Aug 9, 2024
This is pretty minor change, but due to guardian/pan-domain-authentication#147 and guardian/pan-domain-authentication#153, there are a couple of changes required: * imports for `PublicKey` * `settings.privateKey` becomes `settings.signingKeyPair.getPrivate`
rtyley
added a commit
to guardian/login.gutools
that referenced
this pull request
Aug 9, 2024
This is pretty minor change, but due to guardian/pan-domain-authentication#147 and guardian/pan-domain-authentication#153, there are a couple of changes required: * imports for `PublicKey` * `settings.privateKey` becomes `settings.signingKeyPair.getPrivate`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change deletes the
PublicKey&PrivateKeyclasses in ourcom.gu.pandomainauthpackage, which were simple string-wrapper classes that held base64-encoded text representations of cryptographic keys.The
java.securityclassesPublicKey&PrivateKeyalready exist and, as they represent fully-decoded keys, don't need to be repeatedly decoded from strings the way we had to with our string-wrapper classes. Consequently, thesignData()&verifySignature()methods onCryptoare now a bit simpler and more efficient.We can also use the
java.security.KeyPairclass to represent the pair of public & private keys.