Skip to content

Commit

Permalink
Fixes #25903: Refactor API tokens after clear-text removal
Browse files Browse the repository at this point in the history
  • Loading branch information
amousset committed Nov 19, 2024
1 parent 506d211 commit ef47b5e
Show file tree
Hide file tree
Showing 13 changed files with 207 additions and 141 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ import com.normation.rudder.repository.ldap.LDAPDiffMapper
import com.normation.rudder.repository.ldap.LDAPEntityMapper
import com.normation.rudder.services.user.PersonIdentService
import com.normation.zio.*
import java.security.MessageDigest
import org.joda.time.DateTime
import zio.*
import zio.syntax.*
Expand Down Expand Up @@ -102,12 +101,15 @@ final class RoLDAPApiAccountRepository(
val systemAcl: List[ApiAclElement]
) extends RoApiAccountRepository {

// TODO: avoid storing in memory
val clearTextSystemToken = ApiToken.generate(tokenGen, "-system")

val systemAPIAccount: ApiAccount = {
ApiAccount(
ApiAccountId("rudder-system-api-account"),
ApiAccountKind.System,
ApiAccountName("Rudder system account"),
ApiToken(ApiToken.generate_secret(tokenGen, "-system")),
clearTextSystemToken.hash(),
"For internal use",
isEnabled = true,
creationDate = DateTime.now,
Expand Down Expand Up @@ -152,36 +154,16 @@ final class RoLDAPApiAccountRepository(
// a hash but a clear text token to avoid accepting the hash as valid token itself.
//
override def getByToken(token: ApiToken): IOResult[Option[ApiAccount]] = {
if (token.isHashed) {
None.succeed
} else if (MessageDigest.isEqual(token.value.getBytes(), systemAPIAccount.token.value.getBytes())) {
// Constant-time comparison
val hashedToken = token.hash()
if (systemAPIAccount.token.equalsHash(hashedToken)) {
Some(systemAPIAccount).succeed
} else {
val hash = ApiToken.hash(token.value)
for {
ldap <- ldapConnexion
// here, be careful to the semantic of get with a filter!
optEntry <- ldap.get(rudderDit.API_ACCOUNTS.dn, BuildFilter.EQ(RudderLDAPConstants.A_API_TOKEN, hash))
optEntry <- ldap.get(rudderDit.API_ACCOUNTS.dn, BuildFilter.EQ(RudderLDAPConstants.A_API_TOKEN, hashedToken.value))
optRes <- optEntry match {
case None => {
// Fallback on v1 clear text tokens
for {
optEntry <-
// here, be careful to the semantic of get with a filter!
ldap.get(rudderDit.API_ACCOUNTS.dn, BuildFilter.EQ(RudderLDAPConstants.A_API_TOKEN, token.value))
optRes <- optEntry match {
case None => None.succeed
case Some(e) =>
mapper
.entry2ApiAccount(e)
.map(Some(_))
.toIO
}
} yield {
optRes
}
}
case None => None.succeed
case Some(e) => mapper.entry2ApiAccount(e).map(Some(_)).toIO
}
} yield {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ package com.normation.rudder.api

import cats.data.*
import cats.implicits.*
import com.normation.rudder.api.ApiToken.prefixV2
import com.normation.rudder.api.ApiTokenHash.prefixV1
import com.normation.rudder.api.ApiTokenHash.prefixV2
import com.normation.rudder.facts.nodes.NodeSecurityContext
import enumeratum.*
import java.nio.charset.StandardCharsets
Expand All @@ -58,54 +59,88 @@ final case class ApiAccountId(value: String) extends AnyVal
final case class ApiAccountName(value: String) extends AnyVal

/**
* The actual authentication token.
* The actual authentication token, in clear text.
*
* TODO: Once support for plain text tokens is dropped, make separate types for plain and hashed tokens.
Current situation is confusing, and hence a bit risky.
* All clear text tokens are 32 alphanumeric characters, optionally
* followed by a "-system" suffix, indicating a system token.
*
* There are two versions of tokens:
*
* * v1: 32 alphanumeric characters stored as clear text
* they are also displayed in clear text in the interface.
* * v2: starting from Rudder 8.1, tokens are still 32 alphanumeric characters,
* but are now stored hashed in sha512 (128 characters), prefixed with "v2:".
* The tokens are only displayed once at creation.
*
* Both can have a `-system` suffix to mark the system token.
*
* To make the difference, we use a prefix to the hash value in v2
*
* * If it starts with "v2:", it is a v2 SHA512 hash of the token
* * If it does not start with "v2:", it is a clear-text v1 token
* Note: v2 tokens can never start with "v" as they are encoded as en hexadecimal string
*/
case class ApiToken(value: String) extends AnyVal {
case class ApiToken(secret: String) extends AnyVal {
// Avoid printing the value in logs, regardless of token type
override def toString: String = "[REDACTED ApiToken]"

// For cases we need to print a part of the plain token for debug.
// For cases when we need to print a part of the plain token for debug.
// Show the first 4 chars: enough to disambiguate, and preserves 166 bits of randomness.
def exposeSecretBeginning: String = {
value.take(4) + "[SHORTENED ApiToken]"
secret.take(4) + "[SHORTENED ApiToken]"
}

def hash(): ApiTokenHash = {
this.hashV2()
}

def isHashed: Boolean = {
value.startsWith(prefixV2)
// Hash V2 is a SHA512
private def hashV2(): ApiTokenHash = {
val digest = MessageDigest.getInstance("SHA-512")
val hash = digest.digest(secret.getBytes(StandardCharsets.UTF_8))
ApiTokenHash(
ApiTokenHash.prefixV2 + new String(Hex.encode(hash), StandardCharsets.UTF_8)
)
}
}

object ApiToken {
private val tokenSize = 32
private val prefixV2 = "v2:"

def hash(clearText: String): String = {
val digest = MessageDigest.getInstance("SHA-512")
prefixV2 + new String(Hex.encode(digest.digest(clearText.getBytes(StandardCharsets.UTF_8))), StandardCharsets.UTF_8)
def generate(tokenGenerator: TokenGenerator, suffix: String = ""): ApiToken = {
ApiToken(tokenGenerator.newToken(tokenSize) + suffix)
}
}

/*
def generate_secret(tokenGenerator: TokenGenerator, suffix: String = ""): String = {
tokenGenerator.newToken(tokenSize) + suffix
* There are two versions of token hashes:
*
* * v1: 32 alphanumeric characters stored as clear text.
* They were also displayed in clear text in the interface.
* They are not supported anymore since 8.3, we just ignore them.
* * v2: starting from Rudder 8.1, tokens are still 32 alphanumeric characters,
* but are now stored hashed in sha512 (128 characters), prefixed with "v2:".
* The secret are only displayed once at creation.
*
* Hashes are stored with a prefix indicating the hash algorithm:
*
* * If it starts with "v2:", it is a v2 SHA512 hash of the token
* * If it does not start with "v2:", it is a clear-text v1 token
* Note: stored v1 tokens can never start with "v" as they are encoded as en hexadecimal string
*/
case class ApiTokenHash(value: String) extends AnyVal {
// Avoid printing the value in logs, regardless of token type
override def toString: String = "[REDACTED ApiTokenHash]"

// Constant time comparison
def equalsHash(token: ApiTokenHash) = {
MessageDigest.isEqual(value.getBytes(), token.value.getBytes())
}

// Provide a hash with the same prefix, but an invalid value that will never match.
def filterValue(): ApiTokenHash = {
// A value that will never match any hash
val hashValue = "empty"

if (value.startsWith(prefixV2)) {
ApiTokenHash(prefixV2 + hashValue)
} else {
ApiTokenHash(prefixV1 + hashValue)
}
}

}

object ApiTokenHash {
// Was never actually used, v1 tokens are not prefixed
val prefixV1 = "v1:"
val prefixV2 = "v2:"
}

case class ApiVersion(
Expand Down Expand Up @@ -354,10 +389,45 @@ final case class ApiAccount(

name: ApiAccountName, // used in event log to know who did actions.

token: ApiTokenHash,
description: String,
isEnabled: Boolean,
creationDate: DateTime,
tokenGenerationDate: DateTime,
tenants: NodeSecurityContext
) {
// The same ApiAccount without the token value
def filterToken(): ApiAccount = {
this.copy(token = token.filterValue())
}
}

/**
* An API principal, containing the secret, to be used just after creation, and never stored.
*/
final case class NewApiAccount(
id: ApiAccountId,
kind: ApiAccountKind,
name: ApiAccountName,
// Clear text token, only used for just-created accounts, never stored
token: ApiToken,
description: String,
isEnabled: Boolean,
creationDate: DateTime,
tokenGenerationDate: DateTime,
tenants: NodeSecurityContext
)
) {
def toApiAccount(): ApiAccount = {
ApiAccount(
id,
kind,
name,
token.hash(),
description,
isEnabled,
creationDate,
tokenGenerationDate,
tenants
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,17 @@ import com.normation.cfclerk.domain.*
import com.normation.cfclerk.services.TechniqueRepository
import com.normation.inventory.domain.NodeId
import com.normation.rudder.api.ApiAccount
import com.normation.rudder.api.ApiAccountKind
import com.normation.rudder.api.ApiAccountKind.PublicApi as PublicApiAccount
import com.normation.rudder.api.ApiAccountKind.System
import com.normation.rudder.api.ApiAccountKind.User
import com.normation.rudder.api.ApiAuthorization.ACL
import com.normation.rudder.api.ApiAuthorization.None as NoAccess
import com.normation.rudder.api.ApiAuthorization.ACL
import com.normation.rudder.api.ApiAuthorization.RO
import com.normation.rudder.api.ApiAuthorization.RW
import com.normation.rudder.api.ApiVersion
import com.normation.rudder.api.NewApiAccount
import com.normation.rudder.apidata.ApiAccountSerialisation.transform
import com.normation.rudder.domain.nodes.*
import com.normation.rudder.domain.policies.*
import com.normation.rudder.domain.properties.*
Expand Down Expand Up @@ -604,19 +607,21 @@ object ApiAccountSerialisation {

implicit val formats: Formats = DefaultFormats

def transform(kind: ApiAccountKind): (Option[String], Option[String], Option[List[JsonApiAcl]]) = {
kind match {
case User | System => (None, None, None)
case PublicApiAccount(authz, expirationDate) =>
val acl = authz match {
case NoAccess | RO | RW => None
case ACL(acls) => Some(acls.flatMap(x => x.actions.map(a => JsonApiAcl(x.path.value, a.name))))
}
(expirationDate.map(DateFormaterService.getDisplayDateTimePicker), Some(authz.kind.name), acl)
}
}

implicit class Json(val account: ApiAccount) extends AnyVal {
def toJson: JObject = {
val (expirationDate, authzType, acl): (Option[String], Option[String], Option[List[JsonApiAcl]]) = {
account.kind match {
case User | System => (None, None, None)
case PublicApiAccount(authz, expirationDate) =>
val acl = authz match {
case NoAccess | RO | RW => None
case ACL(acls) => Some(acls.flatMap(x => x.actions.map(a => JsonApiAcl(x.path.value, a.name))))
}
(expirationDate.map(DateFormaterService.getDisplayDateTimePicker), Some(authz.kind.name), acl)
}
}
val (expirationDate, authzType, acl) = transform(account.kind)

("id" -> account.id.value) ~
("name" -> account.name.value) ~
Expand All @@ -634,3 +639,28 @@ object ApiAccountSerialisation {
}
}
}

object NewApiAccountSerialisation {

implicit val formats: Formats = DefaultFormats

implicit class Json(val account: NewApiAccount) extends AnyVal {
def toJson: JObject = {
val (expirationDate, authzType, acl) = transform(account.kind)

("id" -> account.id.value) ~
("name" -> account.name.value) ~
("token" -> account.token.secret) ~
("tokenGenerationDate" -> DateFormaterService.serialize(account.tokenGenerationDate)) ~
("kind" -> account.kind.kind.name) ~
("description" -> account.description) ~
("creationDate" -> DateFormaterService.serialize(account.creationDate)) ~
("enabled" -> account.isEnabled) ~
("expirationDate" -> expirationDate) ~
("expirationDateDefined" -> expirationDate.isDefined) ~
("authorizationType" -> authzType) ~
("acl" -> acl.map(x => Extraction.decompose(x))) ~
("tenants" -> account.tenants.serialize)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1046,7 +1046,7 @@ class LDAPEntityMapper(
for {
id <- e.required(A_API_UUID).map(ApiAccountId(_))
name <- e.required(A_NAME).map(ApiAccountName(_))
token <- e.required(A_API_TOKEN).map(ApiToken(_))
token <- e.required(A_API_TOKEN).map(ApiTokenHash(_))
creationDatetime <- e.requiredAs[GeneralizedTime](_.getAsGTime, A_CREATION_DATETIME)
tokenCreationDatetime <- e.requiredAs[GeneralizedTime](_.getAsGTime, A_API_TOKEN_CREATION_DATETIME)
isEnabled = e.getAsBoolean(A_IS_ENABLED).getOrElse(false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -875,7 +875,7 @@ class ApiAccountUnserialisationImpl extends ApiAccountUnserialisation {
ApiAccountId(id),
kind,
ApiAccountName(name),
ApiToken(token),
ApiTokenHash(token),
description,
isEnabled,
creationDate,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class TestApiToken extends Specification {
val token = ApiToken("UBeJJbm1tPDwILWVHXqBdgmIm3s4xjtY")
token.toString() must beEqualTo("[REDACTED ApiToken]")
}
"be partly hidden in controled exposure" in {
"be partly hidden in controlled exposure" in {
val token = ApiToken("UBeJJbm1tPDwILWVHXqBdgmIm3s4xjtY")
token.exposeSecretBeginning must beEqualTo("UBeJ[SHORTENED ApiToken]")
val shortToken = ApiToken("test")
Expand All @@ -58,4 +58,21 @@ class TestApiToken extends Specification {
veryShortToken.exposeSecretBeginning must beEqualTo("t[SHORTENED ApiToken]")
}
}

"Hashed API tokens" should {
"be hidden in strings" in {
val token = ApiTokenHash("UBeJJbm1tPDwILWVHXqBdgmIm3s4xjtY")
token.toString() must beEqualTo("[REDACTED ApiTokenHash]")
}

"be compared correctly" in {
val token1 = ApiTokenHash("UBeJJbm1tPDwILWVHXqBdgmIm3s4xjtY")
val token2 = ApiTokenHash("UBeJJbm1tPDwILWVHXqBdgmIm3s4xjtY")
val token3 = ApiTokenHash("UBeJJbm1tPDwILWVHXqBdgmIm3s4xjtZ")
val token4 = ApiTokenHash("")
token1.equalsHash(token2) must beTrue
token1.equalsHash(token3) must beFalse
token1.equalsHash(token4) must beFalse
}
}
}
Loading

0 comments on commit ef47b5e

Please sign in to comment.