You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by rc...@apache.org on 2021/11/04 09:52:17 UTC
[james-project] branch master updated: JAMES-3539
PushSubscription/set create should validate encryption keys
This is an automated email from the ASF dual-hosted git repository.
rcordier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
The following commit(s) were added to refs/heads/master by this push:
new 6981cdc JAMES-3539 PushSubscription/set create should validate encryption keys
6981cdc is described below
commit 6981cdc3889bc02b8714cec168b627d0c4ff2f1b
Author: Benoit TELLIER <bt...@linagora.com>
AuthorDate: Thu Nov 4 16:52:11 2021 +0700
JAMES-3539 PushSubscription/set create should validate encryption keys
---
.../james/jmap/api/model/PushSubscription.scala | 31 ++++--
.../PushSubscriptionSetMethodContract.scala | 124 ++++++++++++++++++++-
.../PushSubscriptionSetCreatePerformer.scala | 27 +++--
.../jmap/pushsubscription/PushListenerTest.scala | 4 +-
4 files changed, 158 insertions(+), 28 deletions(-)
diff --git a/server/data/data-jmap/src/main/scala/org/apache/james/jmap/api/model/PushSubscription.scala b/server/data/data-jmap/src/main/scala/org/apache/james/jmap/api/model/PushSubscription.scala
index 4466519..86545b4 100644
--- a/server/data/data-jmap/src/main/scala/org/apache/james/jmap/api/model/PushSubscription.scala
+++ b/server/data/data-jmap/src/main/scala/org/apache/james/jmap/api/model/PushSubscription.scala
@@ -19,8 +19,6 @@
package org.apache.james.jmap.api.model
-import org.apache.james.jmap.api.model.ExpireTimeInvalidException.TIME_FORMATTER
-
import java.net.URL
import java.security.interfaces.ECPublicKey
import java.time.ZonedDateTime
@@ -30,6 +28,7 @@ import java.util.{Base64, UUID}
import com.google.crypto.tink.HybridEncrypt
import com.google.crypto.tink.apps.webpush.WebPushHybridEncrypt
import com.google.crypto.tink.subtle.EllipticCurves
+import org.apache.james.jmap.api.model.ExpireTimeInvalidException.TIME_FORMATTER
import scala.util.Try
@@ -60,22 +59,26 @@ case class PushSubscriptionExpiredTime(value: ZonedDateTime) {
def isBefore(date: ZonedDateTime): Boolean = value.isBefore(date)
}
-object PushSubscriptionKeys {
- def validate(keys: PushSubscriptionKeys): Try[PushSubscriptionKeys] = Try(keys.asHybridEncrypt()).map(_ => keys)
-}
-
case class PushSubscriptionKeys(p256dh: String, auth: String) {
+ def validate: Either[IllegalArgumentException, PushSubscriptionKeys] =
+ Try(asHybridEncrypt()).map(_ => this)
+ .toEither
+ .left.map {
+ case e: IllegalArgumentException => e
+ case e => new IllegalArgumentException(e)
+ }
+
// Follows https://datatracker.ietf.org/doc/html/rfc8291
// Message Encryption for Web Push
def encrypt(payload: Array[Byte]): Array[Byte] = asHybridEncrypt()
.encrypt(payload, null)
private def asHybridEncrypt(): HybridEncrypt = new WebPushHybridEncrypt.Builder()
- .withAuthSecret(Base64.getDecoder().decode(auth))
+ .withAuthSecret(Base64.getUrlDecoder().decode(auth))
.withRecipientPublicKey(asECPublicKey())
.build()
- private def asECPublicKey(): ECPublicKey = EllipticCurves.getEcPublicKey(Base64.getDecoder.decode(p256dh))
+ private def asECPublicKey(): ECPublicKey = EllipticCurves.getEcPublicKey(Base64.getUrlDecoder.decode(p256dh))
}
case class PushSubscriptionCreationRequest(deviceClientId: DeviceClientId,
@@ -85,11 +88,23 @@ case class PushSubscriptionCreationRequest(deviceClientId: DeviceClientId,
types: Seq[TypeName]) {
def validate: Either[IllegalArgumentException, PushSubscriptionCreationRequest] =
+ for {
+ _ <- validateTypes
+ _ <- validateKeys
+ } yield {
+ this
+ }
+
+ private def validateTypes: Either[IllegalArgumentException, PushSubscriptionCreationRequest] =
if (types.isEmpty) {
scala.Left(new IllegalArgumentException("types must not be empty"))
} else {
Right(this)
}
+
+ private def validateKeys: Either[IllegalArgumentException, PushSubscriptionCreationRequest] =
+ keys.map(_.validate.map(_ => this))
+ .getOrElse(Right(this))
}
object PushSubscription {
diff --git a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/PushSubscriptionSetMethodContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/PushSubscriptionSetMethodContract.scala
index b02fd61..1db5b6d 100644
--- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/PushSubscriptionSetMethodContract.scala
+++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/PushSubscriptionSetMethodContract.scala
@@ -938,6 +938,122 @@ trait PushSubscriptionSetMethodContract {
}
@Test
+ def setMethodShouldRejectInvalidKey(): Unit = {
+ val request: String =
+ """{
+ | "using": ["urn:ietf:params:jmap:core"],
+ | "methodCalls": [
+ | [
+ | "PushSubscription/set",
+ | {
+ | "create": {
+ | "4f29": {
+ | "deviceClientId": "a889-ffea-910",
+ | "url": "https://example.com/push/?device=X8980fc&client=12c6d086",
+ | "types": ["Mailbox"],
+ | "keys": {
+ | "p256dh": "QmFkIGtleQo",
+ | "auth": "YXV0aCBzZWNyZXQK"
+ | }
+ | }
+ | }
+ | },
+ | "c1"
+ | ]
+ | ]
+ | }""".stripMargin
+
+ val response: String = `given`
+ .body(request)
+ .when
+ .post
+ .`then`
+ .statusCode(SC_OK)
+ .contentType(JSON)
+ .extract
+ .body
+ .asString
+
+ assertThatJson(response)
+ .isEqualTo(
+ s"""{
+ | "sessionState": "${SESSION_STATE.value}",
+ | "methodResponses": [
+ | [
+ | "PushSubscription/set",
+ | {
+ | "notCreated": {
+ | "4f29": {
+ | "type": "invalidArguments",
+ | "description": "java.security.spec.InvalidKeySpecException: java.security.InvalidKeyException: IOException: null"
+ | }
+ | }
+ | },
+ | "c1"
+ | ]
+ | ]
+ |}""".stripMargin)
+ }
+
+ @Test
+ def setMethodShouldAcceptValidKey(pushServer: ClientAndServer): Unit = {
+ val request: String =
+ s"""{
+ | "using": ["urn:ietf:params:jmap:core"],
+ | "methodCalls": [
+ | [
+ | "PushSubscription/set",
+ | {
+ | "create": {
+ | "4f29": {
+ | "deviceClientId": "a889-ffea-910",
+ | "url": "${getPushServerUrl(pushServer)}",
+ | "types": ["Mailbox"],
+ | "keys": {
+ | "p256dh": "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE5ozzvKUAB7GIfJ44eG-sxEcjT1O2jtk9QVD-MzFOH988CAPlSdkitm16NsMxUWksq6qGwu-r6zT7GCM9oGPXtQ==",
+ | "auth": "Z7B0LmM6iTZD85EWtNRwIg=="
+ | }
+ | }
+ | }
+ | },
+ | "c1"
+ | ]
+ | ]
+ | }""".stripMargin
+
+ val response: String = `given`
+ .body(request)
+ .when
+ .post
+ .`then`
+ .statusCode(SC_OK)
+ .contentType(JSON)
+ .extract
+ .body
+ .asString
+
+ assertThatJson(response)
+ .isEqualTo(
+ s"""{
+ | "sessionState": "${SESSION_STATE.value}",
+ | "methodResponses": [
+ | [
+ | "PushSubscription/set",
+ | {
+ | "created": {
+ | "4f29": {
+ | "id": "$${json-unit.ignore}",
+ | "expires": "$${json-unit.ignore}"
+ | }
+ | }
+ | },
+ | "c1"
+ | ]
+ | ]
+ |}""".stripMargin)
+ }
+
+ @Test
def updateMixed(server: GuiceJamesServer): Unit = {
val probe = server.getProbe(classOf[PushSubscriptionProbe])
val pushSubscription1 = probe
@@ -1469,8 +1585,8 @@ trait PushSubscriptionSetMethodContract {
val uaPublicKey: ECPublicKey = uaKeyPair.getPublic.asInstanceOf[ECPublicKey]
val authSecret: Array[Byte] = Random.randBytes(16)
- val p256dh: String = Base64.getEncoder.encodeToString(uaPublicKey.getEncoded)
- val auth: String = Base64.getEncoder.encodeToString(authSecret)
+ val p256dh: String = Base64.getUrlEncoder.encodeToString(uaPublicKey.getEncoded)
+ val auth: String = Base64.getUrlEncoder.encodeToString(authSecret)
val request: String =
s"""{
@@ -1534,8 +1650,8 @@ trait PushSubscriptionSetMethodContract {
val uaPublicKey: ECPublicKey = uaKeyPair.getPublic.asInstanceOf[ECPublicKey]
val authSecret: Array[Byte] = "secret123secret1".getBytes
- val p256dh: String = Base64.getEncoder.encodeToString(uaPublicKey.getEncoded)
- val auth: String = Base64.getEncoder.encodeToString(authSecret)
+ val p256dh: String = Base64.getUrlEncoder.encodeToString(uaPublicKey.getEncoded)
+ val auth: String = Base64.getUrlEncoder.encodeToString(authSecret)
val request: String =
s"""{
diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/PushSubscriptionSetCreatePerformer.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/PushSubscriptionSetCreatePerformer.scala
index 60c8411..4fa1e55 100644
--- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/PushSubscriptionSetCreatePerformer.scala
+++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/PushSubscriptionSetCreatePerformer.scala
@@ -1,5 +1,8 @@
package org.apache.james.jmap.method
+import java.nio.charset.StandardCharsets
+
+import javax.inject.Inject
import org.apache.james.jmap.api.model.{DeviceClientIdInvalidException, ExpireTimeInvalidException, PushSubscriptionCreationRequest, PushSubscriptionExpiredTime, PushSubscriptionId, PushSubscriptionKeys, PushSubscriptionServerURL, VerificationCode}
import org.apache.james.jmap.api.pushsubscription.PushSubscriptionRepository
import org.apache.james.jmap.core.SetError.SetErrorDescription
@@ -8,13 +11,10 @@ import org.apache.james.jmap.json.{PushSerializer, PushSubscriptionSerializer}
import org.apache.james.jmap.method.PushSubscriptionSetCreatePerformer.{CreationFailure, CreationResult, CreationResults, CreationSuccess}
import org.apache.james.jmap.pushsubscription.{PushRequest, PushTTL, WebPushClient}
import org.apache.james.mailbox.MailboxSession
-import play.api.libs.json.{JsError, JsObject, JsPath, JsSuccess, Json, JsonValidationError}
+import play.api.libs.json.{JsObject, JsPath, Json, JsonValidationError}
import reactor.core.scala.publisher.{SFlux, SMono}
import reactor.core.scheduler.Schedulers
-import java.nio.charset.StandardCharsets
-import javax.inject.Inject
-
object PushSubscriptionSetCreatePerformer {
trait CreationResult
@@ -61,16 +61,15 @@ class PushSubscriptionSetCreatePerformer @Inject()(pushSubscriptionRepository: P
}.collectSeq()
.map(CreationResults)
- private def parseCreate(jsObject: JsObject): Either[PushSubscriptionCreationParseException, PushSubscriptionCreationRequest] =
- PushSubscriptionCreation.validateProperties(jsObject)
- .flatMap(validJsObject => pushSubscriptionSerializer.deserializePushSubscriptionCreationRequest(validJsObject) match {
- case JsSuccess(creationRequest, _) =>
- creationRequest.validate match {
- case Left(e) => Left(PushSubscriptionCreationParseException(SetError.invalidArguments(SetErrorDescription(e.getMessage))))
- case Right(validSuccess) => Right(validSuccess)
- }
- case JsError(errors) => Left(PushSubscriptionCreationParseException(pushSubscriptionSetError(errors)))
- })
+ private def parseCreate(jsObject: JsObject): Either[Exception, PushSubscriptionCreationRequest] = for {
+ validJsObject <- PushSubscriptionCreation.validateProperties(jsObject)
+ parsedRequest <- pushSubscriptionSerializer.deserializePushSubscriptionCreationRequest(validJsObject).asEither
+ .left.map(errors => PushSubscriptionCreationParseException(pushSubscriptionSetError(errors)))
+ validatedRequest <- parsedRequest.validate
+ .left.map(e => PushSubscriptionCreationParseException(SetError.invalidArguments(SetErrorDescription(e.getMessage))))
+ } yield {
+ validatedRequest
+ }
private def create(clientId: PushSubscriptionCreationId, request: PushSubscriptionCreationRequest, mailboxSession: MailboxSession): SMono[CreationResult] =
SMono.fromPublisher(pushSubscriptionRepository.save(mailboxSession.getUser, request))
diff --git a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/pushsubscription/PushListenerTest.scala b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/pushsubscription/PushListenerTest.scala
index 15d8e0e..5ee1bf0 100644
--- a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/pushsubscription/PushListenerTest.scala
+++ b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/pushsubscription/PushListenerTest.scala
@@ -221,8 +221,8 @@ class PushListenerTest {
val id = SMono(pushSubscriptionRepository.save(bob, PushSubscriptionCreationRequest(
deviceClientId = DeviceClientId("junit"),
- keys = Some(PushSubscriptionKeys(p256dh = Base64.getEncoder.encodeToString(uaPublicKey.getEncoded),
- auth = Base64.getEncoder.encodeToString(authSecret))),
+ keys = Some(PushSubscriptionKeys(p256dh = Base64.getUrlEncoder.encodeToString(uaPublicKey.getEncoded),
+ auth = Base64.getUrlEncoder.encodeToString(authSecret))),
url = url,
types = Seq(EmailTypeName, MailboxTypeName)))).block().id
SMono(pushSubscriptionRepository.validateVerificationCode(bob, id)).block()
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org