From f3113397867160e21eece60839d7583a379e6012 Mon Sep 17 00:00:00 2001 From: Jiri Tyr Date: Fri, 1 Nov 2013 15:44:19 +0000 Subject: [PATCH 1/3] Adding LDAP StartTLS support Some LDAP server do not allow authenticate with unencrypted password. This patch is adding the StartTLS support which takes care of the encryption. In order to enable the StartTLS, go to "System Settings" and select the "Enable StartTLS" in the Authentication section. Then make sure that you add your LDAP certificate into the Java keystore: $ keytool -import \ -file /etc/pki/tls/certs/cacert.pem \ -alias myName \ -keystore /var/lib/gitbucket/keystore You can list all keys from the keystore like this: $ keytool -list -keystore /var/lib/gitbucket/keystore --- contrib/redhat/gitbucket.conf | 3 ++ contrib/redhat/gitbucket.init | 3 ++ .../scala/app/SystemSettingsController.scala | 3 +- .../scala/service/SystemSettingsService.scala | 8 +++- src/main/scala/util/LDAPUtil.scala | 37 ++++++++++++++++--- src/main/twirl/admin/system.scala.html | 7 ++++ 6 files changed, 52 insertions(+), 9 deletions(-) diff --git a/contrib/redhat/gitbucket.conf b/contrib/redhat/gitbucket.conf index c3959f36e..1b60b3246 100644 --- a/contrib/redhat/gitbucket.conf +++ b/contrib/redhat/gitbucket.conf @@ -16,5 +16,8 @@ # URL prefix for the GitBucket page (http://://) #GITBUCKET_PREFIX= +# Java keystore (for LDAP StartTLS) +#GITBUCKET_KEYSTORE=/var/lib/gitbucket/keystore + # Other Java option #GITBUCKET_JVM_OPTS= diff --git a/contrib/redhat/gitbucket.init b/contrib/redhat/gitbucket.init index 3aed80262..75f7525cc 100644 --- a/contrib/redhat/gitbucket.init +++ b/contrib/redhat/gitbucket.init @@ -14,6 +14,7 @@ # Default values GITBUCKET_HOME=/var/lib/gitbucket GITBUCKET_WAR_FILE=/usr/share/gitbucket/lib/gitbucket.war +GITBUCKET_KEYSTORE=/var/lib/gitbucket/keystore # Pull in cq settings [ -f /etc/sysconfig/gitbucket ] && . /etc/sysconfig/gitbucket @@ -29,6 +30,8 @@ RETVAL=0 start() { echo -n $"Starting GitBucket server: " + GITBUCKET_JVM_OPTS="${GITBUCKET_JVM_OPTS} -Djavax.net.ssl.trustStore=${GITBUCKET_KEYSTORE}" + # Compile statup parameters if [ $GITBUCKET_PORT ]; then START_OPTS="${START_OPTS} --port=${GITBUCKET_PORT}" diff --git a/src/main/scala/app/SystemSettingsController.scala b/src/main/scala/app/SystemSettingsController.scala index 9466a8388..560c54efe 100644 --- a/src/main/scala/app/SystemSettingsController.scala +++ b/src/main/scala/app/SystemSettingsController.scala @@ -33,7 +33,8 @@ trait SystemSettingsControllerBase extends ControllerBase with FlashMapSupport { "bindPassword" -> trim(label("Bind Password", optional(text()))), "baseDN" -> trim(label("Base DN", text(required))), "userNameAttribute" -> trim(label("User name attribute", text(required))), - "mailAttribute" -> trim(label("Mail address attribute", text(required))) + "mailAttribute" -> trim(label("Mail address attribute", text(required))), + "tls" -> trim(label("Enable StartTLS", optional(boolean()))) )(Ldap.apply)) )(SystemSettings.apply) diff --git a/src/main/scala/service/SystemSettingsService.scala b/src/main/scala/service/SystemSettingsService.scala index 5ed9eb3a4..2c62047c7 100644 --- a/src/main/scala/service/SystemSettingsService.scala +++ b/src/main/scala/service/SystemSettingsService.scala @@ -32,6 +32,7 @@ trait SystemSettingsService { props.setProperty(LdapBaseDN, ldap.baseDN) props.setProperty(LdapUserNameAttribute, ldap.userNameAttribute) props.setProperty(LdapMailAddressAttribute, ldap.mailAttribute) + ldap.tls.foreach(x => props.setProperty(LdapTls, x.toString)) } } props.store(new java.io.FileOutputStream(GitBucketConf), null) @@ -69,7 +70,8 @@ trait SystemSettingsService { getOptionValue(props, LdapBindPassword, None), getValue(props, LdapBaseDN, ""), getValue(props, LdapUserNameAttribute, ""), - getValue(props, LdapMailAddressAttribute, ""))) + getValue(props, LdapMailAddressAttribute, ""), + getOptionValue[Boolean](props, LdapTls, None))) } else { None } @@ -97,7 +99,8 @@ object SystemSettingsService { bindPassword: Option[String], baseDN: String, userNameAttribute: String, - mailAttribute: String) + mailAttribute: String, + tls: Option[Boolean]) case class Smtp( host: String, @@ -129,6 +132,7 @@ object SystemSettingsService { private val LdapBaseDN = "ldap.baseDN" private val LdapUserNameAttribute = "ldap.username_attribute" private val LdapMailAddressAttribute = "ldap.mail_attribute" + private val LdapTls = "ldap.tls" private def getValue[A: ClassTag](props: java.util.Properties, key: String, default: A): A = defining(props.getProperty(key)){ value => diff --git a/src/main/scala/util/LDAPUtil.scala b/src/main/scala/util/LDAPUtil.scala index 4c6347c70..d6ef2a9b7 100644 --- a/src/main/scala/util/LDAPUtil.scala +++ b/src/main/scala/util/LDAPUtil.scala @@ -3,6 +3,8 @@ package util import util.ControlUtil._ import service.SystemSettingsService import com.novell.ldap._ +import java.security.Security +import org.slf4j.LoggerFactory import service.SystemSettingsService.Ldap import scala.annotation.tailrec @@ -11,7 +13,8 @@ import scala.annotation.tailrec */ object LDAPUtil { - private val LDAP_VERSION: Int = 3 + private val LDAP_VERSION: Int = LDAPConnection.LDAP_V3 + private val logger = LoggerFactory.getLogger("LDAPUtil") /** * Try authentication by LDAP using given configuration. @@ -22,7 +25,8 @@ object LDAPUtil { ldapSettings.host, ldapSettings.port.getOrElse(SystemSettingsService.DefaultLdapPort), ldapSettings.bindDN.getOrElse(""), - ldapSettings.bindPassword.getOrElse("") + ldapSettings.bindPassword.getOrElse(""), + ldapSettings.tls.getOrElse(false) ) match { case Some(conn) => { withConnection(conn) { conn => @@ -41,7 +45,8 @@ object LDAPUtil { ldapSettings.host, ldapSettings.port.getOrElse(SystemSettingsService.DefaultLdapPort), userDN, - password + password, + ldapSettings.tls.getOrElse(false) ) match { case Some(conn) => { withConnection(conn) { conn => @@ -55,15 +60,35 @@ object LDAPUtil { } } - private def bind(host: String, port: Int, dn: String, password: String): Option[LDAPConnection] = { - val conn: LDAPConnection = new LDAPConnection + private def bind(host: String, port: Int, dn: String, password: String, tls: Boolean): Option[LDAPConnection] = { + if (tls) { + // Dynamically set Sun as the security provider + Security.addProvider(new com.sun.net.ssl.internal.ssl.Provider()) + } + + val conn: LDAPConnection = new LDAPConnection(new LDAPJSSEStartTLSFactory()) try { + // Connect to the server conn.connect(host, port) + + if (tls) { + // Secure the connection + conn.startTLS() + } + + // Bind to the server conn.bind(LDAP_VERSION, dn, password.getBytes) + Some(conn) } catch { case e: Exception => { - if (conn.isConnected) conn.disconnect() + // Provide more information if something goes wrong + logger.info("" + e) + + if (conn.isConnected) { + conn.disconnect() + } + None } } diff --git a/src/main/twirl/admin/system.scala.html b/src/main/twirl/admin/system.scala.html index 361138d56..63449436d 100644 --- a/src/main/twirl/admin/system.scala.html +++ b/src/main/twirl/admin/system.scala.html @@ -94,6 +94,13 @@ +
+
+ +
+
From cc241c5a7b34425f9e4019f758e13ae44e61c81f Mon Sep 17 00:00:00 2001 From: Jiri Tyr Date: Tue, 5 Nov 2013 15:08:03 +0000 Subject: [PATCH 2/3] Moving keystore definition into settings --- contrib/redhat/gitbucket.conf | 3 --- contrib/redhat/gitbucket.init | 3 --- src/main/scala/app/SystemSettingsController.scala | 3 ++- src/main/scala/service/SystemSettingsService.scala | 9 +++++++-- src/main/scala/util/LDAPUtil.scala | 12 +++++++++--- src/main/twirl/admin/system.scala.html | 7 +++++++ 6 files changed, 25 insertions(+), 12 deletions(-) diff --git a/contrib/redhat/gitbucket.conf b/contrib/redhat/gitbucket.conf index 1b60b3246..c3959f36e 100644 --- a/contrib/redhat/gitbucket.conf +++ b/contrib/redhat/gitbucket.conf @@ -16,8 +16,5 @@ # URL prefix for the GitBucket page (http://://) #GITBUCKET_PREFIX= -# Java keystore (for LDAP StartTLS) -#GITBUCKET_KEYSTORE=/var/lib/gitbucket/keystore - # Other Java option #GITBUCKET_JVM_OPTS= diff --git a/contrib/redhat/gitbucket.init b/contrib/redhat/gitbucket.init index 75f7525cc..3aed80262 100644 --- a/contrib/redhat/gitbucket.init +++ b/contrib/redhat/gitbucket.init @@ -14,7 +14,6 @@ # Default values GITBUCKET_HOME=/var/lib/gitbucket GITBUCKET_WAR_FILE=/usr/share/gitbucket/lib/gitbucket.war -GITBUCKET_KEYSTORE=/var/lib/gitbucket/keystore # Pull in cq settings [ -f /etc/sysconfig/gitbucket ] && . /etc/sysconfig/gitbucket @@ -30,8 +29,6 @@ RETVAL=0 start() { echo -n $"Starting GitBucket server: " - GITBUCKET_JVM_OPTS="${GITBUCKET_JVM_OPTS} -Djavax.net.ssl.trustStore=${GITBUCKET_KEYSTORE}" - # Compile statup parameters if [ $GITBUCKET_PORT ]; then START_OPTS="${START_OPTS} --port=${GITBUCKET_PORT}" diff --git a/src/main/scala/app/SystemSettingsController.scala b/src/main/scala/app/SystemSettingsController.scala index 560c54efe..c95271277 100644 --- a/src/main/scala/app/SystemSettingsController.scala +++ b/src/main/scala/app/SystemSettingsController.scala @@ -34,7 +34,8 @@ trait SystemSettingsControllerBase extends ControllerBase with FlashMapSupport { "baseDN" -> trim(label("Base DN", text(required))), "userNameAttribute" -> trim(label("User name attribute", text(required))), "mailAttribute" -> trim(label("Mail address attribute", text(required))), - "tls" -> trim(label("Enable StartTLS", optional(boolean()))) + "tls" -> trim(label("Enable StartTLS", optional(boolean()))), + "keystore" -> trim(label("Keystore", optional(text()))) )(Ldap.apply)) )(SystemSettings.apply) diff --git a/src/main/scala/service/SystemSettingsService.scala b/src/main/scala/service/SystemSettingsService.scala index 2c62047c7..ba57533c6 100644 --- a/src/main/scala/service/SystemSettingsService.scala +++ b/src/main/scala/service/SystemSettingsService.scala @@ -33,6 +33,7 @@ trait SystemSettingsService { props.setProperty(LdapUserNameAttribute, ldap.userNameAttribute) props.setProperty(LdapMailAddressAttribute, ldap.mailAttribute) ldap.tls.foreach(x => props.setProperty(LdapTls, x.toString)) + ldap.keystore.foreach(x => props.setProperty(LdapKeystore, x)) } } props.store(new java.io.FileOutputStream(GitBucketConf), null) @@ -71,7 +72,8 @@ trait SystemSettingsService { getValue(props, LdapBaseDN, ""), getValue(props, LdapUserNameAttribute, ""), getValue(props, LdapMailAddressAttribute, ""), - getOptionValue[Boolean](props, LdapTls, None))) + getOptionValue[Boolean](props, LdapTls, None), + getOptionValue(props, LdapKeystore, None))) } else { None } @@ -100,7 +102,8 @@ object SystemSettingsService { baseDN: String, userNameAttribute: String, mailAttribute: String, - tls: Option[Boolean]) + tls: Option[Boolean], + keystore: Option[String]) case class Smtp( host: String, @@ -113,6 +116,7 @@ object SystemSettingsService { val DefaultSmtpPort = 25 val DefaultLdapPort = 389 + val DefaultLdapKeystore = "/var/lib/gitbucket/keystore" private val AllowAccountRegistration = "allow_account_registration" private val Gravatar = "gravatar" @@ -133,6 +137,7 @@ object SystemSettingsService { private val LdapUserNameAttribute = "ldap.username_attribute" private val LdapMailAddressAttribute = "ldap.mail_attribute" private val LdapTls = "ldap.tls" + private val LdapKeystore = "ldap.keystore" private def getValue[A: ClassTag](props: java.util.Properties, key: String, default: A): A = defining(props.getProperty(key)){ value => diff --git a/src/main/scala/util/LDAPUtil.scala b/src/main/scala/util/LDAPUtil.scala index d6ef2a9b7..3f0bad811 100644 --- a/src/main/scala/util/LDAPUtil.scala +++ b/src/main/scala/util/LDAPUtil.scala @@ -26,7 +26,8 @@ object LDAPUtil { ldapSettings.port.getOrElse(SystemSettingsService.DefaultLdapPort), ldapSettings.bindDN.getOrElse(""), ldapSettings.bindPassword.getOrElse(""), - ldapSettings.tls.getOrElse(false) + ldapSettings.tls.getOrElse(false), + ldapSettings.keystore.getOrElse(SystemSettingsService.DefaultLdapKeystore) ) match { case Some(conn) => { withConnection(conn) { conn => @@ -46,7 +47,8 @@ object LDAPUtil { ldapSettings.port.getOrElse(SystemSettingsService.DefaultLdapPort), userDN, password, - ldapSettings.tls.getOrElse(false) + ldapSettings.tls.getOrElse(false), + ldapSettings.keystore.getOrElse(SystemSettingsService.DefaultLdapKeystore) ) match { case Some(conn) => { withConnection(conn) { conn => @@ -60,10 +62,14 @@ object LDAPUtil { } } - private def bind(host: String, port: Int, dn: String, password: String, tls: Boolean): Option[LDAPConnection] = { + private def bind(host: String, port: Int, dn: String, password: String, tls: Boolean, keystore: String): Option[LDAPConnection] = { if (tls) { // Dynamically set Sun as the security provider Security.addProvider(new com.sun.net.ssl.internal.ssl.Provider()) + + // Dynamically set the property that JSSE uses to identify + // the keystore that holds trusted root certificates + System.setProperty("javax.net.ssl.trustStore", keystore); } val conn: LDAPConnection = new LDAPConnection(new LDAPJSSEStartTLSFactory()) diff --git a/src/main/twirl/admin/system.scala.html b/src/main/twirl/admin/system.scala.html index 63449436d..26c208713 100644 --- a/src/main/twirl/admin/system.scala.html +++ b/src/main/twirl/admin/system.scala.html @@ -101,6 +101,13 @@ +
+ +
+ + +
+
From 612aba1365e576d58ebcb0560ee1bb7c8665d12b Mon Sep 17 00:00:00 2001 From: Jiri Tyr Date: Thu, 7 Nov 2013 14:56:31 +0000 Subject: [PATCH 3/3] Use the system keystore by default Default system keystore is in: $JAVA_HOME/lib/security/jssecacerts or in: $JAVA_HOME/lib/security/cacerts Custom keystore can be set either in /etc/sysconfig/gitbucket by specifying the following option: GITBUCKET_JVM_OPTS="-Djavax.net.ssl.trustStore=/path/to/your/cacerts" or in Gitbucket's System Settings. --- src/main/scala/app/SystemSettingsController.scala | 2 +- src/main/scala/service/SystemSettingsService.scala | 1 - src/main/scala/util/LDAPUtil.scala | 14 ++++++++------ src/main/twirl/admin/system.scala.html | 2 +- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/main/scala/app/SystemSettingsController.scala b/src/main/scala/app/SystemSettingsController.scala index c95271277..68efda53b 100644 --- a/src/main/scala/app/SystemSettingsController.scala +++ b/src/main/scala/app/SystemSettingsController.scala @@ -34,7 +34,7 @@ trait SystemSettingsControllerBase extends ControllerBase with FlashMapSupport { "baseDN" -> trim(label("Base DN", text(required))), "userNameAttribute" -> trim(label("User name attribute", text(required))), "mailAttribute" -> trim(label("Mail address attribute", text(required))), - "tls" -> trim(label("Enable StartTLS", optional(boolean()))), + "tls" -> trim(label("Enable TLS", optional(boolean()))), "keystore" -> trim(label("Keystore", optional(text()))) )(Ldap.apply)) )(SystemSettings.apply) diff --git a/src/main/scala/service/SystemSettingsService.scala b/src/main/scala/service/SystemSettingsService.scala index ba57533c6..85e94f797 100644 --- a/src/main/scala/service/SystemSettingsService.scala +++ b/src/main/scala/service/SystemSettingsService.scala @@ -116,7 +116,6 @@ object SystemSettingsService { val DefaultSmtpPort = 25 val DefaultLdapPort = 389 - val DefaultLdapKeystore = "/var/lib/gitbucket/keystore" private val AllowAccountRegistration = "allow_account_registration" private val Gravatar = "gravatar" diff --git a/src/main/scala/util/LDAPUtil.scala b/src/main/scala/util/LDAPUtil.scala index 3f0bad811..3f19b2830 100644 --- a/src/main/scala/util/LDAPUtil.scala +++ b/src/main/scala/util/LDAPUtil.scala @@ -14,7 +14,7 @@ import scala.annotation.tailrec object LDAPUtil { private val LDAP_VERSION: Int = LDAPConnection.LDAP_V3 - private val logger = LoggerFactory.getLogger("LDAPUtil") + private val logger = LoggerFactory.getLogger(getClass().getName()) /** * Try authentication by LDAP using given configuration. @@ -27,7 +27,7 @@ object LDAPUtil { ldapSettings.bindDN.getOrElse(""), ldapSettings.bindPassword.getOrElse(""), ldapSettings.tls.getOrElse(false), - ldapSettings.keystore.getOrElse(SystemSettingsService.DefaultLdapKeystore) + ldapSettings.keystore.getOrElse("") ) match { case Some(conn) => { withConnection(conn) { conn => @@ -48,7 +48,7 @@ object LDAPUtil { userDN, password, ldapSettings.tls.getOrElse(false), - ldapSettings.keystore.getOrElse(SystemSettingsService.DefaultLdapKeystore) + ldapSettings.keystore.getOrElse("") ) match { case Some(conn) => { withConnection(conn) { conn => @@ -67,9 +67,11 @@ object LDAPUtil { // Dynamically set Sun as the security provider Security.addProvider(new com.sun.net.ssl.internal.ssl.Provider()) - // Dynamically set the property that JSSE uses to identify - // the keystore that holds trusted root certificates - System.setProperty("javax.net.ssl.trustStore", keystore); + if (keystore.compareTo("") != 0) { + // Dynamically set the property that JSSE uses to identify + // the keystore that holds trusted root certificates + System.setProperty("javax.net.ssl.trustStore", keystore) + } } val conn: LDAPConnection = new LDAPConnection(new LDAPJSSEStartTLSFactory()) diff --git a/src/main/twirl/admin/system.scala.html b/src/main/twirl/admin/system.scala.html index 26c208713..9a02cb5a2 100644 --- a/src/main/twirl/admin/system.scala.html +++ b/src/main/twirl/admin/system.scala.html @@ -97,7 +97,7 @@