fix #741

MAIF · Dec 18, 2020 · f09f965 · f09f965
1 parent f9aad97
commit f09f965
Show file tree

Hide file tree

Showing 7 changed files with 77 additions and 31 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -4,6 +4,40 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [1.5.0-alpha.4] - 2020-12-18
+
+https://github.com/MAIF/otoroshi/issues?q=is%3Aissue+label%3A1.5.0-alpha.4+is%3Aclosed
+https://github.com/MAIF/otoroshi/compare/v1.5.0-alpha.3...v1.5.0-alpha.4
+https://github.com/MAIF/otoroshi/releases/tag/v1.5.0-alpha.4
+
+- [UI] : add button to export form data to YAML descriptor (#679)
+- Experiment with a MutatingAdmissionWebhook to add a helper as a sidecar (#681)
+- Experiment with a ValidatingAdmissionWebhook to return useful errors when deploying otoroshi entities (#682)
+- Rename 'mtlsSettings' to 'tlsSettings' in the UI (#684)
+- Use standard kubernetes service names in target transformation (#688)
+- deprecate HasAllowedApiKeyValidator plugin (#696)
+- Remove whitelist/blacklist from UI (#697)
+- Custom template button does not work anymore (#698)
+- Cleanup possible hostnames for the kubernetes internal cluster calls (#700)
+- try to reduce memory impact of initial classpath scanning (#701)
+- only organization admins can create others admins  (#704)
+- when an organization admin creates other admins, enforce new admin organizations and teams (#705)
+- flag in kubernetes config to accepts apikeys only with daikoku tokens (#706)
+- jwt-verifiers not imported with kubernetes job (#707)
+- workflow job (#708)
+- weird npe on job list since 1.5.0-alpha.3 (#709)
+- fix bad jsonpath functions (#710)
+- include jsonpath operator in transformation utils (#711)
+- include simple el in transformation utils (#712)
+- json editor adds '{}' at the end when pasting a json document (#714)
+- strip path removes too much stuff (#715)
+- io.otoroshi/id is not in annotations in documentation (#716)
+- Add a flag in service to avoid adding default hosts (#718)
+- Make global client_credential flow available by default  (#723)
+- issue when generating subcas (#726)
+- Fix issuer DN in certificate to avoid certificate check in go (#729)
+- Some "add" doesn't work for HTTP headers in Service descriptor (#734)
+
 ## [1.5.0-alpha.3] - 2020-11-18
 
 https://github.com/MAIF/otoroshi/issues?q=is%3Aissue+label%3A1.5.0-alpha.3+is%3Aclosed

diff --git a/otoroshi/app/plugins/apikeys.scala b/otoroshi/app/plugins/apikeys.scala
@@ -25,6 +25,7 @@ import play.api.mvc.{Result, Results}
 import play.core.parsers.FormUrlEncodedParser
 import security.IdGenerator
 import ssl.{Cert, DynamicSSLEngineProvider}
+import utils.http.DN
 
 import scala.collection.concurrent.TrieMap
 import scala.concurrent.duration._
@@ -190,7 +191,7 @@ class CertificateAsApikey extends PreRouting {
       case Some(cert) => {
         val conf         = context.configFor("CertificateAsApikey")
         val serialNumber = cert.getSerialNumber.toString
-        val subjectDN    = cert.getSubjectDN.getName
+        val subjectDN    = DN(cert.getSubjectDN.getName).stringify
         val clientId     = Base64.encodeBase64String((subjectDN + "-" + serialNumber).getBytes)
         // TODO: validate CA DN based on config array
         // TODO: validate CA serial based on config array

diff --git a/otoroshi/app/plugins/clientcert.scala b/otoroshi/app/plugins/clientcert.scala
@@ -11,7 +11,7 @@ import play.api.mvc.Result
 import utils.RegexPool
 import utils.RequestImplicits._
 import utils.future.Implicits._
-import utils.http.MtlsConfig
+import utils.http.{DN, MtlsConfig}
 
 import scala.collection.concurrent.TrieMap
 import scala.concurrent.{ExecutionContext, Future}
@@ -53,7 +53,7 @@ class HasClientCertMatchingApikeyValidator extends AccessValidator {
                     chain.headOption match {
                       case Some(cert) =>
                         FastFuture.successful(
-                          RegexPool(dn).matches(cert.getIssuerDN.getName)
+                          RegexPool(dn).matches(DN(cert.getIssuerDN.getName).stringify)
                         )
                       case None => FastFuture.successful(false)
                     }
@@ -121,13 +121,13 @@ class HasClientCertMatchingValidator extends AccessValidator {
         val regexAllowedIssuerDNs =
           (config \ "regexIssuerDNs").asOpt[JsArray].map(_.value.map(_.as[String])).getOrElse(Seq.empty[String])
         if (certs.exists(cert => allowedSerialNumbers.exists(s => s == cert.getSerialNumber.toString(16))) ||
-            certs.exists(cert => allowedSubjectDNs.exists(s => RegexPool(s).matches(cert.getSubjectDN.getName))) ||
-            certs.exists(cert => allowedIssuerDNs.exists(s => RegexPool(s).matches(cert.getIssuerDN.getName))) ||
+            certs.exists(cert => allowedSubjectDNs.exists(s => RegexPool(s).matches(DN(cert.getSubjectDN.getName).stringify))) ||
+            certs.exists(cert => allowedIssuerDNs.exists(s => RegexPool(s).matches(DN(cert.getIssuerDN.getName).stringify))) ||
             certs.exists(
-              cert => regexAllowedSubjectDNs.exists(s => RegexPool.regex(s).matches(cert.getSubjectDN.getName))
+              cert => regexAllowedSubjectDNs.exists(s => RegexPool.regex(s).matches(DN(cert.getSubjectDN.getName).stringify))
             ) ||
             certs.exists(
-              cert => regexAllowedIssuerDNs.exists(s => RegexPool.regex(s).matches(cert.getIssuerDN.getName))
+              cert => regexAllowedIssuerDNs.exists(s => RegexPool.regex(s).matches(DN(cert.getIssuerDN.getName).stringify))
             )) {
           FastFuture.successful(true)
         } else {
@@ -243,10 +243,10 @@ class HasClientCertMatchingHttpValidator extends AccessValidator {
     val regexAllowedIssuerDNs =
       (values \ "regexIssuerDNs").asOpt[JsArray].map(_.value.map(_.as[String])).getOrElse(Seq.empty[String])
     if (certs.exists(cert => allowedSerialNumbers.exists(s => s == cert.getSerialNumber.toString(16))) ||
-        certs.exists(cert => allowedSubjectDNs.exists(s => RegexPool(s).matches(cert.getSubjectDN.getName))) ||
-        certs.exists(cert => allowedIssuerDNs.exists(s => RegexPool(s).matches(cert.getIssuerDN.getName))) ||
-        certs.exists(cert => regexAllowedSubjectDNs.exists(s => RegexPool.regex(s).matches(cert.getSubjectDN.getName))) ||
-        certs.exists(cert => regexAllowedIssuerDNs.exists(s => RegexPool.regex(s).matches(cert.getIssuerDN.getName)))) {
+        certs.exists(cert => allowedSubjectDNs.exists(s => RegexPool(s).matches(DN(cert.getSubjectDN.getName).stringify))) ||
+        certs.exists(cert => allowedIssuerDNs.exists(s => RegexPool(s).matches(DN(cert.getIssuerDN.getName).stringify))) ||
+        certs.exists(cert => regexAllowedSubjectDNs.exists(s => RegexPool.regex(s).matches(DN(cert.getSubjectDN.getName).stringify))) ||
+        certs.exists(cert => regexAllowedIssuerDNs.exists(s => RegexPool.regex(s).matches(DN(cert.getIssuerDN.getName).stringify)))) {
       true
     } else {
       false
@@ -379,20 +379,20 @@ class ClientCertChainHeader extends RequestTransformer {
       chain.map(
         c =>
           Json.obj(
-            "subjectDN"    -> c.getSubjectDN.getName,
-            "issuerDN"     -> c.getIssuerDN.getName,
+            "subjectDN"    -> DN(c.getSubjectDN.getName).stringify,
+            "issuerDN"     -> DN(c.getIssuerDN.getName).stringify,
             "notAfter"     -> c.getNotAfter.getTime,
             "notBefore"    -> c.getNotBefore.getTime,
             "serialNumber" -> c.getSerialNumber.toString(16),
-            "subjectCN" -> Option(c.getSubjectDN.getName)
+            "subjectCN" -> Option(DN(c.getSubjectDN.getName).stringify)
               .flatMap(_.split(",").toSeq.map(_.trim).find(_.toLowerCase().startsWith("cn=")))
               .map(_.replace("CN=", "").replace("cn=", ""))
-              .getOrElse(c.getSubjectDN.getName)
+              .getOrElse(DN(c.getSubjectDN.getName).stringify)
               .asInstanceOf[String],
-            "issuerCN" -> Option(c.getIssuerDN.getName)
+            "issuerCN" -> Option(DN(c.getIssuerDN.getName).stringify)
               .flatMap(_.split(",").toSeq.map(_.trim).find(_.toLowerCase().startsWith("cn=")))
               .map(_.replace("CN=", "").replace("cn=", ""))
-              .getOrElse(c.getIssuerDN.getName)
+              .getOrElse(DN(c.getIssuerDN.getName).stringify)
               .asInstanceOf[String]
         )
       )
@@ -424,7 +424,7 @@ class ClientCertChainHeader extends RequestTransformer {
 
         val pemMap = if (sendAsPem) Map(pemHeaderName -> ctx.request.clientCertChainPemString) else Map.empty
         val dnsMap =
-          if (sendDns) Map(dnsHeaderName -> Json.stringify(JsArray(chain.map(c => JsString(c.getSubjectDN.getName)))))
+          if (sendDns) Map(dnsHeaderName -> Json.stringify(JsArray(chain.map(c => JsString(DN(c.getSubjectDN.getName).stringify)))))
           else Map.empty
         val chainMap = if (sendChain) Map(chainHeaderName -> Json.stringify(jsonChain(chain))) else Map.empty
 

diff --git a/otoroshi/app/plugins/jobs/kubernetes/crds.scala b/otoroshi/app/plugins/jobs/kubernetes/crds.scala
@@ -29,6 +29,7 @@ import play.api.mvc.{Result, Results}
 import security.IdGenerator
 import ssl.{Cert, DynamicSSLEngineProvider}
 import utils.TypedMap
+import utils.http.DN
 
 import scala.concurrent.duration._
 import scala.concurrent.{Await, ExecutionContext, Future}
@@ -540,7 +541,7 @@ class ClientSupport(val client: KubernetesClient, logger: Logger)(implicit ec: E
             case None => None
             case Some(dn) =>
               DynamicSSLEngineProvider.certificates.find {
-                case (_, cert) => cert.id == dn || cert.certificate.map(_.getSubjectDN.getName).contains(dn)
+                case (_, cert) => cert.id == dn || cert.certificate.exists(c => DN(c.getSubjectDN.getName).isEqualsTo(DN(dn)))
               }
           }
           val maybeCert = foundACertWithSameIdAndCsr(id, csrJson, caOpt.map(_._2), certs)

diff --git a/otoroshi/app/ssl/ssl.scala b/otoroshi/app/ssl/ssl.scala
@@ -60,6 +60,7 @@ import scala.concurrent.{Await, ExecutionContext, Future}
 import scala.util.{Failure, Success, Try}
 import scala.concurrent.duration._
 import otoroshi.utils.syntax.implicits._
+import utils.http.DN
 
 /**
  * git over http works with otoroshi
@@ -1499,7 +1500,7 @@ object CertificateData {
     val client: Boolean = usages.contains(KeyPurposeId.id_kp_clientAuth)
     // val client: Boolean = Try(cert.getExtensionValue("2.5.29.37")) match {
     Json.obj(
-      "issuerDN"        -> cert.getIssuerDN.getName,
+      "issuerDN"        -> DN(cert.getIssuerDN.getName).stringify,
       "notAfter"        -> cert.getNotAfter.getTime,
       "notBefore"       -> cert.getNotBefore.getTime,
       "serialNumber"    -> cert.getSerialNumber.toString(16),
@@ -1508,7 +1509,7 @@ object CertificateData {
       "sigAlgOID"       -> cert.getSigAlgOID,
       "_signature"      -> new String(encoder.encode(cert.getSignature)),
       "signature"       -> DigestUtils.sha256Hex(cert.getSignature).toUpperCase().grouped(2).mkString(":"),
-      "subjectDN"       -> cert.getSubjectDN.getName,
+      "subjectDN"       -> DN(cert.getSubjectDN.getName).stringify,
       "domain"          -> domain,
       "rawDomain"       -> rawDomain.map(JsString.apply).getOrElse(JsNull).as[JsValue],
       "version"         -> cert.getVersion,
@@ -2282,28 +2283,28 @@ object SSLImplicits {
       s"${PemHeaders.BeginCertificate}\n${Base64.getEncoder.encodeToString(cert.getEncoded).grouped(64).mkString("\n")}\n${PemHeaders.EndCertificate}\n"
     def altNames: Seq[String] = CertInfo.getSubjectAlternativeNames(cert, logger).asScala.toSeq
     def rawDomain: Option[String] = {
-      Option(cert.getSubjectDN.getName)
+      Option(DN(cert.getSubjectDN.getName).stringify)
         .flatMap(_.split(",").toSeq.map(_.trim).find(_.toLowerCase.startsWith("cn=")))
         .map(_.replace("CN=", "").replace("cn=", ""))
     }
     def maybeDomain: Option[String] = domains.headOption
     def domain: String              = domains.headOption.getOrElse(cert.getSubjectDN.getName)
     def domains: Seq[String]        = (rawDomain ++ altNames).toSeq
     def asJson: JsObject = Json.obj(
-      "subjectDN"    -> cert.getSubjectDN.getName,
-      "issuerDN"     -> cert.getIssuerDN.getName,
+      "subjectDN"    -> DN(cert.getSubjectDN.getName).stringify,
+      "issuerDN"     -> DN(cert.getIssuerDN.getName).stringify,
       "notAfter"     -> cert.getNotAfter.getTime,
       "notBefore"    -> cert.getNotBefore.getTime,
       "serialNumber" -> cert.getSerialNumber.toString(16),
-      "subjectCN" -> Option(cert.getSubjectDN.getName)
+      "subjectCN" -> Option(DN(cert.getSubjectDN.getName).stringify)
         .flatMap(_.split(",").toSeq.map(_.trim).find(_.startsWith("CN=")))
         .map(_.replace("CN=", ""))
-        .getOrElse(cert.getSubjectDN.getName)
+        .getOrElse(DN(cert.getSubjectDN.getName).stringify)
         .asInstanceOf[String],
-      "issuerCN" -> Option(cert.getIssuerDN.getName)
+      "issuerCN" -> Option(DN(cert.getIssuerDN.getName).stringify)
         .flatMap(_.split(",").toSeq.map(_.trim).find(_.startsWith("CN=")))
         .map(_.replace("CN=", ""))
-        .getOrElse(cert.getIssuerDN.getName)
+        .getOrElse(DN(cert.getIssuerDN.getName).stringify)
         .asInstanceOf[String]
     )
   }

diff --git a/otoroshi/app/utils/httpclient.scala b/otoroshi/app/utils/httpclient.scala
@@ -46,8 +46,14 @@ import scala.xml.{Elem, XML}
 
 case class DNPart(raw: String) {
   private val parts = raw.split("=").map(_.trim)
-  val name = parts.head.toLowerCase()
-    .applyOnWithPredicate(_ == "sn")(p => "surname")
+  val name = {
+    val head = parts.head.toLowerCase()
+    if (head == "sn") {
+      "surname"
+    } else {
+      head
+    }
+  }
   val value = parts.last
 }
 
@@ -56,6 +62,9 @@ case class DN(raw: String) {
   def isEqualsTo(other: DN): Boolean = {
     parts.size == other.parts.size && parts.forall(p => other.parts.exists(o => o.name == p.name && o.value == p.value))
   }
+  def stringify: String = {
+    parts.sortWith((a, b) => a.name.compareTo(b.name) > 0).map(p => s"${p.name.toUpperCase()}=${p.value}").mkString(", ")
+  }
 }
 
 case class MtlsConfig(certs: Seq[String] = Seq.empty,

diff --git a/scripts/changelog.sh b/scripts/changelog.sh
@@ -1,2 +1,2 @@
 curl -H "Authorization: bearer ${GITHUB_TOKEN}" \
-  -X POST -d '{"query":"{\n  repository(owner: \"MAIF\", name: \"otoroshi\") {\n    milestone(number: 6) {\n      id\n      issues(first: 500, labels: [\"1.5.0-alpha.3\"]) {\n        edges {\n          node {\n            number,\n   title\n          }\n        }\n      }\n    }\n  }\n}","variables":null}' https://api.github.com/graphql | jqn 'at("data.repository.milestone.issues.edges") | map(a => a.map(i => i.node.title + " (#" + i.node.number + ")" ))'
+  -X POST -d '{"query":"{\n  repository(owner: \"MAIF\", name: \"otoroshi\") {\n    milestone(number: 6) {\n      id\n      issues(first: 500, labels: [\"1.5.0-alpha.4\"]) {\n        edges {\n          node {\n            number,\n   title\n          }\n        }\n      }\n    }\n  }\n}","variables":null}' https://api.github.com/graphql | jqn 'at("data.repository.milestone.issues.edges") | map(a => a.map(i => i.node.title + " (#" + i.node.number + ")" ))'