From 1d798744944027aaae92801917364280b662733b Mon Sep 17 00:00:00 2001 From: Sixto Martin Date: Fri, 20 Nov 2020 17:13:26 +0100 Subject: [PATCH 1/2] Set SHA-256 as default alg in settings. Support Alg Deprecated rejection. Notify with Logger.info if sha-1 alg used on Signature --- README.md | 16 +++- .../onelogin/saml2/authn/SamlResponse.java | 6 +- .../onelogin/saml2/logout/LogoutRequest.java | 11 +++ .../onelogin/saml2/logout/LogoutResponse.java | 10 ++ .../saml2/settings/Saml2Settings.java | 20 +++- .../saml2/settings/SettingsBuilder.java | 9 +- .../java/com/onelogin/saml2/util/Util.java | 94 ++++++++++++++++++- .../saml2/test/authn/AuthnResponseTest.java | 49 ++++++++++ .../saml2/test/logout/LogoutRequestTest.java | 34 +++++++ .../saml2/test/logout/LogoutResponseTest.java | 37 ++++++++ .../onelogin/saml2/test/util/UtilsTest.java | 6 ++ .../metadata/signed_metadata_settings256.xml | 38 ++++++++ .../main/resources/onelogin.saml.properties | 14 ++- 13 files changed, 333 insertions(+), 11 deletions(-) create mode 100644 core/src/test/resources/data/metadata/signed_metadata_settings256.xml diff --git a/README.md b/README.md index e91b0eda..07a34129 100644 --- a/README.md +++ b/README.md @@ -282,7 +282,7 @@ onelogin.saml2.idp.x509cert = # let the toolkit know which Algorithm was used. Possible values: sha1, sha256, sha384 or sha512 # 'sha1' is the default value. # onelogin.saml2.idp.certfingerprint = -# onelogin.saml2.idp.certfingerprint_algorithm = sha1 +# onelogin.saml2.idp.certfingerprint_algorithm = sha256 # Security settings # @@ -342,7 +342,18 @@ onelogin.saml2.security.want_xml_validation = true # 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256' # 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha384' # 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha512' -onelogin.saml2.security.signature_algorithm = http://www.w3.org/2000/09/xmldsig#rsa-sha1 +onelogin.saml2.security.signature_algorithm = http://www.w3.org/2001/04/xmldsig-more#rsa-sha256 + +# Algorithm that the toolkit will use on digest process. Options: +# 'http://www.w3.org/2000/09/xmldsig#sha1' +# 'http://www.w3.org/2001/04/xmlenc#sha256' +# 'http://www.w3.org/2001/04/xmldsig-more#sha384' +# 'http://www.w3.org/2001/04/xmlenc#sha512' +onelogin.saml2.security.digest_algorithm = http://www.w3.org/2001/04/xmlenc#sha256 + + +# Reject Signatures with deprecated algorithms (sha1) +onelogin.saml2.security.reject_deprecated_alg = true # Organization onelogin.saml2.organization.name = SP Java @@ -362,6 +373,7 @@ onelogin.saml2.contacts.support.email_address = support@example.com # onelogin.saml2.unique_id_prefix = _ ``` + ##### KeyStores The Auth constructor supports the ability to read SP public cert/private key from a KeyStore. A KeyStoreSettings object must be provided with the KeyStore, the Alias and the KeyEntry password. diff --git a/core/src/main/java/com/onelogin/saml2/authn/SamlResponse.java b/core/src/main/java/com/onelogin/saml2/authn/SamlResponse.java index 29136337..7f38b81c 100644 --- a/core/src/main/java/com/onelogin/saml2/authn/SamlResponse.java +++ b/core/src/main/java/com/onelogin/saml2/authn/SamlResponse.java @@ -323,12 +323,14 @@ public boolean isValid(String requestId) { String fingerprint = settings.getIdpCertFingerprint(); String alg = settings.getIdpCertFingerprintAlgorithm(); - if (hasSignedResponse && !Util.validateSign(samlResponseDocument, certList, fingerprint, alg, Util.RESPONSE_SIGNATURE_XPATH)) { + Boolean rejectDeprecatedAlg = settings.getRejectDeprecatedAlg(); + + if (hasSignedResponse && !Util.validateSign(samlResponseDocument, certList, fingerprint, alg, Util.RESPONSE_SIGNATURE_XPATH, rejectDeprecatedAlg)) { throw new ValidationError("Signature validation failed. SAML Response rejected", ValidationError.INVALID_SIGNATURE); } final Document documentToCheckAssertion = encrypted ? decryptedDocument : samlResponseDocument; - if (hasSignedAssertion && !Util.validateSign(documentToCheckAssertion, certList, fingerprint, alg, Util.ASSERTION_SIGNATURE_XPATH)) { + if (hasSignedAssertion && !Util.validateSign(documentToCheckAssertion, certList, fingerprint, alg, Util.ASSERTION_SIGNATURE_XPATH, rejectDeprecatedAlg)) { throw new ValidationError("Signature validation failed. SAML Response rejected", ValidationError.INVALID_SIGNATURE); } } diff --git a/core/src/main/java/com/onelogin/saml2/logout/LogoutRequest.java b/core/src/main/java/com/onelogin/saml2/logout/LogoutRequest.java index a9431136..9ae8499c 100644 --- a/core/src/main/java/com/onelogin/saml2/logout/LogoutRequest.java +++ b/core/src/main/java/com/onelogin/saml2/logout/LogoutRequest.java @@ -456,6 +456,17 @@ public Boolean isValid() throws Exception { if (signAlg == null || signAlg.isEmpty()) { signAlg = Constants.RSA_SHA1; } + + if (signAlg.equals(Constants.RSA_SHA1)) { + Boolean rejectDeprecatedAlg = settings.getRejectDeprecatedAlg(); + if (rejectDeprecatedAlg) { + LOGGER.error("A deprecated algorithm (RSA_SHA1) found in the Signature element, rejecting it"); + return false; + } else { + LOGGER.info("RSA_SHA1 alg found in a Signature element, consider request a more robust alg"); + } + } + String relayState = request.getEncodedParameter("RelayState"); String signedQuery = "SAMLRequest=" + request.getEncodedParameter("SAMLRequest"); diff --git a/core/src/main/java/com/onelogin/saml2/logout/LogoutResponse.java b/core/src/main/java/com/onelogin/saml2/logout/LogoutResponse.java index 6d2b990e..9cf14e7c 100644 --- a/core/src/main/java/com/onelogin/saml2/logout/LogoutResponse.java +++ b/core/src/main/java/com/onelogin/saml2/logout/LogoutResponse.java @@ -258,6 +258,16 @@ public Boolean isValid(String requestId) { signAlg = Constants.RSA_SHA1; } + if (signAlg.equals(Constants.RSA_SHA1)) { + Boolean rejectDeprecatedAlg = settings.getRejectDeprecatedAlg(); + if (rejectDeprecatedAlg) { + LOGGER.error("A deprecated algorithm (RSA_SHA1) found in the Signature element, rejecting it"); + return false; + } else { + LOGGER.info("RSA_SHA1 alg found in a Signature element, consider request a more robust alg"); + } + } + String signedQuery = "SAMLResponse=" + request.getEncodedParameter("SAMLResponse"); String relayState = request.getEncodedParameter("RelayState"); diff --git a/core/src/main/java/com/onelogin/saml2/settings/Saml2Settings.java b/core/src/main/java/com/onelogin/saml2/settings/Saml2Settings.java index a47c273a..f5c8565b 100644 --- a/core/src/main/java/com/onelogin/saml2/settings/Saml2Settings.java +++ b/core/src/main/java/com/onelogin/saml2/settings/Saml2Settings.java @@ -76,6 +76,7 @@ public class Saml2Settings { private String digestAlgorithm = Constants.SHA1; private boolean rejectUnsolicitedResponsesWithInResponseTo = false; private boolean allowRepeatAttributeName = false; + private boolean rejectDeprecatedAlg = false; private String uniqueIDPrefix = null; // Compress @@ -140,10 +141,17 @@ public final String getSpNameIDFormat() { /** * @return the allowRepeatAttributeName setting value */ - public boolean isAllowRepeatAttributeName () { + public boolean isAllowRepeatAttributeName() { return allowRepeatAttributeName; } + /** + * @return the rejectDeprecatedAlg setting value + */ + public boolean getRejectDeprecatedAlg() { + return rejectDeprecatedAlg; + } + /** * @return the spX509cert setting value */ @@ -478,6 +486,16 @@ public void setAllowRepeatAttributeName (boolean allowRepeatAttributeName) { this.allowRepeatAttributeName = allowRepeatAttributeName; } + /** + * Set the rejectDeprecatedAlg setting value + * + * @param rejectDeprecatedAlg + * the rejectDeprecatedAlg value to be set + */ + public void setRejectDeprecatedAlg (boolean rejectDeprecatedAlg) { + this.rejectDeprecatedAlg = rejectDeprecatedAlg; + } + /** * Set the spX509cert setting value provided as X509Certificate object * diff --git a/core/src/main/java/com/onelogin/saml2/settings/SettingsBuilder.java b/core/src/main/java/com/onelogin/saml2/settings/SettingsBuilder.java index 802ed322..1b30a4ec 100644 --- a/core/src/main/java/com/onelogin/saml2/settings/SettingsBuilder.java +++ b/core/src/main/java/com/onelogin/saml2/settings/SettingsBuilder.java @@ -101,6 +101,7 @@ public class SettingsBuilder { public final static String SECURITY_DIGEST_ALGORITHM = "onelogin.saml2.security.digest_algorithm"; public final static String SECURITY_REJECT_UNSOLICITED_RESPONSES_WITH_INRESPONSETO = "onelogin.saml2.security.reject_unsolicited_responses_with_inresponseto"; public final static String SECURITY_ALLOW_REPEAT_ATTRIBUTE_NAME_PROPERTY_KEY = "onelogin.saml2.security.allow_duplicated_attribute_name"; + public final static String SECURITY_REJECT_DEPRECATED_ALGORITHM = "onelogin.saml2.security.reject_deprecated_alg"; // Compress public final static String COMPRESS_REQUEST = "onelogin.saml2.compress.request"; @@ -376,8 +377,14 @@ private void loadSecuritySetting() { } Boolean allowRepeatAttributeName = loadBooleanProperty(SECURITY_ALLOW_REPEAT_ATTRIBUTE_NAME_PROPERTY_KEY); - if (allowRepeatAttributeName != null) + if (allowRepeatAttributeName != null) { saml2Setting.setAllowRepeatAttributeName(allowRepeatAttributeName); + } + + Boolean rejectDeprecatedAlg = loadBooleanProperty(SECURITY_REJECT_DEPRECATED_ALGORITHM); + if (rejectDeprecatedAlg != null) { + saml2Setting.setRejectDeprecatedAlg(rejectDeprecatedAlg); + } } /** diff --git a/core/src/main/java/com/onelogin/saml2/util/Util.java b/core/src/main/java/com/onelogin/saml2/util/Util.java index 883ea55d..6b72894b 100644 --- a/core/src/main/java/com/onelogin/saml2/util/Util.java +++ b/core/src/main/java/com/onelogin/saml2/util/Util.java @@ -923,13 +923,30 @@ public static boolean validateSign(final Document doc, final X509Certificate cer */ public static boolean validateSign(final Document doc, final List certList, final String fingerprint, final String alg, final String xpath) { + return validateSign(doc, certList, fingerprint,alg, xpath, false); + } + + /** + * Validate the signature pointed to by the xpath + * + * @param doc The document we should validate + * @param certList The public certificates + * @param fingerprint The fingerprint of the public certificate + * @param alg The signature algorithm method + * @param xpath the xpath of the ds:Signture node to validate + * @param rejectDeprecatedAlg Flag to invalidate or not Signatures with deprecated alg + * + * @return True if the signature exists and is valid, false otherwise. + */ + public static boolean validateSign(final Document doc, final List certList, final String fingerprint, + final String alg, final String xpath, final Boolean rejectDeprecatedAlg) { try { final NodeList signatures = query(doc, xpath); if (signatures.getLength() == 1) { final Node signNode = signatures.item(0); - Map signatureData = getSignatureData(signNode, alg); + Map signatureData = getSignatureData(signNode, alg, rejectDeprecatedAlg); if (signatureData.isEmpty()) { return false; } @@ -984,6 +1001,26 @@ public static boolean validateSign(final Document doc, final List 0) { for (int i = 0; i < signNodesToValidate.getLength(); i++) { Node signNode = signNodesToValidate.item(i); - if (!validateSignNode(signNode, cert, fingerprint, alg)) { + if (!validateSignNode(signNode, cert, fingerprint, alg, rejectDeprecatedAlg)) { return false; } } @@ -1026,6 +1063,26 @@ public static Boolean validateMetadataSign(Document doc, X509Certificate cert, S * @return True if the sign is valid, false otherwise. */ private static Map getSignatureData(Node signNode, String alg) { + return getSignatureData(signNode, alg, false); + } + + /** + * Validate signature (Metadata). + * + * @param doc + * The document we should validate + * @param cert + * The public certificate + * @param fingerprint + * The fingerprint of the public certificate + * @param alg + * The signature algorithm method + * @param rejectDeprecatedAlg + * Flag to invalidate or not Signatures with deprecated alg + * + * @return True if the sign is valid, false otherwise. + */ + private static Map getSignatureData(Node signNode, String alg, Boolean rejectDeprecatedAlg) { Map signatureData = new HashMap<>(); try { Element sigElement = (Element) signNode; @@ -1036,6 +1093,15 @@ private static Map getSignatureData(Node signNode, String alg) { throw new Exception(sigMethodAlg + " is not a valid supported algorithm"); } + if (sigMethodAlg.equals(Constants.RSA_SHA1)) { + if (rejectDeprecatedAlg) { + LOGGER.error("A deprecated algorithm (RSA_SHA1) found in the Signature element, rejecting it"); + return signatureData; + } else { + LOGGER.info("RSA_SHA1 alg found in a Signature element, consider request a more robust alg"); + } + } + signatureData.put("signature", signature); String extractedFingerprint = null; @@ -1073,7 +1139,29 @@ private static Map getSignatureData(Node signNode, String alg) { * @throws Exception */ public static Boolean validateSignNode(Node signNode, X509Certificate cert, String fingerprint, String alg) { - Map signatureData = getSignatureData(signNode, alg); + return validateSignNode(signNode, cert, fingerprint, alg, false); + } + + /** + * Validate signature of the Node. + * + * @param signNode + * The document we should validate + * @param cert + * The public certificate + * @param fingerprint + * The fingerprint of the public certificate + * @param alg + * The signature algorithm method + * @param rejectDeprecatedAlg + * Flag to invalidate or not Signatures with deprecated alg + * + * @return True if the sign is valid, false otherwise. + * + * @throws Exception + */ + public static Boolean validateSignNode(Node signNode, X509Certificate cert, String fingerprint, String alg, Boolean rejectDeprecatedAlg) { + Map signatureData = getSignatureData(signNode, alg, rejectDeprecatedAlg); if (signatureData.isEmpty()) { return false; } diff --git a/core/src/test/java/com/onelogin/saml2/test/authn/AuthnResponseTest.java b/core/src/test/java/com/onelogin/saml2/test/authn/AuthnResponseTest.java index bfb2af8e..cae24ce2 100644 --- a/core/src/test/java/com/onelogin/saml2/test/authn/AuthnResponseTest.java +++ b/core/src/test/java/com/onelogin/saml2/test/authn/AuthnResponseTest.java @@ -5,6 +5,7 @@ import com.onelogin.saml2.exception.SettingsException; import com.onelogin.saml2.exception.ValidationError; import com.onelogin.saml2.http.HttpRequest; +import com.onelogin.saml2.logout.LogoutRequest; import com.onelogin.saml2.model.SamlResponseStatus; import com.onelogin.saml2.settings.Saml2Settings; import com.onelogin.saml2.settings.SettingsBuilder; @@ -2409,6 +2410,54 @@ public void testIsValid_signedEncryptedAssertion() throws Exception { assertResponseValid(settings, samlResponseEncoded, true, false, "The Message of the Response is not signed and the SP requires it"); } + /** + * Tests the isValid method of SamlResponse + * Case: Signed with deprecated method and flag enabled + * + * @throws ValidationError + * @throws SettingsException + * @throws IOException + * @throws SAXException + * @throws ParserConfigurationException + * @throws XPathExpressionException + * @throws Error + * + * @see com.onelogin.saml2.authn.SamlResponse#isValid + */ + @Test + public void testIsInValidSignWithDeprecatedAlg() throws IOException, Error, XPathExpressionException, ParserConfigurationException, SAXException, SettingsException, ValidationError { + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.my.properties").build(); + settings.setWantAssertionsSigned(false); + settings.setWantMessagesSigned(false); + String samlResponseEncoded = Util.getFileAsString("data/responses/signed_message_response.xml.base64"); + + settings.setStrict(true); + SamlResponse samlResponse = new SamlResponse(settings, newHttpRequest(samlResponseEncoded)); + assertTrue(samlResponse.isValid()); + + settings.setRejectDeprecatedAlg(true); + SamlResponse samlResponse2 = new SamlResponse(settings, newHttpRequest(samlResponseEncoded)); + assertFalse(samlResponse2.isValid()); + + settings.setRejectDeprecatedAlg(false); + samlResponseEncoded = Util.getFileAsString("data/responses/signed_assertion_response.xml.base64"); + SamlResponse samlResponse3 = new SamlResponse(settings, newHttpRequest(samlResponseEncoded)); + assertTrue(samlResponse3.isValid()); + + settings.setRejectDeprecatedAlg(true); + SamlResponse samlResponse4 = new SamlResponse(settings, newHttpRequest(samlResponseEncoded)); + assertFalse(samlResponse4.isValid()); + + settings.setRejectDeprecatedAlg(false); + samlResponseEncoded = Util.getFileAsString("data/responses/double_signed_response.xml.base64"); + SamlResponse samlResponse5 = new SamlResponse(settings, newHttpRequest(samlResponseEncoded)); + assertTrue(samlResponse5.isValid()); + + settings.setRejectDeprecatedAlg(true); + SamlResponse samlResponse6 = new SamlResponse(settings, newHttpRequest(samlResponseEncoded)); + assertFalse(samlResponse6.isValid()); + } + /** * Tests the isValid method of SamlResponse * Case: valid sign response / sign assertion / both signed diff --git a/core/src/test/java/com/onelogin/saml2/test/logout/LogoutRequestTest.java b/core/src/test/java/com/onelogin/saml2/test/logout/LogoutRequestTest.java index a8e73d76..cf9fab1f 100644 --- a/core/src/test/java/com/onelogin/saml2/test/logout/LogoutRequestTest.java +++ b/core/src/test/java/com/onelogin/saml2/test/logout/LogoutRequestTest.java @@ -766,6 +766,40 @@ public void testIsInValidSign() throws Exception { assertEquals("In order to validate the sign on the Logout Request, the x509cert of the IdP is required", logoutRequest.getError()); } + /** + * Tests the isValid method of LogoutRequest + * Case: Signed with deprecated method and flag enabled + * + * @throws Exception + * + * @see com.onelogin.saml2.logout.LogoutRequest#isValid + */ + @Test + public void testIsInValidSignWithDeprecatedAlg() throws Exception { + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.my.properties").build(); + settings.setStrict(false); + settings.setWantMessagesSigned(true); + + final String requestURL = "https://pitbulk.no-ip.org/newonelogin/demo1/index.php?sls"; + String samlRequestEncoded = "lVLBitswEP0Vo7tjeWzJtki8LIRCYLvbNksPewmyPc6K2pJqyXQ/v1LSQlroQi/DMJr33rwZbZ2cJysezNms/gt+X9H55G2etBOXlx1ZFy2MdMoJLWd0wvfieP/xQcCGCrsYb3ozkRvI+wjpHC5eGU2Sw35HTg3lA8hqZFwWFcMKsStpxbEsxoLXeQN9OdY1VAgk+YqLC8gdCUQB7tyKB+281D6UaF6mtEiBPudcABcMXkiyD26Ulv6CevXeOpFlVvlunb5ttEmV3ZjlnGn8YTRO5qx0NuBs8kzpAd829tXeucmR5NH4J/203I8el6gFRUqbFPJnyEV51Wq30by4TLW0/9ZyarYTxt4sBsjUYLMZvRykl1Fxm90SXVkfwx4P++T4KSafVzmpUcVJ/sfSrQZJPphllv79W8WKGtLx0ir8IrVTqD1pT2MH3QAMSs4KTvui71jeFFiwirOmprwPkYW063+5uRq4urHiiC4e8hCX3J5wqAEGaPpw9XB5JmkBdeDqSlkz6CmUXdl0Qae5kv2F/1384wu3PwE="; + String relayState = "_1037fbc88ec82ce8e770b2bed1119747bb812a07e6"; + String sigAlg = "http://www.w3.org/2000/09/xmldsig#rsa-sha1"; + String signature = "XCwCyI5cs7WhiJlB5ktSlWxSBxv+6q2xT3c8L7dLV6NQG9LHWhN7gf8qNsahSXfCzA0Ey9dp5BQ0EdRvAk2DIzKmJY6e3hvAIEp1zglHNjzkgcQmZCcrkK9Czi2Y1WkjOwR/WgUTUWsGJAVqVvlRZuS3zk3nxMrLH6f7toyvuJc="; + + HttpRequest httpRequest = new HttpRequest(requestURL, (String)null) + .addParameter("SAMLRequest", samlRequestEncoded) + .addParameter("RelayState", relayState) + .addParameter("SigAlg", sigAlg) + .addParameter("Signature", signature); + LogoutRequest logoutRequest = new LogoutRequest(settings, httpRequest); + logoutRequest = new LogoutRequest(settings, httpRequest); + assertTrue(logoutRequest.isValid()); + + settings.setRejectDeprecatedAlg(true); + LogoutRequest logoutRequest2 = new LogoutRequest(settings, httpRequest); + assertFalse(logoutRequest2.isValid()); + } + /** * Tests the isValid method of LogoutRequest * Case: No SAML Logout Request diff --git a/core/src/test/java/com/onelogin/saml2/test/logout/LogoutResponseTest.java b/core/src/test/java/com/onelogin/saml2/test/logout/LogoutResponseTest.java index f6f14c58..fe04cb7c 100644 --- a/core/src/test/java/com/onelogin/saml2/test/logout/LogoutResponseTest.java +++ b/core/src/test/java/com/onelogin/saml2/test/logout/LogoutResponseTest.java @@ -552,6 +552,43 @@ public void testIsInValidSign() throws URISyntaxException, IOException, XMLEntit assertEquals("In order to validate the sign on the Logout Response, the x509cert of the IdP is required", logoutResponse.getError()); } + /** + * Tests the isValid method of LogoutResponse + * Case: Signed with deprecated method and flag enabled + * + * @throws IOException + * @throws URISyntaxException + * @throws XMLEntityException + * @throws Error + * + * @see com.onelogin.saml2.logout.LogoutResponse#isValid + */ + @Test + public void testIsInValidSignWithDeprecatedAlg() throws URISyntaxException, IOException, XMLEntityException, Error { + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.my.properties").build(); + settings.setStrict(false); + settings.setWantMessagesSigned(true); + + final String requestURL = "https://pitbulk.no-ip.org/newonelogin/demo1/index.php?sls"; + String samlResponseEncoded = "fZJva8IwEMa/Ssl7TZrW/gnqGHMMwSlM8cXeyLU9NaxNQi9lfvxVZczB5ptwSe733MPdjQma2qmFPdjOvyE5awiDU1MbUpevCetaoyyQJmWgQVK+VOvH14WSQ6Fca70tbc1ukPsEEGHrtTUsmM8mbDfKUhnFci8gliGINI/yXIAAiYnsw6JIRgWWAKlkwRZb6skJ64V6nKjDuSEPxvdPIowHIhpIsQkTFaYqSt9ZMEPy2oC/UEfvHSnOnfZFV38MjR1oN7TtgRv8tAZre9CGV9jYkGtT4Wnoju6Bauprme/ebOyErZbPi9XLfLnDoohwhHGc5WVSVhjCKM6rBMpYQpWJrIizfZ4IZNPxuTPqYrmd/m+EdONqPOfy8yG5rhxv0EMFHs52xvxWaHyd3tqD7+j37clWGGyh7vD+POiSrdZdWSIR49NrhR9R/teGTL8A"; + String relayState = "https://pitbulk.no-ip.org/newonelogin/demo1/index.php"; + String sigAlg = "http://www.w3.org/2000/09/xmldsig#rsa-sha1"; + String signature = "vfWbbc47PkP3ejx4bjKsRX7lo9Ml1WRoE5J5owF/0mnyKHfSY6XbhO1wwjBV5vWdrUVX+xp6slHyAf4YoAsXFS0qhan6txDiZY4Oec6yE+l10iZbzvie06I4GPak4QrQ4gAyXOSzwCrRmJu4gnpeUxZ6IqKtdrKfAYRAcVfNKGA="; + + HttpRequest httpRequest = new HttpRequest(requestURL, (String)null) + .addParameter("SAMLResponse", samlResponseEncoded) + .addParameter("RelayState", relayState) + .addParameter("SigAlg", sigAlg) + .addParameter("Signature", signature); + + LogoutResponse logoutResponse = new LogoutResponse(settings, httpRequest); + assertTrue(logoutResponse.isValid()); + + settings.setRejectDeprecatedAlg(true); + LogoutResponse logoutResponse2 = new LogoutResponse(settings, httpRequest); + assertFalse(logoutResponse2.isValid()); + } + /** * Tests the isValid method of LogoutResponse * Case: No SAML Logout Response diff --git a/core/src/test/java/com/onelogin/saml2/test/util/UtilsTest.java b/core/src/test/java/com/onelogin/saml2/test/util/UtilsTest.java index 61616237..b8eb5b97 100644 --- a/core/src/test/java/com/onelogin/saml2/test/util/UtilsTest.java +++ b/core/src/test/java/com/onelogin/saml2/test/util/UtilsTest.java @@ -1234,6 +1234,12 @@ public void testValidateMetadataSign() throws URISyntaxException, IOException, C assertTrue(Util.validateMetadataSign(signedMetadataDocument, null, fingerprint_sha1, null)); assertTrue(Util.validateMetadataSign(signedMetadataDocument, null, fingerprint_sha1, "SHA-1")); assertTrue(Util.validateMetadataSign(signedMetadataDocument, null, fingerprint_sha256, "SHA-256")); + + // Deprecated Alg + String signed256MetadataStr = Util.getFileAsString("data/metadata/signed_metadata_settings256.xml"); + Document signed256MetadataDocument = Util.loadXML(signed256MetadataStr); + assertFalse(Util.validateMetadataSign(signedMetadataDocument, null, fingerprint_sha1, "SHA-1", true)); + assertTrue(Util.validateMetadataSign(signed256MetadataDocument, null, fingerprint_sha1, "SHA-1", true)); } /** diff --git a/core/src/test/resources/data/metadata/signed_metadata_settings256.xml b/core/src/test/resources/data/metadata/signed_metadata_settings256.xml new file mode 100644 index 00000000..3fcb1151 --- /dev/null +++ b/core/src/test/resources/data/metadata/signed_metadata_settings256.xml @@ -0,0 +1,38 @@ + + + + + 4Sefh/fD7F51Eh+fyfx9xiko0Q0=RFlwMeIJf5SB1CV5URpjSdpa9BrtDa/TFN2kUncsQYus/xWuxucbygHhhuCz4/3k4K7cUhFk5x3KtBC0yW4cUXBYagE0qdMpYtfXLDfrQE1rT2TgACUXJh/wWxkqR6NuAbw3Fy9PnxsLKKksO8ZoJZyYl6IgcQmIVx2ii3ACKdQ= +MIICgTCCAeoCCQCbOlrWDdX7FTANBgkqhkiG9w0BAQUFADCBhDELMAkGA1UEBhMCTk8xGDAWBgNVBAgTD0FuZHJlYXMgU29sYmVyZzEMMAoGA1UEBxMDRm9vMRAwDgYDVQQKEwdVTklORVRUMRgwFgYDVQQDEw9mZWlkZS5lcmxhbmcubm8xITAfBgkqhkiG9w0BCQEWEmFuZHJlYXNAdW5pbmV0dC5ubzAeFw0wNzA2MTUxMjAxMzVaFw0wNzA4MTQxMjAxMzVaMIGEMQswCQYDVQQGEwJOTzEYMBYGA1UECBMPQW5kcmVhcyBTb2xiZXJnMQwwCgYDVQQHEwNGb28xEDAOBgNVBAoTB1VOSU5FVFQxGDAWBgNVBAMTD2ZlaWRlLmVybGFuZy5ubzEhMB8GCSqGSIb3DQEJARYSYW5kcmVhc0B1bmluZXR0Lm5vMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDivbhR7P516x/S3BqKxupQe0LONoliupiBOesCO3SHbDrl3+q9IbfnfmE04rNuMcPsIxB161TdDpIesLCn7c8aPHISKOtPlAeTZSnb8QAu7aRjZq3+PbrP5uW3TcfCGPtKTytHOge/OlJbo078dVhXQ14d1EDwXJW1rRXuUt4C8QIDAQABMA0GCSqGSIb3DQEBBQUAA4GBACDVfp86HObqY+e8BUoWQ9+VMQx1ASDohBjwOsg2WykUqRXF+dLfcUH9dWR63CtZIKFDbStNomPnQz7nbK+onygwBspVEbnHuUihZq3ZUdmumQqCw4Uvs/1Uvq3orOo/WJVhTyvLgFVK2QarQ4/67OZfHd7R+POBXhophSMv1ZOo + + + + MIICgTCCAeoCCQCbOlrWDdX7FTANBgkqhkiG9w0BAQUFADCBhDELMAkGA1UEBhMCTk8xGDAWBgNVBAgTD0FuZHJlYXMgU29sYmVyZzEMMAoGA1UEBxMDRm9vMRAwDgYDVQQKEwdVTklORVRUMRgwFgYDVQQDEw9mZWlkZS5lcmxhbmcubm8xITAfBgkqhkiG9w0BCQEWEmFuZHJlYXNAdW5pbmV0dC5ubzAeFw0wNzA2MTUxMjAxMzVaFw0wNzA4MTQxMjAxMzVaMIGEMQswCQYDVQQGEwJOTzEYMBYGA1UECBMPQW5kcmVhcyBTb2xiZXJnMQwwCgYDVQQHEwNGb28xEDAOBgNVBAoTB1VOSU5FVFQxGDAWBgNVBAMTD2ZlaWRlLmVybGFuZy5ubzEhMB8GCSqGSIb3DQEJARYSYW5kcmVhc0B1bmluZXR0Lm5vMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDivbhR7P516x/S3BqKxupQe0LONoliupiBOesCO3SHbDrl3+q9IbfnfmE04rNuMcPsIxB161TdDpIesLCn7c8aPHISKOtPlAeTZSnb8QAu7aRjZq3+PbrP5uW3TcfCGPtKTytHOge/OlJbo078dVhXQ14d1EDwXJW1rRXuUt4C8QIDAQABMA0GCSqGSIb3DQEBBQUAA4GBACDVfp86HObqY+e8BUoWQ9+VMQx1ASDohBjwOsg2WykUqRXF+dLfcUH9dWR63CtZIKFDbStNomPnQz7nbK+onygwBspVEbnHuUihZq3ZUdmumQqCw4Uvs/1Uvq3orOo/WJVhTyvLgFVK2QarQ4/67OZfHd7R+POBXhophSMv1ZOo + + + + + + + MIICgTCCAeoCCQCbOlrWDdX7FTANBgkqhkiG9w0BAQUFADCBhDELMAkGA1UEBhMCTk8xGDAWBgNVBAgTD0FuZHJlYXMgU29sYmVyZzEMMAoGA1UEBxMDRm9vMRAwDgYDVQQKEwdVTklORVRUMRgwFgYDVQQDEw9mZWlkZS5lcmxhbmcubm8xITAfBgkqhkiG9w0BCQEWEmFuZHJlYXNAdW5pbmV0dC5ubzAeFw0wNzA2MTUxMjAxMzVaFw0wNzA4MTQxMjAxMzVaMIGEMQswCQYDVQQGEwJOTzEYMBYGA1UECBMPQW5kcmVhcyBTb2xiZXJnMQwwCgYDVQQHEwNGb28xEDAOBgNVBAoTB1VOSU5FVFQxGDAWBgNVBAMTD2ZlaWRlLmVybGFuZy5ubzEhMB8GCSqGSIb3DQEJARYSYW5kcmVhc0B1bmluZXR0Lm5vMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDivbhR7P516x/S3BqKxupQe0LONoliupiBOesCO3SHbDrl3+q9IbfnfmE04rNuMcPsIxB161TdDpIesLCn7c8aPHISKOtPlAeTZSnb8QAu7aRjZq3+PbrP5uW3TcfCGPtKTytHOge/OlJbo078dVhXQ14d1EDwXJW1rRXuUt4C8QIDAQABMA0GCSqGSIb3DQEBBQUAA4GBACDVfp86HObqY+e8BUoWQ9+VMQx1ASDohBjwOsg2WykUqRXF+dLfcUH9dWR63CtZIKFDbStNomPnQz7nbK+onygwBspVEbnHuUihZq3ZUdmumQqCw4Uvs/1Uvq3orOo/WJVhTyvLgFVK2QarQ4/67OZfHd7R+POBXhophSMv1ZOo + + + + + urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress + + + + sp_test + SP test + http://sp.example.com + + + technical_name + technical@example.com + + + support_name + support@example.com + + \ No newline at end of file diff --git a/samples/java-saml-tookit-jspsample/src/main/resources/onelogin.saml.properties b/samples/java-saml-tookit-jspsample/src/main/resources/onelogin.saml.properties index 4d705fa6..1a071318 100644 --- a/samples/java-saml-tookit-jspsample/src/main/resources/onelogin.saml.properties +++ b/samples/java-saml-tookit-jspsample/src/main/resources/onelogin.saml.properties @@ -85,7 +85,7 @@ onelogin.saml2.idp.x509cert = # let the toolkit know which Algorithm was used. Possible values: sha1, sha256, sha384 or sha512 # 'sha1' is the default value. # onelogin.saml2.idp.certfingerprint = -# onelogin.saml2.idp.certfingerprint_algorithm = sha1 +# onelogin.saml2.idp.certfingerprint_algorithm = sha256 # Security settings @@ -145,7 +145,17 @@ onelogin.saml2.security.want_xml_validation = true # 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256' # 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha384' # 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha512' -onelogin.saml2.security.signature_algorithm = http://www.w3.org/2000/09/xmldsig#rsa-sha1 +onelogin.saml2.security.signature_algorithm = http://www.w3.org/2001/04/xmldsig-more#rsa-sha256 + +# Algorithm that the toolkit will use on digest process. Options: +# 'http://www.w3.org/2000/09/xmldsig#sha1' +# 'http://www.w3.org/2001/04/xmlenc#sha256' +# 'http://www.w3.org/2001/04/xmldsig-more#sha384' +# 'http://www.w3.org/2001/04/xmlenc#sha512' +onelogin.saml2.security.digest_algorithm = http://www.w3.org/2001/04/xmlenc#sha256 + +# Reject Signatures with deprecated algorithms (sha1) +onelogin.saml2.security.reject_deprecated_alg = true # Organization onelogin.saml2.organization.name = SP Java From e6d0501ba967b93832632ee4e963a0017e20c790 Mon Sep 17 00:00:00 2001 From: Sixto Martin Date: Mon, 23 Nov 2020 11:36:57 +0100 Subject: [PATCH 2/2] Improve deprecated-alg code --- .../onelogin/saml2/logout/LogoutRequest.java | 11 +++----- .../onelogin/saml2/logout/LogoutResponse.java | 11 +++----- .../java/com/onelogin/saml2/util/Util.java | 25 +++++++++++++------ 3 files changed, 24 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/com/onelogin/saml2/logout/LogoutRequest.java b/core/src/main/java/com/onelogin/saml2/logout/LogoutRequest.java index 9ae8499c..9caa9b39 100644 --- a/core/src/main/java/com/onelogin/saml2/logout/LogoutRequest.java +++ b/core/src/main/java/com/onelogin/saml2/logout/LogoutRequest.java @@ -457,14 +457,9 @@ public Boolean isValid() throws Exception { signAlg = Constants.RSA_SHA1; } - if (signAlg.equals(Constants.RSA_SHA1)) { - Boolean rejectDeprecatedAlg = settings.getRejectDeprecatedAlg(); - if (rejectDeprecatedAlg) { - LOGGER.error("A deprecated algorithm (RSA_SHA1) found in the Signature element, rejecting it"); - return false; - } else { - LOGGER.info("RSA_SHA1 alg found in a Signature element, consider request a more robust alg"); - } + Boolean rejectDeprecatedAlg = settings.getRejectDeprecatedAlg(); + if (Util.mustRejectDeprecatedSignatureAlgo(signAlg, rejectDeprecatedAlg)) { + return false; } String relayState = request.getEncodedParameter("RelayState"); diff --git a/core/src/main/java/com/onelogin/saml2/logout/LogoutResponse.java b/core/src/main/java/com/onelogin/saml2/logout/LogoutResponse.java index 9cf14e7c..e943d826 100644 --- a/core/src/main/java/com/onelogin/saml2/logout/LogoutResponse.java +++ b/core/src/main/java/com/onelogin/saml2/logout/LogoutResponse.java @@ -258,14 +258,9 @@ public Boolean isValid(String requestId) { signAlg = Constants.RSA_SHA1; } - if (signAlg.equals(Constants.RSA_SHA1)) { - Boolean rejectDeprecatedAlg = settings.getRejectDeprecatedAlg(); - if (rejectDeprecatedAlg) { - LOGGER.error("A deprecated algorithm (RSA_SHA1) found in the Signature element, rejecting it"); - return false; - } else { - LOGGER.info("RSA_SHA1 alg found in a Signature element, consider request a more robust alg"); - } + Boolean rejectDeprecatedAlg = settings.getRejectDeprecatedAlg(); + if (Util.mustRejectDeprecatedSignatureAlgo(signAlg, rejectDeprecatedAlg)) { + return false; } String signedQuery = "SAMLResponse=" + request.getEncodedParameter("SAMLResponse"); diff --git a/core/src/main/java/com/onelogin/saml2/util/Util.java b/core/src/main/java/com/onelogin/saml2/util/Util.java index 6b72894b..1c32e5e4 100644 --- a/core/src/main/java/com/onelogin/saml2/util/Util.java +++ b/core/src/main/java/com/onelogin/saml2/util/Util.java @@ -27,6 +27,7 @@ import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; import java.security.spec.PKCS8EncodedKeySpec; +import java.util.Arrays; import java.util.Calendar; import java.util.HashMap; import java.util.HashSet; @@ -116,6 +117,8 @@ public final class Util { /** Indicates if JAXP 1.5 support has been detected. */ private static boolean JAXP_15_SUPPORTED = isJaxp15Supported(); + private static final Set DEPRECATED_ALGOS = new HashSet<>(Arrays.asList(Constants.RSA_SHA1, Constants.DSA_SHA1)); + static { System.setProperty("org.apache.xml.security.ignoreLineBreaks", "true"); org.apache.xml.security.Init.init(); @@ -1093,13 +1096,8 @@ private static Map getSignatureData(Node signNode, String alg, Bo throw new Exception(sigMethodAlg + " is not a valid supported algorithm"); } - if (sigMethodAlg.equals(Constants.RSA_SHA1)) { - if (rejectDeprecatedAlg) { - LOGGER.error("A deprecated algorithm (RSA_SHA1) found in the Signature element, rejecting it"); - return signatureData; - } else { - LOGGER.info("RSA_SHA1 alg found in a Signature element, consider request a more robust alg"); - } + if (Util.mustRejectDeprecatedSignatureAlgo(sigMethodAlg, rejectDeprecatedAlg)) { + return signatureData; } signatureData.put("signature", signature); @@ -1122,6 +1120,19 @@ private static Map getSignatureData(Node signNode, String alg, Bo return signatureData; } + public static Boolean mustRejectDeprecatedSignatureAlgo(String signAlg, Boolean rejectDeprecatedAlg) { + if (DEPRECATED_ALGOS.contains(signAlg)) { + String errorMsg = "Found a deprecated algorithm "+ signAlg +" related to the Signature element,"; + if (rejectDeprecatedAlg) { + LOGGER.error(errorMsg + " rejecting it"); + return true; + } else { + LOGGER.info(errorMsg + " consider requesting a more robust algorithm"); + } + } + return false; + } + /** * Validate signature of the Node. *