Issues/453 security improvements

dsf-bpe/dsf-bpe-server-jetty/src/main/java/dev/dsf/bpe/config/BpeDbMigratorConfig.java

@@ -17,6 +17,7 @@
 
 import java.util.Map;
 
+import org.springframework.beans.factory.InitializingBean;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
@@ -32,7 +33,7 @@
 
 @Configuration
 @PropertySource(value = "file:conf/config.properties", encoding = "UTF-8", ignoreResourceNotFound = true)
-public class BpeDbMigratorConfig implements DbMigratorConfig
+public class BpeDbMigratorConfig implements DbMigratorConfig, InitializingBean
 {
 	private static final String DB_LIQUIBASE_USER = "db.liquibase_user";
 	private static final String DB_SERVER_USERS_GROUP = "db.server_users_group";
@@ -97,6 +98,26 @@ public static PropertySourcesPlaceholderConfigurer propertySourcesPlaceholderCon
 		return new PropertySourcesPlaceholderConfigurer();
 	}
 
+	@Override
+	public void afterPropertiesSet() throws Exception
+	{
+		if (!POSTGRES_UNQUOTED_IDENTIFIER.matcher(dbLiquibaseUsername).matches())
+			throw new RuntimeException("Property 'dev.dsf.bpe.db.liquibase.username' value not matching "
+					+ POSTGRES_UNQUOTED_IDENTIFIER_STRING);
+		if (!POSTGRES_UNQUOTED_IDENTIFIER.matcher(dbUsersGroup).matches())
+			throw new RuntimeException(
+					"Property 'dev.dsf.bpe.db.user.group' value not matching " + POSTGRES_UNQUOTED_IDENTIFIER_STRING);
+		if (!POSTGRES_UNQUOTED_IDENTIFIER.matcher(dbUsername).matches())
+			throw new RuntimeException("Property 'dev.dsf.bpe.db.user.username' value not matching "
+					+ POSTGRES_UNQUOTED_IDENTIFIER_STRING);
+		if (!POSTGRES_UNQUOTED_IDENTIFIER.matcher(dbEngineUsersGroup).matches())
+			throw new RuntimeException("Property 'dev.dsf.bpe.db.user.engine.group' value not matching "
+					+ POSTGRES_UNQUOTED_IDENTIFIER_STRING);
+		if (!POSTGRES_UNQUOTED_IDENTIFIER.matcher(dbEngineUsername).matches())
+			throw new RuntimeException("Property 'dev.dsf.bpe.db.user.engine.username' value not matching "
+					+ POSTGRES_UNQUOTED_IDENTIFIER_STRING);
+	}
+
 	@Override
 	public String getDbUrl()
 	{

dsf-bpe/dsf-bpe-server/src/main/java/dev/dsf/bpe/authentication/IdentityProviderImpl.java

@@ -80,14 +80,14 @@ public Identity getIdentity(X509Certificate[] certificates)
 			Organization o = localOrganization.get();
 			Endpoint e = localEndpoint.get();
 
-			return new PractitionerIdentityImpl(o, e, getDsfRolesFor(p, certWrapper.thumbprint(), null, null),
-					certWrapper, p, getPractitionerRolesFor(p, certWrapper.thumbprint(), null, null), null);
+			return new PractitionerIdentityImpl(o, e, getDsfRolesFor(p, certWrapper.getThumbprint(), null, null),
+					certWrapper, p, getPractitionerRolesFor(p, certWrapper.getThumbprint(), null, null), null);
 		}
 		else
 		{
 			logger.warn(
 					"Certificate with thumbprint '{}' for '{}' unknown, not configured as local user or local organization unknown",
-					certWrapper.thumbprint(), certWrapper.subjectDn());
+					certWrapper.getThumbprint(), certWrapper.getSubjectDn());
 			return null;
 		}
 	}

dsf-bpe/dsf-bpe-server/src/main/java/dev/dsf/bpe/client/oidc/OidcClientJersey.java

@@ -144,7 +144,12 @@ public DecodedJWT getAccessTokenDecoded(OidcConfiguration configuration, Jwks jw
 					"OIDC provider does not support Client Credentials Grant, supported grant types: "
 							+ configuration.grantTypesSupported());
 
