Merge pull request #147 from guardian/use-java.security-classes-in-pr…

…eference-to-string-wrappers

Use `java.security` classes in preference to string-wrappers

pan-domain-auth-core/src/main/scala/com/gu/pandomainauth/model/PanDomainAuthSettings.scala

@@ -1,10 +1,11 @@
 package com.gu.pandomainauth.model
 
-import com.gu.pandomainauth.{PrivateKey, PublicKey}
+import com.gu.pandomainauth.service.Crypto
+
+import java.security.KeyPair
 
 case class PanDomainAuthSettings(
-  publicKey: PublicKey,
-  privateKey: PrivateKey,
+  signingKeyPair: KeyPair,
   cookieSettings: CookieSettings,
   oAuthSettings: OAuthSettings,
   google2FAGroupSettings: Option[Google2FAGroupSettings]
@@ -53,8 +54,7 @@ object PanDomainAuthSettings{
     }
 
     PanDomainAuthSettings(
-      PublicKey(settingMap("publicKey")),
-      PrivateKey(settingMap("privateKey")),
+      Crypto.keyPairFrom(settingMap),
       cookieSettings,
       oAuthSettings,
       google2faSettings

pan-domain-auth-play/src/main/scala/com/gu/pandomainauth/action/Actions.scala

@@ -198,7 +198,7 @@ trait AuthActions {
   }
 
   def readAuthenticatedUser(request: RequestHeader): Option[AuthenticatedUser] = readCookie(request) map { cookie =>
-    CookieUtils.parseCookieData(cookie.cookie.value, settings.publicKey)
+    CookieUtils.parseCookieData(cookie.cookie.value, settings.signingKeyPair.getPublic)
   }
 
   def readCookie(request: RequestHeader): Option[PandomainCookie] = {
@@ -211,7 +211,7 @@ trait AuthActions {
   def generateCookie(authedUser: AuthenticatedUser): Cookie =
     Cookie(
       name = settings.cookieSettings.cookieName,
-      value = CookieUtils.generateCookieData(authedUser, settings.privateKey),
+      value = CookieUtils.generateCookieData(authedUser, settings.signingKeyPair.getPrivate),
       domain = Some(domain),
       secure = true,
       httpOnly = true
@@ -237,7 +237,7 @@ trait AuthActions {
     */
   def extractAuth(request: RequestHeader): AuthenticationStatus = {
     readCookie(request).map { cookie =>
-      PanDomain.authStatus(cookie.cookie.value, settings.publicKey, validateUser, apiGracePeriod, system, cacheValidation, cookie.forceExpiry)
+      PanDomain.authStatus(cookie.cookie.value, settings.signingKeyPair.getPublic, validateUser, apiGracePeriod, system, cacheValidation, cookie.forceExpiry)
     } getOrElse NotAuthenticated
   }
 

pan-domain-auth-verification/src/main/scala/com/gu/pandomainauth/PanDomain.scala

@@ -3,6 +3,8 @@ package com.gu.pandomainauth
 import com.gu.pandomainauth.model._
 import com.gu.pandomainauth.service.CookieUtils
 
+import java.security.PublicKey
+
 
 object PanDomain {
   /**

pan-domain-auth-verification/src/main/scala/com/gu/pandomainauth/PublicSettings.scala

@@ -2,10 +2,13 @@ package com.gu.pandomainauth
 
 import java.util.concurrent.atomic.AtomicReference
 import java.util.concurrent.{Executors, ScheduledExecutorService, TimeUnit}
-
 import com.amazonaws.services.s3.AmazonS3
+import com.gu.pandomainauth.PublicSettings.validateAndParseKeyText
+import com.gu.pandomainauth.service.Crypto
 import org.slf4j.LoggerFactory
 
+import java.security.PublicKey
+import java.util.regex.Pattern
 import scala.concurrent.ExecutionContext
 import scala.concurrent.duration._
 
@@ -63,13 +66,11 @@ object PublicSettings {
     fetchSettings(settingsFileKey, bucketName, s3Client) flatMap extractSettings flatMap extractPublicKey
   }
 
-  private[pandomainauth] def extractPublicKey(settings: Map[String, String]): Either[SettingsFailure, PublicKey] = for {
-    rawKey <- settings.get("publicKey").toRight(PublicKeyNotFoundFailure)
-    publicKey <- validateKey(PublicKey(rawKey))
-  } yield publicKey
+  private[pandomainauth] def extractPublicKey(settings: Map[String, String]): Either[SettingsFailure, PublicKey] =
+    settings.get("publicKey").toRight(PublicKeyNotFoundFailure).flatMap(validateAndParseKeyText)
 
-  private[pandomainauth] def validateKey(pubKey: PublicKey): Either[SettingsFailure, PublicKey] = {
-    if ("[a-zA-Z0-9+/\n]+={0,3}".r.pattern.matcher(pubKey.key).matches) Right(pubKey)
-    else Left(PublicKeyFormatFailure)
-  }
+  private val KeyPattern: Pattern = "[a-zA-Z0-9+/\n]+={0,3}".r.pattern
+
+  private[pandomainauth] def validateAndParseKeyText(pubKeyText: String): Either[SettingsFailure, PublicKey] =
+    Either.cond(KeyPattern.matcher(pubKeyText).matches, Crypto.publicKeyFor(pubKeyText), PublicKeyFormatFailure)
 }

pan-domain-auth-verification/src/main/scala/com/gu/pandomainauth/service/CookieUtils.scala

@@ -1,8 +1,6 @@
 package com.gu.pandomainauth.service
 
-import java.security.SignatureException
-
-import com.gu.pandomainauth.{PrivateKey, PublicKey}
+import java.security.{PrivateKey, PublicKey, SignatureException}
 import com.gu.pandomainauth.model.{AuthenticatedUser, CookieParseException, CookieSignatureInvalidException, User}
 import org.apache.commons.codec.binary.Base64
 

pan-domain-auth-verification/src/main/scala/com/gu/pandomainauth/service/Crypto.scala

@@ -1,12 +1,11 @@
 package com.gu.pandomainauth.service
 
-import java.security.spec.{PKCS8EncodedKeySpec, X509EncodedKeySpec}
-import java.security.{KeyFactory, Security, Signature}
-
-import com.gu.pandomainauth.{PrivateKey, PublicKey}
 import org.apache.commons.codec.binary.Base64._
 import org.bouncycastle.jce.provider.BouncyCastleProvider
 
+import java.security.spec.{PKCS8EncodedKeySpec, X509EncodedKeySpec}
+import java.security._
+
 
 object Crypto {
   /**
@@ -25,19 +24,25 @@ object Crypto {
 
   def signData(data: Array[Byte], prvKey: PrivateKey): Array[Byte] = {
     val rsa = Signature.getInstance(signatureAlgorithm, "BC")
-    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)
 
     rsa.update(data)
     rsa.verify(signature)
   }
+
+  def publicKeyFor(base64EncodedKey: String): PublicKey =
+    keyFactory.generatePublic(new X509EncodedKeySpec(decodeBase64(base64EncodedKey)))
+  def privateKeyFor(base64EncodedKey: String): PrivateKey =
+    keyFactory.generatePrivate(new PKCS8EncodedKeySpec(decodeBase64(base64EncodedKey)))
+
+  def keyPairFrom(settingMap: Map[String,String]): KeyPair =
+    new KeyPair(publicKeyFor(settingMap("publicKey")), privateKeyFor(settingMap("privateKey")))
 }

pan-domain-auth-verification/src/test/scala/com/gu/pandomainauth/PanDomainTest.scala

@@ -20,22 +20,22 @@ class PanDomainTest extends AnyFreeSpec with Matchers with Inside {
     cacheValidation: Boolean = false,
     forceExpiry: Boolean = false,
   ) = {
-    PanDomain.authStatus(cookieData, testPublicKey, validateUser, apiGracePeriod, system, cacheValidation, forceExpiry)
+    PanDomain.authStatus(cookieData, testPublicKey.key, validateUser, apiGracePeriod, system, cacheValidation, forceExpiry)
   }
 
   "authStatus" - {
     val authUser = AuthenticatedUser(User("test", "user", "test.user@example.com", None), "testsuite", Set("testsuite"), new Date().getTime + 86400, multiFactor = true)
-    val validCookieData = CookieUtils.generateCookieData(authUser, testPrivateKey)
+    val validCookieData = CookieUtils.generateCookieData(authUser, testPrivateKey.key)
 
     "returns `Authenticated` for valid cookie data that passes the validation check" in {
       def validateUser(au: AuthenticatedUser): Boolean = au.multiFactor && au.user.emailDomain == "example.com"
-      val cookieData = CookieUtils.generateCookieData(authUser, testPrivateKey)
+      val cookieData = CookieUtils.generateCookieData(authUser, testPrivateKey.key)
 
       authStatus(cookieData, validateUser) shouldBe a [Authenticated]
     }
 
     "gives back the provided auth user if successful" in {
-      val cookieData = CookieUtils.generateCookieData(authUser, testPrivateKey)
+      val cookieData = CookieUtils.generateCookieData(authUser, testPrivateKey.key)
 
       authStatus(cookieData) should equal(Authenticated(authUser))
     }
@@ -45,40 +45,40 @@ class PanDomainTest extends AnyFreeSpec with Matchers with Inside {
     }
 
     "returns `InvalidCookie` if the cookie fails its signature check" in {
-      val incorrectCookieData = CookieUtils.generateCookieData(authUser, testINCORRECTPrivateKey)
+      val incorrectCookieData = CookieUtils.generateCookieData(authUser, testINCORRECTPrivateKey.key)
 
       authStatus(incorrectCookieData) shouldBe a [InvalidCookie]
     }
 
     "returns `Expired` if the time is after the cookie's expiry" in {
       val expiredAuthUser = authUser.copy(expires = new Date().getTime - 86400)
-      val cookieData = CookieUtils.generateCookieData(expiredAuthUser, testPrivateKey)
+      val cookieData = CookieUtils.generateCookieData(expiredAuthUser, testPrivateKey.key)
 
       authStatus(cookieData) shouldBe a [Expired]
     }
 
     "returns `Expired` if the cookie has expired and is outside the grace period" in {
       val expiredAuthUser = authUser.copy(expires = new Date().getTime - 86400)
-      val cookieData = CookieUtils.generateCookieData(expiredAuthUser, testPrivateKey)
+      val cookieData = CookieUtils.generateCookieData(expiredAuthUser, testPrivateKey.key)
 
       authStatus(cookieData) shouldBe a [Expired]
     }
 
     "returns `Expired` if cookie has not expired, but forceExpiry is set" in {
-      val validCookieData = CookieUtils.generateCookieData(authUser, testPrivateKey)
+      val validCookieData = CookieUtils.generateCookieData(authUser, testPrivateKey.key)
       authStatus(validCookieData, forceExpiry = true) shouldBe a [Expired]
     }
 
     "returns `GracePeriod` if the cookie has expired but is within the grace period" in {
       val expiredAuthUser = authUser.copy(expires = new Date().getTime - 3000)
-      val cookieData = CookieUtils.generateCookieData(expiredAuthUser, testPrivateKey)
+      val cookieData = CookieUtils.generateCookieData(expiredAuthUser, testPrivateKey.key)
 
       authStatus(cookieData, apiGracePeriod = 3600) shouldBe a [GracePeriod]
     }
 
     "returns `NotAuthorized` if the cookie does not pass the verification check" in {
       def validateUser(au: AuthenticatedUser): Boolean = au.multiFactor && au.user.emailDomain == "example.com"
-      val cookieData = CookieUtils.generateCookieData(authUser, testPrivateKey)
+      val cookieData = CookieUtils.generateCookieData(authUser, testPrivateKey.key)
 
       authStatus(cookieData, _ => false) shouldBe a [NotAuthorized]
     }

pan-domain-auth-verification/src/test/scala/com/gu/pandomainauth/PublicSettingsTest.scala

@@ -1,6 +1,7 @@
 package com.gu.pandomainauth
 
-import com.gu.pandomainauth.service.TestKeys
+import com.gu.pandomainauth.service.TestKeys.testPublicKey
+import com.gu.pandomainauth.service.{Crypto, TestKeys}
 import org.scalatest.concurrent.ScalaFutures
 import org.scalatest.freespec.AnyFreeSpec
 import org.scalatest.EitherValues
@@ -10,19 +11,18 @@ import org.scalatest.matchers.should.Matchers
 class PublicSettingsTest extends AnyFreeSpec with Matchers with EitherValues with ScalaFutures {
   "validateKey" - {
     "returns an error if the key looks invalid" in {
-      val invalidKey = PublicKey("not a valid key")
-      PublicSettings.validateKey(invalidKey).left.value shouldEqual PublicKeyFormatFailure
+      val invalidKeyText = "not a valid key"
+      PublicSettings.validateAndParseKeyText(invalidKeyText).left.value shouldEqual PublicKeyFormatFailure
     }
 
     "returns the key if it is valid" in {
-      val key = TestKeys.testPublicKey
-      PublicSettings.validateKey(key) shouldEqual Right(key)
+      PublicSettings.validateAndParseKeyText(testPublicKey.base64Encoded) shouldEqual Right(testPublicKey.key)
     }
   }
 
   "extractPublicKey" - {
     "will get a public key from a valid settings map" in {
-      PublicSettings.extractPublicKey(Map("publicKey" -> TestKeys.testPublicKey.key)) shouldEqual Right(TestKeys.testPublicKey)
+      PublicSettings.extractPublicKey(Map("publicKey" -> testPublicKey.base64Encoded)) shouldEqual Right(testPublicKey.key)
     }
 
     "will reject a key that is not correctly formatted" in {

pan-domain-auth-verification/src/test/scala/com/gu/pandomainauth/service/CookieUtilsTest.scala

@@ -14,33 +14,33 @@ class CookieUtilsTest extends AnyFreeSpec with Matchers {
 
   "generateCookieData" - {
     "generates a a base64-encoded 'data.signature' cookie value" in {
-      CookieUtils.generateCookieData(authUser, testPrivateKey) should fullyMatch regex "[\\w+/]+=*\\.[\\w+/]+=*".r
+      CookieUtils.generateCookieData(authUser, testPrivateKey.key) should fullyMatch regex "[\\w+/]+=*\\.[\\w+/]+=*".r
     }
   }
 
   "parseCookieData" - {
     "can extract an authenticatedUser from real cookie data" in {
-      val cookieData = CookieUtils.generateCookieData(authUser, testPrivateKey)
-      CookieUtils.parseCookieData(cookieData, testPublicKey) should equal(authUser)
+      val cookieData = CookieUtils.generateCookieData(authUser, testPrivateKey.key)
+      CookieUtils.parseCookieData(cookieData, testPublicKey.key) should equal(authUser)
     }
 
     "fails to extract invalid data with a CookieSignatureInvalidException" in {
-      val cookieData = CookieUtils.generateCookieData(authUser, testINCORRECTPrivateKey)
+      val cookieData = CookieUtils.generateCookieData(authUser, testINCORRECTPrivateKey.key)
       intercept[CookieSignatureInvalidException] {
-        CookieUtils.parseCookieData("data.invalidSignature", testPublicKey)
+        CookieUtils.parseCookieData("data.invalidSignature", testPublicKey.key)
       }
     }
 
     "fails to extract incorrectly signed data with a CookieSignatureInvalidException" - {
-      val cookieData = CookieUtils.generateCookieData(authUser, testINCORRECTPrivateKey)
+      val cookieData = CookieUtils.generateCookieData(authUser, testINCORRECTPrivateKey.key)
       intercept[CookieSignatureInvalidException] {
-        CookieUtils.parseCookieData(cookieData, testPublicKey)
+        CookieUtils.parseCookieData(cookieData, testPublicKey.key)
       }
     }
 
     "fails to extract completely incorrect cookie data with a CookieParseException" - {
       intercept[CookieParseException] {
-        CookieUtils.parseCookieData("Completely incorrect cookie data", testPublicKey)
+        CookieUtils.parseCookieData("Completely incorrect cookie data", testPublicKey.key)
       }
     }
   }

pan-domain-auth-verification/src/test/scala/com/gu/pandomainauth/service/CryptoTest.scala

@@ -1,5 +1,6 @@
 package com.gu.pandomainauth.service
 
+import com.gu.pandomainauth.service.Crypto.{privateKeyFor, publicKeyFor}
 import org.scalatest.funsuite.AnyFunSuite
 import org.scalatest.matchers.should.Matchers
 
@@ -9,18 +10,18 @@ class CryptoTest extends AnyFunSuite with Matchers {
 
   test("a valid signature can be successfully verified") {
     val data = "Example payload".getBytes("UTF-8")
-    val signature = Crypto.signData(data, testPrivateKey)
-    Crypto.verifySignature(data, signature, testPublicKey) should equal(true)
+    val signature = Crypto.signData(data, testPrivateKey.key)
+    Crypto.verifySignature(data, signature, testPublicKey.key) shouldEqual true
   }
 
   test("an invalid signature will not be verified") {
     val data = "Example payload".getBytes("UTF-8")
-    Crypto.verifySignature(data, "not a valid signature".getBytes("UTF-8"), testPublicKey) should equal(false)
+    Crypto.verifySignature(data, "not a valid signature".getBytes("UTF-8"), testPublicKey.key) shouldEqual false
   }
 
   test("a valid signature created with the wrong key will not be verified") {
     val data = "Example payload".getBytes("UTF-8")
-    val signature = Crypto.signData(data, testINCORRECTPrivateKey)
-    Crypto.verifySignature(data, signature, testPublicKey) should equal(false)
+    val signature = Crypto.signData(data, testINCORRECTPrivateKey.key)
+    Crypto.verifySignature(data, signature, testPublicKey.key) shouldEqual false
   }
 }

pan-domain-auth-verification/src/test/scala/com/gu/pandomainauth/service/TestKeys.scala

@@ -1,13 +1,19 @@
 package com.gu.pandomainauth.service
 
-import com.gu.pandomainauth.{PrivateKey, PublicKey}
+import com.gu.pandomainauth.service.Crypto.{privateKeyFor, publicKeyFor}
+
+import java.security.Key
 
 object TestKeys {
 
+  case class Example[K <: Key](key: K, base64Encoded: String)
+  def example[K <: Key](f: String => K)(base64Encoded: String): Example[K] =
+    Example(f(base64Encoded), base64Encoded)
+
   /**
    * A test public/private key-pair
    */
-  val testPublicKey = PublicKey(
+  val testPublicKey = example(publicKeyFor)(
                       """MIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEA5AGsiD19GMj8p8jFLRAg
                         |k0z8SFrOU7J3VBCsSn6ByS9tMpkvI9PFWwcmwxgGXAbWkPWOfyC0nNyQPx8MhgRt
                         |zqS+X6j07juaaLnkHh8KmdLYyE7JGH9AfTI2gNI2qvSFhlvYqX8EVVSmooMz6zBu
@@ -20,7 +26,7 @@ object TestKeys {
                         |q19rXt5nBnpqVND80oPPn1wc1WrSy1sm8aQwtKSBoNJgvO6diuKPtX2BnQxzKjEw
                         |p2RyzmRIBIw16kjPNLKGgakrJOZP51gFdOA1qjUA44w0V2mxbszq40aMYFsI5Kyd
                         |qqXkOlqIoeN8DHVaNBPiSakCAwEAAQ==""".stripMargin)
-  val testPrivateKey = PrivateKey(
+  val testPrivateKey = example(privateKeyFor)(
                        """MIIJKQIBAAKCAgEA5AGsiD19GMj8p8jFLRAgk0z8SFrOU7J3VBCsSn6ByS9tMpkv
                          |I9PFWwcmwxgGXAbWkPWOfyC0nNyQPx8MhgRtzqS+X6j07juaaLnkHh8KmdLYyE7J
                          |GH9AfTI2gNI2qvSFhlvYqX8EVVSmooMz6zBuTIrn9aT9eJRsqBtNw5NKp2lB7FIB
@@ -74,7 +80,7 @@ object TestKeys {
   /**
    * A valid private key that does not match the public key (useful to test things fail with the wrong key)
    */
-  val testINCORRECTPrivateKey = PrivateKey(
+  val testINCORRECTPrivateKey = example(privateKeyFor)(
                                 """MIIEpQIBAAKCAQEA2rEzkKiGmC1Dy+MlBESQDhaokUKGKnbyB+8AoZ3dWvMKkUiC
                                   |u6LoXRPePT9ncKVtJk0fnv08Ca+eP4JOaVJMxZoHnDzhwMeRzmofHlqE6IMsU0vq
                                   |ZHZLV0+WDRCjlScZ5xq/Pvi2HUXTDrGp+DmLzibkerPBNd6f2sMeGpuGXD6ha0no