From 28cf5bd5c149d76b0097d8bbf86a10080ffb75d1 Mon Sep 17 00:00:00 2001 From: Thomas Lenz Date: Tue, 22 Nov 2016 16:06:46 +0100 Subject: fix bug in eIDAS SAML-engine that does not allow SIGNATURE_RSA_SHAxxx_MGF1 algorithms for XML signatures --- .../moa/id/auth/modules/eidas/Constants.java | 2 +- .../id/auth/modules/eidas/config/MOASWSigner.java | 67 ++++++++++++++- .../eidas/utils/MOAWhiteListConfigurator.java | 96 ++++++++++++++++++++++ 3 files changed, 161 insertions(+), 4 deletions(-) create mode 100644 id/server/modules/moa-id-module-eIDAS/src/main/java/at/gv/egovernment/moa/id/auth/modules/eidas/utils/MOAWhiteListConfigurator.java (limited to 'id/server/modules/moa-id-module-eIDAS/src/main') diff --git a/id/server/modules/moa-id-module-eIDAS/src/main/java/at/gv/egovernment/moa/id/auth/modules/eidas/Constants.java b/id/server/modules/moa-id-module-eIDAS/src/main/java/at/gv/egovernment/moa/id/auth/modules/eidas/Constants.java index f45b6ffa5..02c9a8f5d 100644 --- a/id/server/modules/moa-id-module-eIDAS/src/main/java/at/gv/egovernment/moa/id/auth/modules/eidas/Constants.java +++ b/id/server/modules/moa-id-module-eIDAS/src/main/java/at/gv/egovernment/moa/id/auth/modules/eidas/Constants.java @@ -45,7 +45,7 @@ public class Constants { public static final String eIDAS_SAML_ENGINE_NAME_ID_CLASS = "class"; //default implementations for eIDAS SAML-engine functionality - public static final String SAML_SIGNING_IMPLENTATION = "eu.eidas.auth.engine.core.impl.SignSW"; + public static final String SAML_SIGNING_IMPLENTATION = "at.gv.egovernment.moa.id.auth.modules.eidas.config.MOASWSigner"; public static final String SAML_ENCRYPTION_IMPLENTATION = "at.gv.egovernment.moa.id.auth.modules.eidas.config.ModifiedEncryptionSW"; //configuration property keys diff --git a/id/server/modules/moa-id-module-eIDAS/src/main/java/at/gv/egovernment/moa/id/auth/modules/eidas/config/MOASWSigner.java b/id/server/modules/moa-id-module-eIDAS/src/main/java/at/gv/egovernment/moa/id/auth/modules/eidas/config/MOASWSigner.java index 302c12aaa..5cf5e83ec 100644 --- a/id/server/modules/moa-id-module-eIDAS/src/main/java/at/gv/egovernment/moa/id/auth/modules/eidas/config/MOASWSigner.java +++ b/id/server/modules/moa-id-module-eIDAS/src/main/java/at/gv/egovernment/moa/id/auth/modules/eidas/config/MOASWSigner.java @@ -22,12 +22,22 @@ */ package at.gv.egovernment.moa.id.auth.modules.eidas.config; +import java.util.Locale; import java.util.Map; +import org.apache.commons.lang.StringUtils; +import org.apache.xml.security.signature.XMLSignature; +import org.opensaml.xml.signature.SignatureConstants; + +import com.google.common.collect.ImmutableSet; + import at.gv.egovernment.moa.id.auth.modules.eidas.Constants; +import at.gv.egovernment.moa.id.auth.modules.eidas.utils.MOAWhiteListConfigurator; +import at.gv.egovernment.moaspss.logging.Logger; import eu.eidas.auth.engine.configuration.SamlEngineConfigurationException; import eu.eidas.auth.engine.configuration.dom.ConfigurationAdapter; import eu.eidas.auth.engine.configuration.dom.ConfigurationKey; +import eu.eidas.auth.engine.configuration.dom.KeyStoreSignatureConfigurator; import eu.eidas.auth.engine.core.impl.KeyStoreProtocolSigner; import eu.eidas.samlengineconfig.CertificateConfigurationManager; @@ -37,20 +47,71 @@ import eu.eidas.samlengineconfig.CertificateConfigurationManager; */ public class MOASWSigner extends KeyStoreProtocolSigner { + private static Map props; + private ImmutableSet sigAlgWhiteList = null; + + private static final ImmutableSet ALLOWED_ALGORITHMS_FOR_VERIFYING = + ImmutableSet.of(SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA256, + SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA384, + SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA512, + // RIPEMD is allowed to verify + SignatureConstants.ALGO_ID_SIGNATURE_RSA_RIPEMD160, + SignatureConstants.ALGO_ID_SIGNATURE_ECDSA_SHA256, + SignatureConstants.ALGO_ID_SIGNATURE_ECDSA_SHA384, + SignatureConstants.ALGO_ID_SIGNATURE_ECDSA_SHA512, + + //Set other algorithms which are not supported by openSAML in default + StringUtils.lowerCase(XMLSignature.ALGO_ID_SIGNATURE_RSA_SHA1_MGF1, Locale.ENGLISH), + StringUtils.lowerCase(XMLSignature.ALGO_ID_SIGNATURE_RSA_SHA224_MGF1, Locale.ENGLISH), + StringUtils.lowerCase(XMLSignature.ALGO_ID_SIGNATURE_RSA_SHA256_MGF1, Locale.ENGLISH), + StringUtils.lowerCase(XMLSignature.ALGO_ID_SIGNATURE_RSA_SHA384_MGF1, Locale.ENGLISH), + StringUtils.lowerCase(XMLSignature.ALGO_ID_SIGNATURE_RSA_SHA512_MGF1, Locale.ENGLISH)); + + private static final ImmutableSet DEFAULT_ALGORITHM_WHITE_LIST = + ImmutableSet.of(SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA256, + SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA384, + SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA512, + // RIPEMD is not allowed to sign + SignatureConstants.ALGO_ID_SIGNATURE_ECDSA_SHA256, + SignatureConstants.ALGO_ID_SIGNATURE_ECDSA_SHA384, + SignatureConstants.ALGO_ID_SIGNATURE_ECDSA_SHA512, + + //Set other algorithms which are not supported by openSAML in default + StringUtils.lowerCase(XMLSignature.ALGO_ID_SIGNATURE_RSA_SHA256_MGF1, Locale.ENGLISH)); + public MOASWSigner(Map properties) throws SamlEngineConfigurationException { super(properties); - + props = properties; + } /** * @param configManager * @throws SamlEngineConfigurationException */ - public MOASWSigner(CertificateConfigurationManager configManager) throws SamlEngineConfigurationException { - super(ConfigurationAdapter.adapt(configManager).getInstances().get(Constants.eIDAS_SAML_ENGINE_NAME).getConfigurationEntries().get(ConfigurationKey.SIGNATURE_CONFIGURATION.getKey()).getParameters()); + public MOASWSigner(CertificateConfigurationManager configManager) throws SamlEngineConfigurationException { + super(props = ConfigurationAdapter.adapt(configManager).getInstances().get(Constants.eIDAS_SAML_ENGINE_NAME).getConfigurationEntries().get(ConfigurationKey.SIGNATURE_CONFIGURATION.getKey()).getParameters()); } + @Override + protected ImmutableSet getSignatureAlgorithmWhiteList() { + try { + if (sigAlgWhiteList == null) { + sigAlgWhiteList = MOAWhiteListConfigurator.getAllowedAlgorithms(DEFAULT_ALGORITHM_WHITE_LIST, + ALLOWED_ALGORITHMS_FOR_VERIFYING, + (new KeyStoreSignatureConfigurator().getSignatureConfiguration(props)).getSignatureAlgorithmWhiteList()); + + } + + return sigAlgWhiteList; + + } catch (SamlEngineConfigurationException e) { + Logger.warn("Can not parse eIDAS signing configuration." , e); + return DEFAULT_ALGORITHM_WHITE_LIST; + + } + } } diff --git a/id/server/modules/moa-id-module-eIDAS/src/main/java/at/gv/egovernment/moa/id/auth/modules/eidas/utils/MOAWhiteListConfigurator.java b/id/server/modules/moa-id-module-eIDAS/src/main/java/at/gv/egovernment/moa/id/auth/modules/eidas/utils/MOAWhiteListConfigurator.java new file mode 100644 index 000000000..7d647ff15 --- /dev/null +++ b/id/server/modules/moa-id-module-eIDAS/src/main/java/at/gv/egovernment/moa/id/auth/modules/eidas/utils/MOAWhiteListConfigurator.java @@ -0,0 +1,96 @@ +/* + * Copyright 2014 Federal Chancellery Austria + * MOA-ID has been developed in a cooperation between BRZ, the Federal + * Chancellery Austria - ICT staff unit, and Graz University of Technology. + * + * Licensed under the EUPL, Version 1.1 or - as soon they will be approved by + * the European Commission - subsequent versions of the EUPL (the "Licence"); + * You may not use this work except in compliance with the Licence. + * You may obtain a copy of the Licence at: + * http://www.osor.eu/eupl/ + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the Licence is distributed on an "AS IS" basis, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the Licence for the specific language governing permissions and + * limitations under the Licence. + * + * This product combines work with different licenses. See the "NOTICE" text + * file for details on the various modules and licenses. + * The "NOTICE" text file is part of the distribution. Any derivative works + * that you distribute must include a readable copy of the "NOTICE" text file. + */ +package at.gv.egovernment.moa.id.auth.modules.eidas.utils; + +import java.util.Locale; +import java.util.regex.Pattern; + +import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang.StringUtils; + +import com.google.common.collect.ImmutableSet; + +import at.gv.egovernment.moa.id.commons.utils.KeyValueUtils; + +/** + * @author tlenz + * + */ +public class MOAWhiteListConfigurator { + private static final Pattern WHITE_LIST_SPLITTER = Pattern.compile("[;,]"); + + + public static ImmutableSet getAllowedAlgorithms( ImmutableSet defaultWhiteList, + ImmutableSet allowedValues, + String algorithmWhiteListValue) { + if (StringUtils.isBlank(algorithmWhiteListValue)) { + return defaultWhiteList; + } + ImmutableSet.Builder allowed = ImmutableSet.builder(); + String[] wlAlgorithms = WHITE_LIST_SPLITTER.split(algorithmWhiteListValue); + if (null != wlAlgorithms && wlAlgorithms.length > 0) { + return getAllowedAlgorithms(defaultWhiteList, allowedValues, ImmutableSet.copyOf(wlAlgorithms)); + } + return defaultWhiteList; + } + + + public static ImmutableSet getAllowedAlgorithms( ImmutableSet defaultWhiteList, + ImmutableSet allowedValues, + ImmutableSet candidateValues) { + if (CollectionUtils.isEmpty(candidateValues)) { + return defaultWhiteList; + } + ImmutableSet.Builder allowed = ImmutableSet.builder(); + boolean modified = false; + for (String candidateValue : candidateValues) { + + /**FIX: + * fix problem with lowerCase and MGF1 signature algorithms + * + */ + candidateValue = StringUtils.trimToNull( + KeyValueUtils.removeAllNewlineFromString(candidateValue)); + if (StringUtils.isNotBlank(candidateValue)) { + String candidateAlgorithm = StringUtils.lowerCase(candidateValue, Locale.ENGLISH); + if (allowedValues.contains(candidateAlgorithm)) { + allowed.add(candidateValue); + if (!modified && !candidateAlgorithm.equals(candidateValue)) { + modified = true; + } + } else { + modified = true; + } + } + } + if (!modified) { + return candidateValues; + } + ImmutableSet set = allowed.build(); + if (set.isEmpty()) { + return defaultWhiteList; + } + return set; + } + +} -- cgit v1.2.3