-		Response response = client.target(configuration.tokenEndpoint()).request(MediaType.APPLICATION_JSON_TYPE)
+		String tokenEndpoint = configuration.tokenEndpoint();
+		if (tokenEndpoint == null || !tokenEndpoint.startsWith("https://"))
+			throw new OidcClientException(
+					"Token endpoint URL from OIDC configuration resource is null or does not start with 'https://'");
+
+		Response response = client.target(tokenEndpoint).request(MediaType.APPLICATION_JSON_TYPE)
 				.header(HttpHeaders.AUTHORIZATION,
 						"Basic " + Base64.getEncoder()
 								.encodeToString(new StringBuilder().append(clientId).append(':').append(clientSecret)
@@ -164,22 +169,6 @@ public DecodedJWT getAccessTokenDecoded(OidcConfiguration configuration, Jwks jw
 		}
 	}
 
-	/**
-	 * Does not verify if the access token is expired. Supported algorithms: RS256, RS384, RS512, ES256, ES384 and
-	 * ES512.
-	 *
-	 * @param accessToken
-	 *            not <code>null</code>
-	 * @param jwks
-	 *            not <code>null</code>
-	 * @return decoded access token
-	 * @throws OidcClientException
-	 *             if verification fails, the public key to verify is unknown or a unsupported signature algorithm was
-	 *             used.
-	 *
-	 * @see DecodedJWT#getExpiresAt()
-	 * @see DecodedJWT#getExpiresAtAsInstant()
-	 */
 	private DecodedJWT verifyAndDecodeAccessToken(String accessToken, Jwks jwks) throws OidcClientException
 	{
 		try
@@ -191,14 +180,15 @@ private DecodedJWT verifyAndDecodeAccessToken(String accessToken, Jwks jwks) thr
 				throw new OidcClientException("Access token has no kid property");
 
 			Optional<JwksKey> key = jwks.getKey(keyId);
-			if (key.isEmpty())
-				throw new OidcClientException("Access token key with kid '" + keyId + "' not in JWKS");
+			if (key.isEmpty() || !key.get().use().equals("sig"))
+				throw new OidcClientException("Access token key with kid '" + keyId + "'  and use 'sig' not in JWKS");
 
-			Optional<Algorithm> algorithm = key.map(JwksKey::toAlgorithm);
+			Optional<Algorithm> algorithm = key.flatMap(JwksKey::toAlgorithm);
 			if (key.isEmpty())
+			{
 				throw new OidcClientException("Access token key with kid '" + keyId
-						+ "' has unsupported type (kty) / algorithm (alg) in JWKS '" + key.get().kty() + "' / '"
-						+ key.get().alg() + "'");
+						+ "' has unsupported type (kty) / algorithm (alg) / key-size in JWKS");
+			}
 
 			try
 			{
@@ -226,7 +216,7 @@ else if (requiredAudiences.size() > 1)
 			}
 			catch (TokenExpiredException e)
 			{
-				throw new OidcClientException("JWT verification failed: claim missing", e);
+				throw new OidcClientException("JWT verification failed: token expired", e);
 			}
 			catch (IncorrectClaimException e)
 			{

dsf-bpe/dsf-bpe-server/src/main/java/dev/dsf/bpe/client/oidc/OidcClientWithCache.java

@@ -32,7 +32,7 @@ private static final record CacheEntry<T>(ZonedDateTime timeout, T resource)
 	{
 	}
 
-	private final Duration cacheTimeoutconfigurationResource;
+	private final Duration cacheTimeoutConfigurationResource;
 	private final Duration cacheTimeoutJwksResource;
 	private final Duration cacheTimeoutAccessTokenBeforeExpiration;
 	private final OidcClientWithDecodedJwt delegate;
@@ -42,7 +42,7 @@ private static final record CacheEntry<T>(ZonedDateTime timeout, T resource)
 	private CacheEntry<DecodedJWT> accessTokenCache;
 
 	/**
-	 * @param cacheTimeoutconfigurationResource
+	 * @param cacheTimeoutConfigurationResource
 	 *            not <code>null</code>, not negative
 	 * @param cacheTimeoutJwksResource
 	 *            not <code>null</code>, not negative
@@ -51,13 +51,13 @@ private static final record CacheEntry<T>(ZonedDateTime timeout, T resource)
 	 * @param delegate
 	 *            not <code>null</code>
 	 */
-	public OidcClientWithCache(Duration cacheTimeoutconfigurationResource, Duration cacheTimeoutJwksResource,
+	public OidcClientWithCache(Duration cacheTimeoutConfigurationResource, Duration cacheTimeoutJwksResource,
 			Duration cacheTimeoutAccessTokenBeforeExpiration, OidcClientWithDecodedJwt delegate)
 	{
-		this.cacheTimeoutconfigurationResource = Objects.requireNonNull(cacheTimeoutconfigurationResource,
-				"cacheTimeoutconfigurationResource");
-		if (cacheTimeoutconfigurationResource.isNegative())
-			throw new IllegalArgumentException("cacheTimeoutconfigurationResource negative");
+		this.cacheTimeoutConfigurationResource = Objects.requireNonNull(cacheTimeoutConfigurationResource,
+				"cacheTimeoutConfigurationResource");
+		if (cacheTimeoutConfigurationResource.isNegative())
+			throw new IllegalArgumentException("cacheTimeoutConfigurationResource negative");
 
 		this.cacheTimeoutJwksResource = Objects.requireNonNull(cacheTimeoutJwksResource, "cacheTimeoutJwksResource");
 		if (cacheTimeoutJwksResource.isNegative())
@@ -73,13 +73,13 @@ public OidcClientWithCache(Duration cacheTimeoutconfigurationResource, Duration
 	@Override
 	public Configuration getConfiguration() throws OidcClientException
 	{
-		if (configurationCache != null && configurationCache.timeout.isBefore(ZonedDateTime.now()))
+		if (configurationCache != null && configurationCache.timeout.isAfter(ZonedDateTime.now()))
 			return configurationCache.resource;
 		else
 		{
 			Configuration configuration = delegate.getConfiguration();
 			configurationCache = new CacheEntry<Configuration>(
-					ZonedDateTime.now().plus(cacheTimeoutconfigurationResource), configuration);
+					ZonedDateTime.now().plus(cacheTimeoutConfigurationResource), configuration);
 			return configuration;
 		}
 	}
@@ -89,7 +89,7 @@ public Jwks getJwks() throws OidcClientException
 	{
 		Configuration configuration = getConfiguration();
 
-		if (jwksCache != null && jwksCache.timeout.isBefore(ZonedDateTime.now()))
+		if (jwksCache != null && jwksCache.timeout.isAfter(ZonedDateTime.now()))
 			return jwksCache.resource;
 		else
 		{
@@ -115,7 +115,7 @@ public DecodedJWT getAccessTokenDecoded() throws OidcClientException
 	@Override
 	public DecodedJWT getAccessTokenDecoded(Configuration configuration, Jwks jwks) throws OidcClientException
 	{
-		if (accessTokenCache != null && accessTokenCache.timeout.isBefore(ZonedDateTime.now()))
+		if (accessTokenCache != null && accessTokenCache.timeout.isAfter(ZonedDateTime.now()))
 			return accessTokenCache.resource;
 		else
 		{

dsf-bpe/dsf-bpe-server/src/main/java/dev/dsf/bpe/spring/config/PropertiesConfig.java

@@ -51,6 +51,7 @@
 import dev.dsf.common.config.AbstractCertificateConfig;
 import dev.dsf.common.config.ProxyConfig;
 import dev.dsf.common.config.ProxyConfigImpl;
+import dev.dsf.common.db.migration.DbMigratorConfig;
 import dev.dsf.common.docker.secrets.DockerSecretsPropertySourceFactory;
 import dev.dsf.common.documentation.Documentation;
 import dev.dsf.common.ui.theme.Theme;
@@ -451,6 +452,13 @@ else if (oldPropertyValue != null && newPropertyValue == null)
 	@Override
 	public void afterPropertiesSet() throws Exception
 	{
+		if (!DbMigratorConfig.POSTGRES_UNQUOTED_IDENTIFIER.matcher(dbUsername).matches())
+			throw new RuntimeException("Property 'dev.dsf.bpe.db.user.username' value not matching "
+					+ DbMigratorConfig.POSTGRES_UNQUOTED_IDENTIFIER_STRING);
+		if (!DbMigratorConfig.POSTGRES_UNQUOTED_IDENTIFIER.matcher(dbEngineUsername).matches())
+			throw new RuntimeException("Property 'dev.dsf.bpe.db.user.engine.username' value not matching "
+					+ DbMigratorConfig.POSTGRES_UNQUOTED_IDENTIFIER_STRING);
+
 		URL url = new URI(dsfServerBaseUrl).toURL();
 		if (!List.of("http", "https").contains(url.getProtocol()))
 		{

dsf-bpe/dsf-bpe-server/src/main/java/dev/dsf/bpe/webservice/ProcessService.java

@@ -23,7 +23,9 @@
 import java.util.Objects;
 import java.util.function.Consumer;
 
+import javax.xml.XMLConstants;
 import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerConfigurationException;
 import javax.xml.transform.TransformerException;
 import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.dom.DOMSource;
@@ -46,6 +48,7 @@
 import jakarta.ws.rs.core.Response;
 import jakarta.ws.rs.core.Response.Status;
 import jakarta.ws.rs.core.StreamingOutput;
+import net.sf.saxon.lib.FeatureKeys;
 
 @RolesAllowed("ADMIN")
 @Path(ProcessService.PATH)
@@ -61,7 +64,20 @@ public ProcessService(ThymeleafTemplateService templateService, RepositoryServic
 		super(templateService, "Process");
 
 		this.repositoryService = repositoryService;
+
 		transformerFactory = TransformerFactory.newInstance();
+		transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+		transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
+
+		try
+		{
+			transformerFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+			transformerFactory.setFeature(FeatureKeys.ALLOW_EXTERNAL_FUNCTIONS, false);
+		}
+		catch (TransformerConfigurationException e)
+		{
+			throw new RuntimeException(e);
+		}
 	}
 
 	@Override

dsf-bpe/dsf-bpe-server/src/main/resources/bpe/template/main.html

@@ -42,7 +42,7 @@
 			<a href="" download="" id="download-link" title="" th:if="${download}" th:href="${download.href}" th:title="${download.title}" th:attr="download = ${download.filename}"><svg class="icon" id="download" viewBox="0 0 24 24"><path d="M18,15v3H6v-3H4v3c0,1.1,0.9,2,2,2h12c1.1,0,2-0.9,2-2v-3H18z M17,11l-1.41-1.41L13,12.17V4h-2v8.17L8.41,9.59L7,11l5,5 L17,11z" /></svg></a>
 		</div>
 		<img id="logo" src="static/logo.svg" title="Logo">
-		<h1 id="heading" th:utext="${heading}"></h1>
+		<h1 id="heading" th:text="${heading}"></h1>
 	</div>
 	<div id="html" th:insert="${htmlFragment} ? ~{(${htmlFragment})::content} : _" th:if="${htmlFragment}"></div>
 </body>

dsf-common/dsf-common-auth/src/main/java/dev/dsf/common/auth/DsfOpenIdCredentials.java

@@ -42,4 +42,9 @@ public interface DsfOpenIdCredentials
 	 * @return <b>defaultValue</b> if no {@link String} entry with the given <b>key</b> in id-token
 	 */
 	String getStringClaimOrDefault(String key, String defaultValue);
+
+	/**
+	 * @return <code>true</code> if token not expired
+	 */
+	boolean isNotExpired();
 }

dsf-common/dsf-common-auth/src/main/java/dev/dsf/common/auth/conf/AbstractIdentity.java

@@ -36,6 +36,8 @@ public abstract class AbstractIdentity implements Identity
 	private final Set<DsfRole> dsfRoles = new HashSet<>();
 	private final X509CertificateWrapper certificate;
 
+	private final String organizationIdentifierValue;
+
 	/**
 	 * @param localIdentity
 	 *            <code>true</code> if this is a local identity
@@ -59,6 +61,34 @@ public AbstractIdentity(boolean localIdentity, Organization organization, Endpoi
 			this.dsfRoles.addAll(dsfRoles);
 
 		this.certificate = certificate;
+
+		this.organizationIdentifierValue = getIdentifierValue(organization::getIdentifier,
+				ORGANIZATION_IDENTIFIER_SYSTEM).orElseThrow();
+	}
+
+	@Override
+	public int hashCode()
+	{
+		return Objects.hash(organizationIdentifierValue);
+	}
+
+	@Override
+	public boolean equals(Object obj)
+	{
+		if (this == obj)
+			return true;
+		if (obj == null)
+			return false;
+		if (getClass() != obj.getClass())
+			return false;
+
+		return Objects.equals(organizationIdentifierValue, ((AbstractIdentity) obj).organizationIdentifierValue);
+	}
+
+	@Override
+	public boolean isNotExpired()
+	{
+		return certificate != null && certificate.isNotExpired();
 	}
 
 	@Override
@@ -74,9 +104,9 @@ public Organization getOrganization()
 	}
 
 	@Override
-	public Optional<String> getOrganizationIdentifierValue()
+	public String getOrganizationIdentifierValue()
 	{
-		return getIdentifierValue(organization::getIdentifier, ORGANIZATION_IDENTIFIER_SYSTEM);
+		return organizationIdentifierValue;
 	}
 
 	protected Optional<String> getIdentifierValue(Supplier<List<Identifier>> identifiers, String identifierSystem)