Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #23208: Add a warning in the logs when using local auth with non-bcrypt passwords #4944

Draft
wants to merge 2 commits into
base: branches/rudder/8.0
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.normation.rudder.services.healthcheck

import better.files.File.root
import bootstrap.liftweb.AppConfigAuth
import com.normation.errors.Inconsistency
import com.normation.errors.IOResult
import com.normation.rudder.domain.logger.{HealthcheckLoggerPure => logger}
Expand Down Expand Up @@ -169,3 +170,16 @@ final class CheckFileDescriptorLimit(val nodeInfoService: NodeInfoService) exten
}
}
}

final class CheckLocalAccountHashMethod(val authConfigProvider: FileUserDetailListProvider) extends Check {
def name: CheckName = CheckName("Local users password hashes")
def run: IOResult[HealthcheckResult] = for {
hash <- authConfigProvider.authConfig.encoder
} yield {
if (PasswordEncoder.safeEncoders.contains(hash)) {
Ok(name, s"${hash} password hash")
} else {
Warning(name, s"Unsafe password hash method ${hash}, you should switch to bcrypt")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -235,15 +235,10 @@ class AppConfigAuth extends ApplicationContextAware {

if (login.isEmpty || password.isEmpty) {
(defaultEncoder, Map.empty[String, RudderUserDetail])
} else {
// check if the password is in plain text (for compatibility before #19308) or bcrypt-encoded
val passwordEncoder = if (password.startsWith("$2y$")) {
PasswordEncoder.BCRYPT
} else {
PasswordEncoder.PlainText
}
} else if (password.startsWith("$2y$")) {
// Actual bcrypt hash
(
passwordEncoder,
defaultEncoder,
Map(
login -> RudderUserDetail(
RudderAccount.User(
Expand All @@ -255,6 +250,12 @@ class AppConfigAuth extends ApplicationContextAware {
)
)
)
} else {
// Plain text, not supported anymore
logger.warn(
"Fallback admin account is configured with a plain text password. It is not supported anymore, and the account is disabled."
)
(defaultEncoder, Map.empty[String, RudderUserDetail])
}
} else {
(defaultEncoder, Map.empty[String, RudderUserDetail])
Expand Down Expand Up @@ -414,11 +415,11 @@ object AuthenticationMethods {
}
}

// always add "rootAdmin" has the first method
// always add "rootAdmin" as the first method
// and de-duplicate methods
val auths = ("rootAdmin" +: names).distinct.map(AuthenticationMethods(_))

// for each methods, check that the provider file is present, or log an error and
// for each method, check that the provider file is present, or log an error and
// disable that provider
auths.flatMap { a =>
if (a.name == "rootAdmin") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3267,7 +3267,8 @@ object RudderConfigInit {
List(
CheckCoreNumber,
CheckFreeSpace,
new CheckFileDescriptorLimit(nodeInfoService)
new CheckFileDescriptorLimit(nodeInfoService),
new CheckLocalAccountHashMethod(rudderUserListProvider)
)
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,17 +118,17 @@ object PasswordEncoder {
}
}

val PlainText = new PasswordEncoder() {
val PlainText = new PasswordEncoder() {
override def encode(rawPassword: CharSequence): String = rawPassword.toString
override def matches(rawPassword: CharSequence, encodedPassword: String): Boolean = rawPassword.toString == encodedPassword
}
// Unsalted hash functions :
val MD5 = new DigestEncoder("MD5")
val SHA1 = new DigestEncoder("SHA-1")
val SHA256 = new DigestEncoder("SHA-256")
val SHA512 = new DigestEncoder("SHA-512")
val MD5 = new DigestEncoder("MD5")
val SHA1 = new DigestEncoder("SHA-1")
val SHA256 = new DigestEncoder("SHA-256")
val SHA512 = new DigestEncoder("SHA-512")
// Salted hash functions :
val BCRYPT = new PasswordEncoder() {
val BCRYPT = new PasswordEncoder() {
override def encode(rawPassword: CharSequence): String = {
val salt: Array[Byte] = new Array(16)
random.nextBytes(salt)
Expand All @@ -147,6 +147,8 @@ object PasswordEncoder {
}
}
}
// Encoders which are safe to use, exclude simple hashes as they are vulnerable
val safeEncoders = BCRYPT :: Nil

def parse(name: String): Either[String, PasswordEncoder] = {
name.toLowerCase match {
Expand Down Expand Up @@ -495,6 +497,11 @@ object UserFileProcessing {
Inconsistency("Authentication file is malformed, the root tag '<authentication>' was not found").fail
} else {
val hash = PasswordEncoder.parse((root(0) \ "@hash").text).getOrElse(PasswordEncoder.BCRYPT)
if (!PasswordEncoder.safeEncoders.contains(hash)) {
ApplicationLoggerPure.Authz.logEffect.warn(
s"Local user authentication is configured with a deprecated and unsafe hash method '${hash}', you should switch to the recommended 'bcrypt' method."
)
}

val isCaseSensitive: IOResult[Boolean] = (root(0) \ "@case-sensitivity").text.toLowerCase match {
case "true" => true.succeed
Expand Down