Skip to content

Commit

Permalink
[RORDEV-1092] Internode SSL certificate_verification: true was caus…
Browse files Browse the repository at this point in the history
…ing problems with nodes discovery (#991)
  • Loading branch information
coutoPL authored Mar 10, 2024
1 parent ad60d64 commit 418bff4
Show file tree
Hide file tree
Showing 48 changed files with 308 additions and 166 deletions.
2 changes: 0 additions & 2 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,6 @@ stages:
ROR_TASK: integration_es717x
IT_es710x:
ROR_TASK: integration_es710x
IT_es74x:
ROR_TASK: integration_es74x
IT_es70x:
ROR_TASK: integration_es70x
IT_es67x:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ object EsConfig {
.subflatMap { fipsConfiguration =>
fipsConfiguration.fipsMode match {
case FipsMode.SslOnly if xpackSettings.securityEnabled =>
Left(RorSettingsInactiveWhenXpackSecurityIsEnabled(SettingsType.Fibs))
Left(RorSettingsInactiveWhenXpackSecurityIsEnabled(SettingsType.Fips))
case FipsMode.NonFips | FipsMode.SslOnly =>
Right(fipsConfiguration)
}
Expand All @@ -98,7 +98,7 @@ object EsConfig {
sealed trait SettingsType
object SettingsType {
case object Ssl extends SettingsType
case object Fibs extends SettingsType
case object Fips extends SettingsType
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ object SslConfiguration {
allowedProtocols: Set[SslConfiguration.Protocol],
allowedCiphers: Set[SslConfiguration.Cipher],
clientAuthenticationEnabled: Boolean,
certificateVerificationEnabled: Boolean)
certificateVerificationEnabled: Boolean,
hostnameVerificationEnabled: Boolean)
extends SslConfiguration
}

Expand All @@ -143,6 +144,7 @@ private object SslDecoders extends Logging {
val allowedCiphers = "allowed_ciphers"
val allowedProtocols = "allowed_protocols"
val certificateVerification = "certificate_verification"
val hostnameVerification = "hostname_verification"
val clientAuthentication = "client_authentication"
val verification = "verification"
val enable = "enable"
Expand Down Expand Up @@ -248,6 +250,7 @@ private object SslDecoders extends Logging {
whenEnabled(c) {
for {
certificateVerification <- c.downField(consts.certificateVerification).as[Option[Boolean]]
hostnameVerification <- c.downField(consts.hostnameVerification).as[Option[Boolean]]
verification <- c.downField(consts.verification).as[Option[Boolean]]
sslCommonProperties <- sslCommonPropertiesDecoder(basePath, c)
} yield
Expand All @@ -257,7 +260,8 @@ private object SslDecoders extends Logging {
allowedProtocols = sslCommonProperties.allowedProtocols,
allowedCiphers = sslCommonProperties.allowedCiphers,
clientAuthenticationEnabled = sslCommonProperties.clientAuthentication.getOrElse(false),
certificateVerificationEnabled = certificateVerification.orElse(verification).getOrElse(false)
certificateVerificationEnabled = certificateVerification.orElse(verification).getOrElse(false),
hostnameVerificationEnabled = hostnameVerification.getOrElse(false)
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ object ConfigLoadingInterpreter extends Logging {
CannotUseRorConfigurationWhenXpackSecurityIsEnabled(
aType match {
case SettingsType.Ssl => "SSL configuration"
case SettingsType.Fibs => "FIBS configuration"
case SettingsType.Fips => "FIBS configuration"
}
)
})
Expand Down
109 changes: 55 additions & 54 deletions core/src/main/scala/tech/beshu/ror/utils/SSLCertHelper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,17 @@ import scala.util.Try
object SSLCertHelper extends Logging {

def prepareSSLEngine(sslContext: SslContext,
hostAndPort: HostAndPort,
channelHandlerContext: ChannelHandlerContext,
serverName: Option[SNIServerName],
enableHostnameVerification: Boolean,
fipsCompliant: Boolean): SSLEngine = {
val sslEngine = if(fipsCompliant) {
val sslEngine = if (fipsCompliant || !enableHostnameVerification) {
sslContext
.newEngine(channelHandlerContext.alloc())
.newEngine(channelHandlerContext.alloc(), hostAndPort.host, hostAndPort.port)
} else {
sslContext
.newEngine(channelHandlerContext.alloc())
.newEngine(channelHandlerContext.alloc(), hostAndPort.host, hostAndPort.port)
.enableHostnameVerification
}
serverName.foreach { name =>
Expand All @@ -64,19 +66,20 @@ object SSLCertHelper extends Logging {
def prepareClientSSLContext(sslConfiguration: SslConfiguration,
fipsCompliant: Boolean,
certificateVerificationEnabled: Boolean): SslContext = {
val builder = if (certificateVerificationEnabled) {
sslConfiguration.clientCertificateConfiguration match {
case Some(truststoreBasedConfiguration: TruststoreBasedConfiguration) =>
SslContextBuilder.forClient.trustManager(getTrustManagerFactory(truststoreBasedConfiguration, fipsCompliant))
case Some(fileBasedConfiguration: ClientCertificateConfiguration.FileBasedConfiguration) =>
SslContextBuilder.forClient.trustManager(getTrustedCertificatesFromPemFile(fileBasedConfiguration).toList.asJava)
case None =>
throw new Exception("Client Authentication could not be enabled because trust certificates has not been configured")
val builder =
if (certificateVerificationEnabled) {
sslConfiguration.clientCertificateConfiguration match {
case Some(truststoreBasedConfiguration: TruststoreBasedConfiguration) =>
SslContextBuilder.forClient.trustManager(getTrustManagerFactory(truststoreBasedConfiguration, fipsCompliant))
case Some(fileBasedConfiguration: ClientCertificateConfiguration.FileBasedConfiguration) =>
SslContextBuilder.forClient.trustManager(getTrustedCertificatesFromPemFile(fileBasedConfiguration).toList.asJava)
case None =>
throw new Exception("Client Authentication could not be enabled because trust certificates has not been configured")
}
} else {
SslContextBuilder.forClient.trustManager(InsecureTrustManagerFactory.INSTANCE)
}
} else {
SslContextBuilder.forClient.trustManager(InsecureTrustManagerFactory.INSTANCE)
}
val result = if(fipsCompliant) {
val result = if (fipsCompliant) {
val keystoreBasedConfiguration = sslConfiguration.serverCertificateConfiguration match {
case keystoreBasedConfiguration: KeystoreBasedConfiguration => keystoreBasedConfiguration
case _ => throw new Exception("KeyStore based configuration is required in FIPS compliant mode")
Expand Down Expand Up @@ -107,7 +110,7 @@ object SSLCertHelper extends Logging {
.attempt
.map {
case Right(sslCtxBuilder) =>
areProtocolAndCiphersValid(sslCtxBuilder, sslConfiguration, fipsCompliant)
areProtocolAndCiphersValid(sslCtxBuilder, sslConfiguration)
if (sslConfiguration.allowedCiphers.nonEmpty) {
sslCtxBuilder.ciphers(sslConfiguration.allowedCiphers.map(_.value).asJava)
}
Expand Down Expand Up @@ -169,9 +172,8 @@ object SSLCertHelper extends Logging {
}

private def areProtocolAndCiphersValid(sslContextBuilder: SslContextBuilder,
config: SslConfiguration,
fipsCompliant: Boolean): Boolean =
trySetProtocolsAndCiphersInsideNewEngine(sslContextBuilder: SslContextBuilder, config, fipsCompliant)
config: SslConfiguration): Boolean =
trySetProtocolsAndCiphersInsideNewEngine(sslContextBuilder: SslContextBuilder, config)
.fold(
ex => {
logger.error("ROR SSL: cannot validate SSL protocols and ciphers! " + ex.getClass.getSimpleName + ": " + ex.getMessage, ex)
Expand All @@ -181,15 +183,15 @@ object SSLCertHelper extends Logging {
)

private def getKeyManagerFactoryInstance(fipsCompliant: Boolean) = {
if(fipsCompliant) {
if (fipsCompliant) {
KeyManagerFactory.getInstance("X509", BouncyCastleJsseProvider.PROVIDER_NAME)
} else {
KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm)
}
}

private def getTrustManagerFactoryInstance(fipsCompliant: Boolean) = {
if(fipsCompliant) {
if (fipsCompliant) {
TrustManagerFactory.getInstance("X509", BouncyCastleJsseProvider.PROVIDER_NAME)
} else {
TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm)
Expand All @@ -199,17 +201,18 @@ object SSLCertHelper extends Logging {
private def loadKeystoreFromFile(keystoreFile: File, password: Array[Char], fipsCompliant: Boolean): IO[KeyStore] = {
Resource
.fromAutoCloseable(IO(new FileInputStream(keystoreFile)))
.use { keystoreFile => IO {
val keystore = if (fipsCompliant) {
logger.info("Trying to load data in FIPS compliant BCFKS format...")
java.security.KeyStore.getInstance("BCFKS", "BCFIPS")
} else {
logger.info("Trying to load data in JKS or PKCS#12 format...")
java.security.KeyStore.getInstance("JKS")
.use { keystoreFile =>
IO {
val keystore = if (fipsCompliant) {
logger.info("Trying to load data in FIPS compliant BCFKS format...")
java.security.KeyStore.getInstance("BCFKS", "BCFIPS")
} else {
logger.info("Trying to load data in JKS or PKCS#12 format...")
java.security.KeyStore.getInstance("JKS")
}
keystore.load(keystoreFile, password)
keystore
}
keystore.load(keystoreFile, password)
keystore
}
}
}

Expand Down Expand Up @@ -267,24 +270,28 @@ object SSLCertHelper extends Logging {
private def loadPrivateKey(file: File): IO[PrivateKey] = {
Resource
.fromAutoCloseable(IO(new FileReader(file)))
.use { privateKeyFileReader => IO {
val pemParser = new PEMParser(privateKeyFileReader)
val privateKeyInfo = PrivateKeyInfo.getInstance(pemParser.readObject())
val converter = new JcaPEMKeyConverter()
converter.getPrivateKey(privateKeyInfo)
}}
.use { privateKeyFileReader =>
IO {
val pemParser = new PEMParser(privateKeyFileReader)
val privateKeyInfo = PrivateKeyInfo.getInstance(pemParser.readObject())
val converter = new JcaPEMKeyConverter()
converter.getPrivateKey(privateKeyInfo)
}
}
}

private def loadCertificateChain(file: File): IO[Array[X509Certificate]] = {
Resource
.fromAutoCloseable(IO(new FileInputStream(file)))
.use { certificateChainFile => IO {
val certFactory = CertificateFactory.getInstance("X.509")
certFactory.generateCertificates(certificateChainFile).asScala.toArray.map {
case cc: X509Certificate => cc
case _ => throw MalformedSslSettings(s"Certificate chain in $file contains invalid X509 certificate")
.use { certificateChainFile =>
IO {
val certFactory = CertificateFactory.getInstance("X.509")
certFactory.generateCertificates(certificateChainFile).asScala.toArray.map {
case cc: X509Certificate => cc
case _ => throw MalformedSslSettings(s"Certificate chain in $file contains invalid X509 certificate")
}
}
}}
}
}

private def getPrivateKeyAndCertificateChainFromKeystore(keystoreBasedConfiguration: KeystoreBasedConfiguration): IO[(PrivateKey, Array[X509Certificate])] = {
Expand All @@ -304,16 +311,8 @@ object SSLCertHelper extends Logging {
}

private def trySetProtocolsAndCiphersInsideNewEngine(sslContextBuilder: SslContextBuilder,
config: SslConfiguration,
fipsCompliant: Boolean) = Try {
val sslEngine = if(fipsCompliant) {
sslContextBuilder.build()
.newEngine(ByteBufAllocator.DEFAULT)
} else {
sslContextBuilder.build()
.newEngine(ByteBufAllocator.DEFAULT)
.enableHostnameVerification
}
config: SslConfiguration) = Try {
val sslEngine = sslContextBuilder.build().newEngine(ByteBufAllocator.DEFAULT)
logger.info("ROR SSL: Available ciphers: " + sslEngine.getEnabledCipherSuites.mkString(","))
if (config.allowedCiphers.nonEmpty) {
sslEngine.setEnabledCipherSuites(config.allowedCiphers.map(_.value).toArray)
Expand All @@ -327,7 +326,7 @@ object SSLCertHelper extends Logging {
}

private def prepareSslContextBuilder(sslConfiguration: SslConfiguration, fipsCompliant: Boolean): IO[SslContextBuilder] = {
if(fipsCompliant) {
if (fipsCompliant) {
val keystoreBasedConfiguration = sslConfiguration.serverCertificateConfiguration match {
case keystoreBasedConfiguration: KeystoreBasedConfiguration => keystoreBasedConfiguration
case _ => throw new Exception("KeyStore based configuration is required in FIPS compliant mode")
Expand All @@ -350,6 +349,8 @@ object SSLCertHelper extends Logging {
}
}

final case class HostAndPort(host: String, port: Int)

private implicit class EnableHostnameVerification(val sslEngine: SSLEngine) extends AnyVal {

def enableHostnameVerification: SSLEngine = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ readonlyrest:
keystore_file: "ror-keystore.jks"
keystore_pass: readonlyrest1
key_pass: readonlyrest2
verification: true
verification: true
hostname_verification: true
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ readonlyrest:
truststore_file: "ror-truststore.jks"
truststore_pass: readonlyrest3
certificate_verification: true
hostname_verification: true

access_control_rules:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ readonlyrest:
server_certificate_file: "server_certificate.pem"
server_certificate_key_file: "server_certificate_key.pem"
client_trusted_certificate_file: "client_certificate.pem"
verification: true
verification: true
hostname_verification: false
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ class SslConfigurationTest
"be loaded from elasticsearch config file" in {
val ssl = RorSsl.load(getResourcePath("/boot_tests/internode_ssl_settings_in_elasticsearch_config/")).runSyncUnsafe().toOption.get
inside(ssl.interNodeSsl) {
case Some(InternodeSslConfiguration(KeystoreBasedConfiguration(keystoreFile, Some(keystorePassword), None, Some(keyPass)), truststoreConfiguration, allowedProtocols, allowedCiphers, clientAuthenticationEnabled, certificateVerificationEnabled)) =>
case Some(InternodeSslConfiguration(KeystoreBasedConfiguration(keystoreFile, Some(keystorePassword), None, Some(keyPass)), truststoreConfiguration, allowedProtocols, allowedCiphers, clientAuthenticationEnabled, certificateVerificationEnabled, hostnameVerificationEnabled)) =>
keystoreFile.value.getName should be("ror-keystore.jks")
keystorePassword should be(KeystorePassword("readonlyrest1"))
keyPass should be(KeyPass("readonlyrest2"))
Expand All @@ -153,27 +153,29 @@ class SslConfigurationTest
allowedCiphers should be(Set.empty)
clientAuthenticationEnabled should be(false)
certificateVerificationEnabled should be(true)
hostnameVerificationEnabled should be (true)
}
ssl.externalSsl should be(None)
}
"be loaded from elasticsearch config file when pem files are used" in {
val ssl = RorSsl.load(getResourcePath("/boot_tests/internode_ssl_settings_pem_files/")).runSyncUnsafe().toOption.get
inside(ssl.interNodeSsl) {
case Some(InternodeSslConfiguration(FileBasedConfiguration(serverCertificateKeyFile, serverCertificateFile), Some(ClientCertificateConfiguration.FileBasedConfiguration(clientTrustedCertificateFile)), allowedProtocols, allowedCiphers, clientAuthenticationEnabled, certificateVerificationEnabled)) =>
case Some(InternodeSslConfiguration(FileBasedConfiguration(serverCertificateKeyFile, serverCertificateFile), Some(ClientCertificateConfiguration.FileBasedConfiguration(clientTrustedCertificateFile)), allowedProtocols, allowedCiphers, clientAuthenticationEnabled, certificateVerificationEnabled, hostnameVerificationEnabled)) =>
serverCertificateKeyFile.value.getName should be("server_certificate_key.pem")
serverCertificateFile.value.getName should be("server_certificate.pem")
clientTrustedCertificateFile.value.getName should be("client_certificate.pem")
allowedProtocols should be(Set.empty)
allowedCiphers should be(Set.empty)
clientAuthenticationEnabled should be(false)
certificateVerificationEnabled should be(true)
hostnameVerificationEnabled should be (false)
}
}
"be loaded from readonlyrest config file" when {
"elasticsearch config file doesn't contain ROR ssl section" in {
val ssl = RorSsl.load(getResourcePath("/boot_tests/internode_ssl_settings_in_readonlyrest_config/")).runSyncUnsafe().toOption.get
inside(ssl.interNodeSsl) {
case Some(InternodeSslConfiguration(KeystoreBasedConfiguration(keystoreFile, Some(keystorePassword), None, Some(keyPass)), Some(ClientCertificateConfiguration.TruststoreBasedConfiguration(truststoreFile, Some(truststorePassword))), allowedProtocols, allowedCiphers, clientAuthenticationEnabled, certificateVerificationEnabled)) =>
case Some(InternodeSslConfiguration(KeystoreBasedConfiguration(keystoreFile, Some(keystorePassword), None, Some(keyPass)), Some(ClientCertificateConfiguration.TruststoreBasedConfiguration(truststoreFile, Some(truststorePassword))), allowedProtocols, allowedCiphers, clientAuthenticationEnabled, certificateVerificationEnabled, hostnameVerificationEnabled)) =>
keystoreFile.value.getName should be("ror-keystore.jks")
keystorePassword should be(KeystorePassword("readonlyrest1"))
keyPass should be(KeyPass("readonlyrest2"))
Expand All @@ -183,6 +185,7 @@ class SslConfigurationTest
allowedCiphers should be(Set.empty)
clientAuthenticationEnabled should be(false)
certificateVerificationEnabled should be(true)
hostnameVerificationEnabled should be (true)
}
ssl.externalSsl should be(None)
}
Expand Down
Loading

0 comments on commit 418bff4

Please sign in to comment.