From 025e831b3db8281241dbca407bacd8bfc0bea2e2 Mon Sep 17 00:00:00 2001 From: James Muehlner Date: Fri, 1 Jul 2022 21:57:47 +0000 Subject: [PATCH 1/2] GUACAMOLE-1372: Add configuration properties for setting private key and cert. --- .../auth/saml/conf/ConfigurationService.java | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/conf/ConfigurationService.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/conf/ConfigurationService.java index 048773729..026f17363 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/conf/ConfigurationService.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/conf/ConfigurationService.java @@ -24,7 +24,12 @@ import com.onelogin.saml2.settings.IdPMetadataParser; import com.onelogin.saml2.settings.Saml2Settings; import com.onelogin.saml2.settings.SettingsBuilder; import com.onelogin.saml2.util.Constants; + +import java.io.File; +import java.io.IOException; import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.util.HashMap; import java.util.Map; import javax.ws.rs.core.UriBuilder; @@ -32,9 +37,12 @@ import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.GuacamoleServerException; import org.apache.guacamole.environment.Environment; import org.apache.guacamole.properties.BooleanGuacamoleProperty; +import org.apache.guacamole.properties.FileGuacamoleProperty; import org.apache.guacamole.properties.IntegerGuacamoleProperty; import org.apache.guacamole.properties.StringGuacamoleProperty; import org.apache.guacamole.properties.URIGuacamoleProperty; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Service for retrieving configuration information regarding the SAML @@ -42,6 +50,11 @@ import org.apache.guacamole.properties.URIGuacamoleProperty; */ public class ConfigurationService { + /** + * Logger for this class. + */ + private static final Logger logger = LoggerFactory.getLogger(ConfigurationService.class); + /** * The URI of the file containing the XML Metadata associated with the * SAML IdP. @@ -161,6 +174,30 @@ public class ConfigurationService { }; + /** + * The file containing the X.509 cert to use when signing or encrypting + * requests to the SAML IdP. + */ + private static final FileGuacamoleProperty SAML_X509_CERT_PATH = + new FileGuacamoleProperty() { + + @Override + public String getName() { return "saml-x509-cert-path"; } + + }; + + /** + * The file containing the private key to use when signing or encrypting + * requests to the SAML IdP. + */ + private static final FileGuacamoleProperty SAML_PRIVATE_KEY_PATH = + new FileGuacamoleProperty() { + + @Override + public String getName() { return "saml-private-key-path"; } + + }; + /** * The Guacamole server environment. */ @@ -329,6 +366,69 @@ public class ConfigurationService { return environment.getProperty(SAML_AUTH_TIMEOUT, 5); } + /** + * Returns the file containing the X.509 certificate to use when signing + * requests to the SAML IdP. If the property is not set, null will be + * returned. + * + * @return + * The file containing the X.509 certificate to use when signing + * requests to the SAML IdP, or null if not defined. + * + * @throws GuacamoleException + * If the X.509 certificate cannot be parsed. + */ + public File getCertificateFile() throws GuacamoleException { + return environment.getProperty(SAML_X509_CERT_PATH); + } + + /** + * Returns the file containing the private key to use when signing + * requests to the SAML IdP. If the property is not set, null will be + * returned. + * + * @return + * The file containing the private key to use when signing + * requests to the SAML IdP, or null if not defined. + * + * @throws GuacamoleException + * If the private key file cannot be parsed. + */ + public File getPrivateKeyFile() throws GuacamoleException { + return environment.getProperty(SAML_PRIVATE_KEY_PATH); + } + + /** + * Returns the contents of a small file, such as a private key or certificate into + * a String. If the file does not exist, or cannot be read for any reason, a warning + * will be logged and null will be returned. + * + * @param file + * The file to read into a string. + * + * @param name + * A human-readable name for the file, to be used when formatting log messages. + * + * @return + * The contents of the file having the given path, or null if the file does not + * exist or cannot be read. + */ + private String readFileContentsIntoString(File file, String name) { + + // Attempt to read the file directly into a String + try { + return new String(Files.readAllBytes(file.toPath()), StandardCharsets.UTF_8); + } + + // If the file cannot be read, log a warning and treat it as if it does not exist + catch (IOException e) { + logger.warn("{} \"{}\" could not be read: {}.", name, file, e.getMessage()); + logger.debug("{} \"{}\" could not be read.", name, file, e); + return null; + } + + } + /** * Returns the collection of SAML settings used to initialize the client. * @@ -380,6 +480,18 @@ public class ConfigurationService { UriBuilder.fromUri(getCallbackUrl()).path("api/ext/saml/callback").build().toString()); } + // If a private key file is set, load the value into the builder now + File privateKeyFile = getPrivateKeyFile(); + if (privateKeyFile != null) + samlMap.put(SettingsBuilder.SP_PRIVATEKEY_PROPERTY_KEY, + readFileContentsIntoString(privateKeyFile, "Private Key")); + + // If a certificate file is set, load the value into the builder now + File certificateFile = getCertificateFile(); + if (certificateFile != null) + samlMap.put(SettingsBuilder.SP_X509CERT_PROPERTY_KEY, + readFileContentsIntoString(certificateFile, "X.509 Certificate")); + SettingsBuilder samlBuilder = new SettingsBuilder(); Saml2Settings samlSettings = samlBuilder.fromValues(samlMap).build(); samlSettings.setStrict(getStrict()); From 616cb3968270be4fc6cc96c72ed967e27872fcfb Mon Sep 17 00:00:00 2001 From: James Muehlner Date: Tue, 5 Jul 2022 20:37:05 +0000 Subject: [PATCH 2/2] GUACAMOLE-1372: Throw fatal exception if files are specified but unreadable. --- .../auth/saml/conf/ConfigurationService.java | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/conf/ConfigurationService.java b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/conf/ConfigurationService.java index 026f17363..95862bd34 100644 --- a/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/conf/ConfigurationService.java +++ b/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/conf/ConfigurationService.java @@ -41,8 +41,6 @@ import org.apache.guacamole.properties.FileGuacamoleProperty; import org.apache.guacamole.properties.IntegerGuacamoleProperty; import org.apache.guacamole.properties.StringGuacamoleProperty; import org.apache.guacamole.properties.URIGuacamoleProperty; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Service for retrieving configuration information regarding the SAML @@ -50,11 +48,6 @@ import org.slf4j.LoggerFactory; */ public class ConfigurationService { - /** - * Logger for this class. - */ - private static final Logger logger = LoggerFactory.getLogger(ConfigurationService.class); - /** * The URI of the file containing the XML Metadata associated with the * SAML IdP. @@ -400,8 +393,8 @@ public class ConfigurationService { /** * Returns the contents of a small file, such as a private key or certificate into - * a String. If the file does not exist, or cannot be read for any reason, a warning - * will be logged and null will be returned. + * a String. If the file does not exist, or cannot be read for any reason, an exception + * will be thrown with the details of the failure. * * @param file * The file to read into a string. @@ -410,10 +403,12 @@ public class ConfigurationService { * A human-readable name for the file, to be used when formatting log messages. * * @return - * The contents of the file having the given path, or null if the file does not - * exist or cannot be read. + * The contents of the file having the given path. + * + * @throws GuacamoleException + * If the provided file does not exist, or cannot be read for any reason. */ - private String readFileContentsIntoString(File file, String name) { + private String readFileContentsIntoString(File file, String name) throws GuacamoleException { // Attempt to read the file directly into a String try { @@ -422,9 +417,8 @@ public class ConfigurationService { // If the file cannot be read, log a warning and treat it as if it does not exist catch (IOException e) { - logger.warn("{} \"{}\" could not be read: {}.", name, file, e.getMessage()); - logger.debug("{} \"{}\" could not be read.", name, file, e); - return null; + throw new GuacamoleServerException( + name + " at \"" + file.getAbsolutePath() + "\" could not be read.", e); } }