From e0423d8e9bd5d4e55b6e47e7c0bc383ee53d97da Mon Sep 17 00:00:00 2001 From: Nabil Abdel-Hafeez <7283535+987Nabil@users.noreply.github.com> Date: Sun, 7 Jan 2024 19:41:12 +0100 Subject: [PATCH] Use `zio.Config.Secret` instead of `String` to avoid leaks (#2508) (#2593) --- .../main/scala/zio/http/ClientSSLConfig.scala | 15 ++++++++--- .../src/main/scala/zio/http/Credentials.scala | 4 ++- .../main/scala/zio/http/HandlerAspect.scala | 6 ++--- zio-http/src/main/scala/zio/http/Header.scala | 27 ++++++++++++++----- zio-http/src/main/scala/zio/http/Proxy.scala | 2 +- .../scala/zio/http/netty/NettyProxy.scala | 2 +- .../netty/client/ClientSSLConverter.scala | 5 ++-- .../test/scala/zio/http/ClientProxySpec.scala | 3 ++- .../http/internal/middlewares/AuthSpec.scala | 9 ++++--- .../scala/zio/http/netty/NettyProxySpec.scala | 3 ++- 10 files changed, 52 insertions(+), 24 deletions(-) diff --git a/zio-http/src/main/scala/zio/http/ClientSSLConfig.scala b/zio-http/src/main/scala/zio/http/ClientSSLConfig.scala index 97701ea1a9..43f55164b2 100644 --- a/zio-http/src/main/scala/zio/http/ClientSSLConfig.scala +++ b/zio-http/src/main/scala/zio/http/ClientSSLConfig.scala @@ -17,6 +17,7 @@ package zio.http import zio.Config +import zio.Config.Secret sealed trait ClientSSLConfig @@ -25,7 +26,7 @@ object ClientSSLConfig { val tpe = Config.string("type") val certPath = Config.string("certPath") val trustStorePath = Config.string("trustStorePath") - val trustStorePassword = Config.secret("trustStorePassword").map(d => new String(d.value.toArray)) + val trustStorePassword = Config.secret("trustStorePassword") val default = Config.succeed(Default) val fromCertFile = certPath.map(FromCertFile(_)) @@ -45,6 +46,14 @@ object ClientSSLConfig { case object Default extends ClientSSLConfig final case class FromCertFile(certPath: String) extends ClientSSLConfig final case class FromCertResource(certPath: String) extends ClientSSLConfig - final case class FromTrustStoreResource(trustStorePath: String, trustStorePassword: String) extends ClientSSLConfig - final case class FromTrustStoreFile(trustStorePath: String, trustStorePassword: String) extends ClientSSLConfig + final case class FromTrustStoreResource(trustStorePath: String, trustStorePassword: Secret) extends ClientSSLConfig + object FromTrustStoreResource { + def apply(trustStorePath: String, trustStorePassword: String): FromTrustStoreResource = + FromTrustStoreResource(trustStorePath, Secret(trustStorePassword)) + } + final case class FromTrustStoreFile(trustStorePath: String, trustStorePassword: Secret) extends ClientSSLConfig + object FromTrustStoreFile { + def apply(trustStorePath: String, trustStorePassword: String): FromTrustStoreFile = + FromTrustStoreFile(trustStorePath, Secret(trustStorePassword)) + } } diff --git a/zio-http/src/main/scala/zio/http/Credentials.scala b/zio-http/src/main/scala/zio/http/Credentials.scala index 5f272250af..51097f02fb 100644 --- a/zio-http/src/main/scala/zio/http/Credentials.scala +++ b/zio-http/src/main/scala/zio/http/Credentials.scala @@ -1,3 +1,5 @@ package zio.http -final case class Credentials(uname: String, upassword: String) +import zio.Config.Secret + +final case class Credentials(uname: String, upassword: Secret) diff --git a/zio-http/src/main/scala/zio/http/HandlerAspect.scala b/zio-http/src/main/scala/zio/http/HandlerAspect.scala index c4c4f7d196..fd39585673 100644 --- a/zio-http/src/main/scala/zio/http/HandlerAspect.scala +++ b/zio-http/src/main/scala/zio/http/HandlerAspect.scala @@ -273,7 +273,7 @@ private[http] trait HandlerAspects extends zio.http.internal.HeaderModifier[Hand * credentials are same as the ones given */ def basicAuth(u: String, p: String): HandlerAspect[Any, Unit] = - basicAuth { credentials => (credentials.uname == u) && (credentials.upassword == p) } + basicAuth { credentials => (credentials.uname == u) && (credentials.upassword == zio.Config.Secret(p)) } /** * Creates a middleware for basic authentication using an effectful @@ -300,7 +300,7 @@ private[http] trait HandlerAspects extends zio.http.internal.HeaderModifier[Hand def bearerAuth(f: String => Boolean): HandlerAspect[Any, Unit] = customAuth( _.header(Header.Authorization) match { - case Some(Header.Authorization.Bearer(token)) => f(token) + case Some(Header.Authorization.Bearer(token)) => f(token.value.asString) case _ => false }, Headers(Header.WWWAuthenticate.Bearer(realm = "Access")), @@ -318,7 +318,7 @@ private[http] trait HandlerAspects extends zio.http.internal.HeaderModifier[Hand )(implicit trace: Trace): HandlerAspect[Env, Unit] = customAuthZIO( _.header(Header.Authorization) match { - case Some(Header.Authorization.Bearer(token)) => f(token) + case Some(Header.Authorization.Bearer(token)) => f(token.value.asString) case _ => ZIO.succeed(false) }, Headers(Header.WWWAuthenticate.Bearer(realm = "Access")), diff --git a/zio-http/src/main/scala/zio/http/Header.scala b/zio-http/src/main/scala/zio/http/Header.scala index 987f82322a..e9ee911cc0 100644 --- a/zio-http/src/main/scala/zio/http/Header.scala +++ b/zio-http/src/main/scala/zio/http/Header.scala @@ -27,6 +27,7 @@ import scala.util.control.NonFatal import scala.util.matching.Regex import scala.util.{Either, Failure, Success, Try} +import zio.Config.Secret import zio._ import zio.http.codec.RichTextCodec @@ -1020,7 +1021,11 @@ object Header { override def name: String = "authorization" - final case class Basic(username: String, password: String) extends Authorization + final case class Basic(username: String, password: Secret) extends Authorization + + object Basic { + def apply(username: String, password: String): Basic = new Basic(username, Secret(password)) + } final case class Digest( response: String, @@ -1036,9 +1041,17 @@ object Header { userhash: Boolean, ) extends Authorization - final case class Bearer(token: String) extends Authorization + final case class Bearer(token: Secret) extends Authorization + + object Bearer { + def apply(token: String): Bearer = Bearer(Secret(token)) + } + + final case class Unparsed(authScheme: String, authParameters: Secret) extends Authorization - final case class Unparsed(authScheme: String, authParameters: String) extends Authorization + object Unparsed { + def apply(authScheme: String, authParameters: String): Unparsed = Unparsed(authScheme, Secret(authParameters)) + } def parse(value: String): Either[String, Authorization] = { val parts = value.split(" ") @@ -1054,20 +1067,20 @@ object Header { def render(header: Authorization): String = header match { case Basic(username, password) => - s"Basic ${Base64.getEncoder.encodeToString(s"$username:$password".getBytes(StandardCharsets.UTF_8))}" + s"Basic ${Base64.getEncoder.encodeToString((s"$username:" ++: password.value).map(_.toByte).toArray)}" case Digest(response, username, realm, uri, opaque, algo, qop, cnonce, nonce, nc, userhash) => s"""Digest response="$response",username="$username",realm="$realm",uri=${uri.toString},opaque="$opaque",algorithm=$algo,""" + s"""qop=$qop,cnonce="$cnonce",nonce="$nonce",nc=$nc,userhash=${userhash.toString}""" - case Bearer(token) => s"Bearer $token" - case Unparsed(scheme, params) => s"$scheme $params" + case Bearer(token) => s"Bearer ${token.value.asString}" + case Unparsed(scheme, params) => s"$scheme ${params.value.asString}" } private def parseBasic(value: String): Either[String, Authorization] = { try { val partsOfBasic = new String(Base64.getDecoder.decode(value)).split(":") if (partsOfBasic.length == 2) { - Right(Basic(partsOfBasic(0), partsOfBasic(1))) + Right(Basic(partsOfBasic(0), Secret(partsOfBasic(1)))) } else { Left("Basic Authorization header value is not in the format username:password") } diff --git a/zio-http/src/main/scala/zio/http/Proxy.scala b/zio-http/src/main/scala/zio/http/Proxy.scala index 8c3bd003f0..8f73dc9f2b 100644 --- a/zio-http/src/main/scala/zio/http/Proxy.scala +++ b/zio-http/src/main/scala/zio/http/Proxy.scala @@ -47,7 +47,7 @@ object Proxy { .string("url") .mapOrFail(s => URL.decode(s).left.map(error => Config.Error.InvalidData(message = error.getMessage))) ++ (Config.string("user") ++ Config - .string("password")).nested("credentials").map { case (u, p) => Credentials(u, p) }.optional ++ + .secret("password")).nested("credentials").map { case (u, p) => Credentials(u, p) }.optional ++ Config.chunkOf("headers", Config.string("name").zip(Config.string("value"))).optional.map { case Some(headers) => Headers(headers.map { case (name, value) => Header.Custom(name, value) }: _*) case None => Headers.empty diff --git a/zio-http/src/main/scala/zio/http/netty/NettyProxy.scala b/zio-http/src/main/scala/zio/http/netty/NettyProxy.scala index c164710052..c253182fbf 100644 --- a/zio-http/src/main/scala/zio/http/netty/NettyProxy.scala +++ b/zio-http/src/main/scala/zio/http/netty/NettyProxy.scala @@ -35,7 +35,7 @@ class NettyProxy private (proxy: Proxy) { uname = credentials.uname upassword = credentials.upassword encodedHeaders = Conversions.headersToNetty(proxy.headers) - } yield new HttpProxyHandler(proxyAddress, uname, upassword, encodedHeaders) + } yield new HttpProxyHandler(proxyAddress, uname, upassword.value.asString, encodedHeaders) private def unauthorizedProxy: Option[HttpProxyHandler] = for { proxyAddress <- buildProxyAddress diff --git a/zio-http/src/main/scala/zio/http/netty/client/ClientSSLConverter.scala b/zio-http/src/main/scala/zio/http/netty/client/ClientSSLConverter.scala index f7a4d3fac6..82e9e33313 100644 --- a/zio-http/src/main/scala/zio/http/netty/client/ClientSSLConverter.scala +++ b/zio-http/src/main/scala/zio/http/netty/client/ClientSSLConverter.scala @@ -20,6 +20,7 @@ import java.io.{FileInputStream, InputStream} import java.security.KeyStore import javax.net.ssl.TrustManagerFactory +import zio.Config.Secret import zio.stacktracer.TracingImplicits.disableAutoTrace import zio.http.ClientSSLConfig @@ -27,11 +28,11 @@ import zio.http.ClientSSLConfig import io.netty.handler.ssl.util.InsecureTrustManagerFactory import io.netty.handler.ssl.{SslContext, SslContextBuilder} object ClientSSLConverter { - private def trustStoreToSslContext(trustStoreStream: InputStream, trustStorePassword: String): SslContext = { + private def trustStoreToSslContext(trustStoreStream: InputStream, trustStorePassword: Secret): SslContext = { val trustStore = KeyStore.getInstance("JKS") val trustManagerFactory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm) - trustStore.load(trustStoreStream, trustStorePassword.toCharArray) + trustStore.load(trustStoreStream, trustStorePassword.value.toArray) trustManagerFactory.init(trustStore) SslContextBuilder .forClient() diff --git a/zio-http/src/test/scala/zio/http/ClientProxySpec.scala b/zio-http/src/test/scala/zio/http/ClientProxySpec.scala index b66a8d031d..c4da90c6ed 100644 --- a/zio-http/src/test/scala/zio/http/ClientProxySpec.scala +++ b/zio-http/src/test/scala/zio/http/ClientProxySpec.scala @@ -18,6 +18,7 @@ package zio.http import java.net.ConnectException +import zio.Config.Secret import zio.test.Assertion._ import zio.test.TestAspect.{sequential, timeout, withLiveClock} import zio.test._ @@ -110,7 +111,7 @@ object ClientProxySpec extends HttpRunnableSpec { proxy = Proxy.empty .url(url) .headers(Headers(DynamicServer.APP_ID, id)) - .credentials(Credentials("test", "test")) + .credentials(Credentials("test", Secret("test"))) out <- Client .request( Request.get(url = url), diff --git a/zio-http/src/test/scala/zio/http/internal/middlewares/AuthSpec.scala b/zio-http/src/test/scala/zio/http/internal/middlewares/AuthSpec.scala index 7b679c7883..25e69097a4 100644 --- a/zio-http/src/test/scala/zio/http/internal/middlewares/AuthSpec.scala +++ b/zio-http/src/test/scala/zio/http/internal/middlewares/AuthSpec.scala @@ -16,6 +16,7 @@ package zio.http.internal.middlewares +import zio.Config.Secret import zio.test.Assertion._ import zio.test._ import zio.{Ref, ZIO} @@ -33,10 +34,10 @@ object AuthSpec extends ZIOHttpSpec with HttpAppTestExtensions { private val failureBearerHeader: Headers = Headers(Header.Authorization.Bearer(bearerToken + "SomethingElse")) private val basicAuthM = HandlerAspect.basicAuth { c => - c.uname.reverse == c.upassword + Secret(c.uname.reverse) == c.upassword } private val basicAuthZIOM = HandlerAspect.basicAuthZIO { c => - ZIO.succeed(c.uname.reverse == c.upassword) + ZIO.succeed(Secret(c.uname.reverse) == c.upassword) } private val bearerAuthM = HandlerAspect.bearerAuth { c => c == bearerToken @@ -48,9 +49,9 @@ object AuthSpec extends ZIOHttpSpec with HttpAppTestExtensions { private val basicAuthContextM = HandlerAspect.customAuthProviding[AuthContext] { r => { r.headers.get(Header.Authorization).flatMap { - case Header.Authorization.Basic(uname, password) if uname.reverse == password => + case Header.Authorization.Basic(uname, password) if Secret(uname.reverse) == password => Some(AuthContext(uname)) - case _ => + case _ => None } diff --git a/zio-http/src/test/scala/zio/http/netty/NettyProxySpec.scala b/zio-http/src/test/scala/zio/http/netty/NettyProxySpec.scala index 2b79ff8053..9df396ae11 100644 --- a/zio-http/src/test/scala/zio/http/netty/NettyProxySpec.scala +++ b/zio-http/src/test/scala/zio/http/netty/NettyProxySpec.scala @@ -16,6 +16,7 @@ package zio.http.netty +import zio.Config.Secret import zio.test.Assertion.{equalTo, isNone, isNull, isSome} import zio.test._ @@ -29,7 +30,7 @@ object NettyProxySpec extends ZIOHttpSpec { test("successfully encode valid proxy") { val username = "unameTest" val password = "upassTest" - val proxy = Proxy(validUrl, Some(Credentials(username, password))) + val proxy = Proxy(validUrl, Some(Credentials(username, Secret(password)))) val encoded = NettyProxy.fromProxy(proxy).encode assert(encoded.map(_.username()))(isSome(equalTo(username))) &&