Skip to content

Commit

Permalink
Use zio.Config.Secret instead of String to avoid leaks (zio#2508) (
Browse files Browse the repository at this point in the history
  • Loading branch information
987Nabil authored Jan 7, 2024
1 parent ba1f618 commit e0423d8
Show file tree
Hide file tree
Showing 10 changed files with 52 additions and 24 deletions.
15 changes: 12 additions & 3 deletions zio-http/src/main/scala/zio/http/ClientSSLConfig.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package zio.http

import zio.Config
import zio.Config.Secret

sealed trait ClientSSLConfig

Expand All @@ -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(_))
Expand All @@ -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))
}
}
4 changes: 3 additions & 1 deletion zio-http/src/main/scala/zio/http/Credentials.scala
Original file line number Diff line number Diff line change
@@ -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)
6 changes: 3 additions & 3 deletions zio-http/src/main/scala/zio/http/HandlerAspect.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")),
Expand All @@ -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")),
Expand Down
27 changes: 20 additions & 7 deletions zio-http/src/main/scala/zio/http/Header.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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(" ")
Expand All @@ -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")
}
Expand Down
2 changes: 1 addition & 1 deletion zio-http/src/main/scala/zio/http/Proxy.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion zio-http/src/main/scala/zio/http/netty/NettyProxy.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,19 @@ 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

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()
Expand Down
3 changes: 2 additions & 1 deletion zio-http/src/test/scala/zio/http/ClientProxySpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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._
Expand Down Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package zio.http.internal.middlewares

import zio.Config.Secret
import zio.test.Assertion._
import zio.test._
import zio.{Ref, ZIO}
Expand All @@ -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
Expand All @@ -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
}

Expand Down
3 changes: 2 additions & 1 deletion zio-http/src/test/scala/zio/http/netty/NettyProxySpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package zio.http.netty

import zio.Config.Secret
import zio.test.Assertion.{equalTo, isNone, isNull, isSome}
import zio.test._

Expand All @@ -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))) &&
Expand Down

0 comments on commit e0423d8

Please sign in to comment.