Merge branch 'master' into fix-injection

Author: pitbulk

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.

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);
 				}
 			}

core/src/main/java/com/onelogin/saml2/logout/LogoutRequest.java

@@ -456,6 +456,12 @@ public Boolean isValid() throws Exception {
 				if (signAlg == null || signAlg.isEmpty()) {
 					signAlg = Constants.RSA_SHA1;
 				}
+
+				Boolean rejectDeprecatedAlg = settings.getRejectDeprecatedAlg();
+				if (Util.mustRejectDeprecatedSignatureAlgo(signAlg, rejectDeprecatedAlg)) {
+					return false;
+				}
+
 				String relayState = request.getEncodedParameter("RelayState");
 
 				String signedQuery = "SAMLRequest=" + request.getEncodedParameter("SAMLRequest");

core/src/main/java/com/onelogin/saml2/logout/LogoutResponse.java

@@ -258,6 +258,11 @@ public Boolean isValid(String requestId) {
 					signAlg = Constants.RSA_SHA1;
 				}
 
+				Boolean rejectDeprecatedAlg = settings.getRejectDeprecatedAlg();
+				if (Util.mustRejectDeprecatedSignatureAlgo(signAlg, rejectDeprecatedAlg)) {
+					return false;
+				}
+
 				String signedQuery = "SAMLResponse=" + request.getEncodedParameter("SAMLResponse");
 
 				String relayState = request.getEncodedParameter("RelayState");

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
 	 *

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";
@@ -401,6 +402,11 @@ private void loadSecuritySetting() {
 		if (allowRepeatAttributeName != null) {
 			saml2Setting.setAllowRepeatAttributeName(allowRepeatAttributeName);
 		}
+
+		Boolean rejectDeprecatedAlg = loadBooleanProperty(SECURITY_REJECT_DEPRECATED_ALGORITHM);
+		if (rejectDeprecatedAlg != null) {
+			saml2Setting.setRejectDeprecatedAlg(rejectDeprecatedAlg);
+		}
 	}
 
 	/**

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<String> 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();
@@ -923,13 +926,30 @@ public static boolean validateSign(final Document doc, final X509Certificate cer
 	 */
 	public static boolean validateSign(final Document doc, final List<X509Certificate> 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<X509Certificate> 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<String,Object> signatureData = getSignatureData(signNode, alg);
+				Map<String,Object> signatureData = getSignatureData(signNode, alg, rejectDeprecatedAlg);
 				if (signatureData.isEmpty()) {
 					return false;
 				}
@@ -984,6 +1004,26 @@ public static boolean validateSign(final Document doc, final List<X509Certificat
      * @return True if the sign is valid, false otherwise.
      */
     public static Boolean validateMetadataSign(Document doc, X509Certificate cert, String fingerprint, String alg) {
+		return validateMetadataSign(doc, cert, fingerprint, 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.
+     */
+    public static Boolean validateMetadataSign(Document doc, X509Certificate cert, String fingerprint, String alg, Boolean rejectDeprecatedAlg) {
         NodeList signNodesToValidate;
 		try {
 			signNodesToValidate = query(doc, "/md:EntitiesDescriptor/ds:Signature");
@@ -999,7 +1039,7 @@ public static Boolean validateMetadataSign(Document doc, X509Certificate cert, S
 			if (signNodesToValidate.getLength() > 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 +1066,26 @@ public static Boolean validateMetadataSign(Document doc, X509Certificate cert, S
      * @return True if the sign is valid, false otherwise.
      */
     private static Map<String,Object> 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<String,Object> getSignatureData(Node signNode, String alg, Boolean rejectDeprecatedAlg) {
 		Map<String,Object> signatureData = new HashMap<>();
 		try {
 			Element sigElement = (Element) signNode;
@@ -1036,6 +1096,10 @@ private static Map<String,Object> getSignatureData(Node signNode, String alg) {
 				throw new Exception(sigMethodAlg + " is not a valid supported algorithm");
 			}
 
+			if (Util.mustRejectDeprecatedSignatureAlgo(sigMethodAlg, rejectDeprecatedAlg)) {
+				return signatureData;
+			}
+
 			signatureData.put("signature", signature);
 
 			String extractedFingerprint = null;
@@ -1056,6 +1120,19 @@ private static Map<String,Object> getSignatureData(Node signNode, String alg) {
 		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.
 	 *
@@ -1073,7 +1150,29 @@ private static Map<String,Object> getSignatureData(Node signNode, String alg) {
 	 * @throws Exception
 	 */
 	public static Boolean validateSignNode(Node signNode, X509Certificate cert, String fingerprint, String alg) {
-		Map<String,Object> 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<String,Object> signatureData = getSignatureData(signNode, alg, rejectDeprecatedAlg);
 		if (signatureData.isEmpty()) {
 			return false;
 		}

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