From 8d3738a22609c5206c50d7f5fbbf17ddc8df6db7 Mon Sep 17 00:00:00 2001 From: Florian Zschocke Date: Sun, 14 Aug 2022 14:45:58 +0200 Subject: [PATCH] Use existing setting but with new values Instead of adding another setting and having to explain how the new one and the existing `requireClientCertificates` setting are interdependent, let's use the existing setting and add new values. It is changed from a boolean to a string, with the values `required`, `optional` and `disabled`. To keep backward compatibility with the old values, the `true` value is mapped to `required` and the `false` value is mapped to `optional`. --- src/main/distrib/data/defaults.properties | 17 ++++++++++----- src/main/java/com/gitblit/Constants.java | 22 ++++++++++++++++++++ src/main/java/com/gitblit/GitBlitServer.java | 15 +++++++------ 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/src/main/distrib/data/defaults.properties b/src/main/distrib/data/defaults.properties index 604caa8f..0d072b58 100644 --- a/src/main/distrib/data/defaults.properties +++ b/src/main/distrib/data/defaults.properties @@ -2132,18 +2132,25 @@ server.certificateAlias = localhost server.storePassword = gitblit # If serving over https (recommended) you might consider requiring clients to -# authenticate with ssl certificates. If enabled, only https clients with the -# a valid client certificate will be able to access Gitblit. +# authenticate with TLS certificates. # -# If disabled, optional client certificate authentication is configurable by -# server.wantClientCertificates +# Possible values are: 'required' (or 'true'), 'optional' (or 'false') and 'none' +# +# If required, only https clients with a valid client certificate will be able +# to access Gitblit. +# +# If optional, client certificate authentication is optional and will be tried +# first before falling-back to form authentication or basic authentication. +# +# If completely disabled ('none'), then the server will not ask the client to +# present a client certificate at all. # # Requiring client certificates to access any of Gitblit may be too extreme, # consider this carefully. # # SINCE 1.2.0 # RESTART REQUIRED -server.requireClientCertificates = false +server.requireClientCertificates = optional # If enabled, client certificate authentication is optional and will be tried # first before falling-back to form authentication or basic authentication. diff --git a/src/main/java/com/gitblit/Constants.java b/src/main/java/com/gitblit/Constants.java index ab503bd3..c73bc24b 100644 --- a/src/main/java/com/gitblit/Constants.java +++ b/src/main/java/com/gitblit/Constants.java @@ -645,6 +645,28 @@ public class Constants { } } + public enum TlsClientCertPolicy { + REQUIRED, TRUE, OPTIONAL, FALSE, DISABLED, NONE; + + public static TlsClientCertPolicy fromString(String value) { + for (TlsClientCertPolicy t : values()) { + if (t.name().equalsIgnoreCase(value)) { + switch(t) { + case TRUE: + return REQUIRED; + case FALSE: + return OPTIONAL; + case NONE: + return DISABLED; + default: + return t; + } + } + } + return TlsClientCertPolicy.OPTIONAL; + } + } + /** * The type of merge Gitblit will use when merging a ticket to the integration branch. *

diff --git a/src/main/java/com/gitblit/GitBlitServer.java b/src/main/java/com/gitblit/GitBlitServer.java index 190cc5d2..63914121 100644 --- a/src/main/java/com/gitblit/GitBlitServer.java +++ b/src/main/java/com/gitblit/GitBlitServer.java @@ -57,6 +57,7 @@ import org.kohsuke.args4j.Option; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.gitblit.Constants.TlsClientCertPolicy; import com.gitblit.authority.GitblitAuthority; import com.gitblit.authority.NewCertificateConfig; import com.gitblit.servlet.GitblitContext; @@ -289,10 +290,15 @@ public class GitBlitServer { logger.info("Setting up HTTPS transport on port " + params.securePort); GitblitSslContextFactory factory = new GitblitSslContextFactory(params.alias, serverKeyStore, serverTrustStore, params.storePassword, caRevocationList); - if (params.requireClientCertificates) { + TlsClientCertPolicy clientCertPolicy = TlsClientCertPolicy.fromString(params.requireClientCertificates); + if (clientCertPolicy == TlsClientCertPolicy.REQUIRED) { factory.setNeedClientAuth(true); + } else if (clientCertPolicy == TlsClientCertPolicy.OPTIONAL) { + factory.setNeedClientAuth(false); + factory.setWantClientAuth(true); } else { - factory.setWantClientAuth((params.wantClientCertificates)); + factory.setNeedClientAuth(false); + factory.setWantClientAuth(false); } ServerConnector connector = new ServerConnector(server, factory); @@ -600,10 +606,7 @@ public class GitBlitServer { public Integer shutdownPort = FILESETTINGS.getInteger(Keys.server.shutdownPort, 8081); @Option(name = "--requireClientCertificates", usage = "Require client X509 certificates for https connections.") - public Boolean requireClientCertificates = FILESETTINGS.getBoolean(Keys.server.requireClientCertificates, false); - - @Option(name = "--wantClientCertificates", usage = "Ask for optional client X509 certificate for https connections. Ignored if client certificates are required.") - public Boolean wantClientCertificates = FILESETTINGS.getBoolean(Keys.server.wantClientCertificates, false); + public String requireClientCertificates = FILESETTINGS.getString(Keys.server.requireClientCertificates, "optional"); /* * Setting overrides -- 2.39.5