Return-Path: X-Original-To: apmail-cxf-commits-archive@www.apache.org Delivered-To: apmail-cxf-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id A19DD19F5C for ; Mon, 21 Mar 2016 17:03:30 +0000 (UTC) Received: (qmail 76611 invoked by uid 500); 21 Mar 2016 17:03:30 -0000 Delivered-To: apmail-cxf-commits-archive@cxf.apache.org Received: (qmail 76431 invoked by uid 500); 21 Mar 2016 17:03:30 -0000 Mailing-List: contact commits-help@cxf.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cxf.apache.org Delivered-To: mailing list commits@cxf.apache.org Received: (qmail 76363 invoked by uid 99); 21 Mar 2016 17:03:30 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 21 Mar 2016 17:03:30 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 50702DFF68; Mon, 21 Mar 2016 17:03:30 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: coheigea@apache.org To: commits@cxf.apache.org Date: Mon, 21 Mar 2016 17:03:32 -0000 Message-Id: <2db05fc317104ffba4d98cda4fc7776c@git.apache.org> In-Reply-To: References: X-Mailer: ASF-Git Admin Mailer Subject: [3/3] cxf-fediz git commit: Added AuthnRequest signature validation for POST Added AuthnRequest signature validation for POST Project: http://git-wip-us.apache.org/repos/asf/cxf-fediz/repo Commit: http://git-wip-us.apache.org/repos/asf/cxf-fediz/commit/d6e13e52 Tree: http://git-wip-us.apache.org/repos/asf/cxf-fediz/tree/d6e13e52 Diff: http://git-wip-us.apache.org/repos/asf/cxf-fediz/diff/d6e13e52 Branch: refs/heads/master Commit: d6e13e5210f82cef8510a8c9a06f76da72b996a0 Parents: 6f06a08 Author: Colm O hEigeartaigh Authored: Mon Mar 21 17:03:14 2016 +0000 Committer: Colm O hEigeartaigh Committed: Mon Mar 21 17:03:14 2016 +0000 ---------------------------------------------------------------------- .../idp/beans/samlsso/AuthnRequestParser.java | 19 ++- .../idp/samlsso/AuthnRequestValidator.java | 164 +++++++++++++++++++ .../WEB-INF/flows/saml-validate-request.xml | 7 +- .../apache/cxf/fediz/systests/idp/IdpTest.java | 142 ++++++++++++++++ .../src/test/resources/stsKeystoreA.properties | 6 + .../samlsso/src/test/resources/stsrealm_a.jks | Bin 0 -> 2061 bytes 6 files changed, 333 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/d6e13e52/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/samlsso/AuthnRequestParser.java ---------------------------------------------------------------------- diff --git a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/samlsso/AuthnRequestParser.java b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/samlsso/AuthnRequestParser.java index db8a013..737425f 100644 --- a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/samlsso/AuthnRequestParser.java +++ b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/samlsso/AuthnRequestParser.java @@ -24,7 +24,11 @@ import java.io.InputStreamReader; import org.w3c.dom.Document; import org.apache.cxf.common.util.Base64Utility; +import org.apache.cxf.fediz.core.exception.ProcessingException; +import org.apache.cxf.fediz.core.exception.ProcessingException.TYPE; import org.apache.cxf.fediz.service.idp.IdpConstants; +import org.apache.cxf.fediz.service.idp.domain.Idp; +import org.apache.cxf.fediz.service.idp.samlsso.AuthnRequestValidator; import org.apache.cxf.fediz.service.idp.util.WebUtils; import org.apache.cxf.rs.security.saml.DeflateEncoderDecoder; import org.apache.cxf.rs.security.saml.sso.SSOConstants; @@ -45,21 +49,32 @@ public class AuthnRequestParser { private static final Logger LOG = LoggerFactory.getLogger(AuthnRequestParser.class); - public void parseSAMLRequest(RequestContext context) { + public void parseSAMLRequest(RequestContext context, Idp idp) throws ProcessingException { String samlRequest = context.getFlowScope().getString(SSOConstants.SAML_REQUEST); LOG.debug("Received SAML Request: {}", samlRequest); + AuthnRequest parsedRequest = null; if (samlRequest == null) { WebUtils.removeAttributeFromFlowScope(context, IdpConstants.SAML_AUTHN_REQUEST); } else { try { - AuthnRequest parsedRequest = extractRequest(samlRequest); + parsedRequest = extractRequest(samlRequest); WebUtils.putAttributeInFlowScope(context, IdpConstants.SAML_AUTHN_REQUEST, parsedRequest); LOG.debug("SAML Request with id '{}' successfully parsed", parsedRequest.getID()); } catch (Exception ex) { LOG.warn("Error parsing request: {}", ex.getMessage()); } } + + if (parsedRequest != null) { + try { + AuthnRequestValidator validator = new AuthnRequestValidator(); + validator.validateAuthnRequest(context, parsedRequest, idp); + } catch (Exception ex) { + LOG.warn("Error validating request {}", ex.getMessage(), ex); + throw new ProcessingException(TYPE.BAD_REQUEST); + } + } } public String retrieveRealm(RequestContext context) { http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/d6e13e52/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/samlsso/AuthnRequestValidator.java ---------------------------------------------------------------------- diff --git a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/samlsso/AuthnRequestValidator.java b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/samlsso/AuthnRequestValidator.java new file mode 100644 index 0000000..ebb48dc --- /dev/null +++ b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/samlsso/AuthnRequestValidator.java @@ -0,0 +1,164 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cxf.fediz.service.idp.samlsso; + +import org.w3c.dom.Document; + +import org.apache.cxf.fediz.core.exception.ProcessingException; +import org.apache.cxf.fediz.core.exception.ProcessingException.TYPE; +import org.apache.cxf.fediz.core.util.CertsUtils; +import org.apache.cxf.fediz.service.idp.domain.Idp; +import org.apache.cxf.fediz.service.idp.util.WebUtils; +import org.apache.wss4j.common.crypto.Crypto; +import org.apache.wss4j.common.ext.WSSecurityException; +import org.apache.wss4j.common.saml.SAMLKeyInfo; +import org.apache.wss4j.common.saml.SAMLUtil; +import org.apache.wss4j.dom.WSDocInfo; +import org.apache.wss4j.dom.engine.WSSConfig; +import org.apache.wss4j.dom.handler.RequestData; +import org.apache.wss4j.dom.saml.WSSSAMLKeyInfoProcessor; +import org.apache.wss4j.dom.validate.Credential; +import org.apache.wss4j.dom.validate.SignatureTrustValidator; +import org.apache.wss4j.dom.validate.Validator; +import org.opensaml.saml.saml2.core.AuthnRequest; +import org.opensaml.saml.security.impl.SAMLSignatureProfileValidator; +import org.opensaml.security.credential.BasicCredential; +import org.opensaml.security.x509.BasicX509Credential; +import org.opensaml.xmlsec.signature.KeyInfo; +import org.opensaml.xmlsec.signature.Signature; +import org.opensaml.xmlsec.signature.support.SignatureException; +import org.opensaml.xmlsec.signature.support.SignatureValidator; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.webflow.execution.RequestContext; + +/** + * Validate the received AuthnRequest + */ +public class AuthnRequestValidator { + + private static final Logger LOG = LoggerFactory.getLogger(AuthnRequestValidator.class); + + public void validateAuthnRequest(RequestContext context, AuthnRequest authnRequest, Idp idp) + throws ProcessingException, WSSecurityException { + if (authnRequest.isSigned()) { + // Check destination + String destination = authnRequest.getDestination(); + LOG.debug("Validating destination: {}", destination); + + String localAddr = WebUtils.getHttpServletRequest(context).getRequestURL().toString(); + if (!localAddr.startsWith(destination)) { + LOG.debug("The destination {} does not match the local address {}", destination, localAddr); + throw new ProcessingException(TYPE.BAD_REQUEST); + } + + // Check signature + Crypto issuerCrypto = CertsUtils.getCryptoFromCertificate(idp.getCertificate()); + validateAuthnRequestSignature(authnRequest.getSignature(), issuerCrypto); + } else { + LOG.debug("No signature is present, therefore the request is rejected"); + throw new ProcessingException(TYPE.BAD_REQUEST); + } + } + + /** + * Validate the AuthnRequest signature + */ + private void validateAuthnRequestSignature( + Signature signature, + Crypto sigCrypto + ) throws WSSecurityException { + RequestData requestData = new RequestData(); + requestData.setSigVerCrypto(sigCrypto); + WSSConfig wssConfig = WSSConfig.getNewInstance(); + requestData.setWssConfig(wssConfig); + // requestData.setCallbackHandler(callbackHandler); + + SAMLKeyInfo samlKeyInfo = null; + + KeyInfo keyInfo = signature.getKeyInfo(); + if (keyInfo != null) { + try { + Document doc = signature.getDOM().getOwnerDocument(); + samlKeyInfo = + SAMLUtil.getCredentialFromKeyInfo( + keyInfo.getDOM(), new WSSSAMLKeyInfoProcessor(requestData, new WSDocInfo(doc)), sigCrypto + ); + } catch (WSSecurityException ex) { + LOG.debug("Error in getting KeyInfo from SAML AuthnRequest: {}", ex.getMessage(), ex); + throw ex; + } + } + + if (samlKeyInfo == null) { + LOG.debug("No KeyInfo supplied in the AuthnRequest signature"); + throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, "invalidSAMLsecurity"); + } + + // Validate Signature against profiles + validateSignatureAgainstProfiles(signature, samlKeyInfo); + + // Now verify trust on the signature + Credential trustCredential = new Credential(); + trustCredential.setPublicKey(samlKeyInfo.getPublicKey()); + trustCredential.setCertificates(samlKeyInfo.getCerts()); + + try { + Validator signatureValidator = new SignatureTrustValidator(); + signatureValidator.validate(trustCredential, requestData); + } catch (WSSecurityException e) { + LOG.debug("Error in validating signature on SAML AuthnRequest: {}", e.getMessage(), e); + throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, "invalidSAMLsecurity"); + } + } + + /** + * Validate a signature against the profiles + */ + private void validateSignatureAgainstProfiles( + Signature signature, + SAMLKeyInfo samlKeyInfo + ) throws WSSecurityException { + // Validate Signature against profiles + SAMLSignatureProfileValidator validator = new SAMLSignatureProfileValidator(); + try { + validator.validate(signature); + } catch (SignatureException ex) { + LOG.debug("Error in validating the SAML Signature: {}", ex.getMessage(), ex); + throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, "invalidSAMLsecurity"); + } + + BasicCredential credential = null; + if (samlKeyInfo.getCerts() != null) { + credential = new BasicX509Credential(samlKeyInfo.getCerts()[0]); + } else if (samlKeyInfo.getPublicKey() != null) { + credential = new BasicCredential(samlKeyInfo.getPublicKey()); + } else { + LOG.debug("Can't get X509Certificate or PublicKey to verify signature"); + throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, "invalidSAMLsecurity"); + } + try { + SignatureValidator.validate(signature, credential); + } catch (SignatureException ex) { + LOG.debug("Error in validating the SAML Signature: {}", ex.getMessage(), ex); + throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, "invalidSAMLsecurity"); + } + } + +} http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/d6e13e52/services/idp/src/main/webapp/WEB-INF/flows/saml-validate-request.xml ---------------------------------------------------------------------- diff --git a/services/idp/src/main/webapp/WEB-INF/flows/saml-validate-request.xml b/services/idp/src/main/webapp/WEB-INF/flows/saml-validate-request.xml index 9014030..b97d020 100644 --- a/services/idp/src/main/webapp/WEB-INF/flows/saml-validate-request.xml +++ b/services/idp/src/main/webapp/WEB-INF/flows/saml-validate-request.xml @@ -32,12 +32,13 @@ + then="parseAndValidateSAMLRequest" else="viewBadRequest" /> - - + + + http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/d6e13e52/systests/samlsso/src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java ---------------------------------------------------------------------- diff --git a/systests/samlsso/src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java b/systests/samlsso/src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java index b0f2bea..441ca2c 100644 --- a/systests/samlsso/src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java +++ b/systests/samlsso/src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java @@ -25,6 +25,8 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.net.URLEncoder; +import java.security.PrivateKey; +import java.security.cert.X509Certificate; import java.util.UUID; import org.w3c.dom.Document; @@ -48,6 +50,9 @@ import org.apache.cxf.rs.security.saml.sso.SSOConstants; import org.apache.cxf.staxutils.StaxUtils; import org.apache.http.auth.AuthScope; import org.apache.http.auth.UsernamePasswordCredentials; +import org.apache.wss4j.common.crypto.Crypto; +import org.apache.wss4j.common.crypto.CryptoFactory; +import org.apache.wss4j.common.crypto.CryptoType; import org.apache.wss4j.common.saml.OpenSAMLUtil; import org.apache.wss4j.common.util.DOM2Writer; import org.apache.wss4j.dom.engine.WSSConfig; @@ -55,7 +60,13 @@ import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; import org.opensaml.core.xml.XMLObject; +import org.opensaml.saml.common.SignableSAMLObject; import org.opensaml.saml.saml2.core.AuthnRequest; +import org.opensaml.security.x509.BasicX509Credential; +import org.opensaml.xmlsec.keyinfo.impl.X509KeyInfoGeneratorFactory; +import org.opensaml.xmlsec.signature.KeyInfo; +import org.opensaml.xmlsec.signature.Signature; +import org.opensaml.xmlsec.signature.support.SignatureConstants; /** * Some tests invoking directly on the IdP for SAML SSO @@ -164,6 +175,8 @@ public class IdpTest { new DefaultAuthnRequestBuilder().createAuthnRequest( null, "urn:org:apache:cxf:fediz:fedizhelloworld", consumerURL ); + authnRequest.setDestination("https://localhost:" + getIdpHttpsPort() + "/fediz-idp/saml"); + signAuthnRequest(authnRequest); Element authnRequestElement = OpenSAMLUtil.toDom(authnRequest, doc); String authnRequestEncoded = encodeAuthnRequest(authnRequestElement); @@ -251,6 +264,8 @@ public class IdpTest { new DefaultAuthnRequestBuilder().createAuthnRequest( null, "urn:org:apache:cxf:fediz:fedizhelloworld-xyz", "https://localhost/acsa" ); + authnRequest.setDestination("https://localhost:" + getIdpHttpsPort() + "/fediz-idp/saml"); + signAuthnRequest(authnRequest); Element authnRequestElement = OpenSAMLUtil.toDom(authnRequest, doc); String authnRequestEncoded = encodeAuthnRequest(authnRequestElement); @@ -282,6 +297,94 @@ public class IdpTest { webClient.close(); } + @org.junit.Test + public void testMissingDestination() throws Exception { + OpenSAMLUtil.initSamlEngine(); + + // Create SAML AuthnRequest + Document doc = DOMUtils.createDocument(); + doc.appendChild(doc.createElement("root")); + // Create the AuthnRequest + AuthnRequest authnRequest = + new DefaultAuthnRequestBuilder().createAuthnRequest( + null, "urn:org:apache:cxf:fediz:fedizhelloworld", "https://localhost/acsa" + ); + signAuthnRequest(authnRequest); + + Element authnRequestElement = OpenSAMLUtil.toDom(authnRequest, doc); + String authnRequestEncoded = encodeAuthnRequest(authnRequestElement); + + String urlEncodedRequest = URLEncoder.encode(authnRequestEncoded, "UTF-8"); + + String relayState = UUID.randomUUID().toString(); + String url = "https://localhost:" + getIdpHttpsPort() + "/fediz-idp/saml?"; + url += SSOConstants.RELAY_STATE + "=" + relayState; + url += "&" + SSOConstants.SAML_REQUEST + "=" + urlEncodedRequest; + + String user = "alice"; + String password = "ecila"; + + final WebClient webClient = new WebClient(); + webClient.getOptions().setUseInsecureSSL(true); + webClient.getCredentialsProvider().setCredentials( + new AuthScope("localhost", Integer.parseInt(getIdpHttpsPort())), + new UsernamePasswordCredentials(user, password)); + + webClient.getOptions().setJavaScriptEnabled(false); + try { + webClient.getPage(url); + Assert.fail("Failure expected on no destination value"); + } catch (FailingHttpStatusCodeException ex) { + Assert.assertEquals(ex.getStatusCode(), 400); + } + + webClient.close(); + } + + @org.junit.Test + public void testUnsignedRequest() throws Exception { + OpenSAMLUtil.initSamlEngine(); + + // Create SAML AuthnRequest + Document doc = DOMUtils.createDocument(); + doc.appendChild(doc.createElement("root")); + // Create the AuthnRequest + AuthnRequest authnRequest = + new DefaultAuthnRequestBuilder().createAuthnRequest( + null, "urn:org:apache:cxf:fediz:fedizhelloworld", "https://localhost/acsa" + ); + authnRequest.setDestination("https://localhost:" + getIdpHttpsPort() + "/fediz-idp/saml"); + + Element authnRequestElement = OpenSAMLUtil.toDom(authnRequest, doc); + String authnRequestEncoded = encodeAuthnRequest(authnRequestElement); + + String urlEncodedRequest = URLEncoder.encode(authnRequestEncoded, "UTF-8"); + + String relayState = UUID.randomUUID().toString(); + String url = "https://localhost:" + getIdpHttpsPort() + "/fediz-idp/saml?"; + url += SSOConstants.RELAY_STATE + "=" + relayState; + url += "&" + SSOConstants.SAML_REQUEST + "=" + urlEncodedRequest; + + String user = "alice"; + String password = "ecila"; + + final WebClient webClient = new WebClient(); + webClient.getOptions().setUseInsecureSSL(true); + webClient.getCredentialsProvider().setCredentials( + new AuthScope("localhost", Integer.parseInt(getIdpHttpsPort())), + new UsernamePasswordCredentials(user, password)); + + webClient.getOptions().setJavaScriptEnabled(false); + try { + webClient.getPage(url); + Assert.fail("Failure expected on an unsigned request"); + } catch (FailingHttpStatusCodeException ex) { + Assert.assertEquals(ex.getStatusCode(), 400); + } + + webClient.close(); + } + private String encodeAuthnRequest(Element authnRequest) throws IOException { String requestMessage = DOM2Writer.nodeToString(authnRequest); @@ -290,4 +393,43 @@ public class IdpTest { return Base64Utility.encode(deflatedBytes); } + + private void signAuthnRequest(AuthnRequest authnRequest) throws Exception { + Crypto crypto = CryptoFactory.getInstance("stsKeystoreA.properties"); + + CryptoType cryptoType = new CryptoType(CryptoType.TYPE.ALIAS); + cryptoType.setAlias("realma"); + X509Certificate[] issuerCerts = crypto.getX509Certificates(cryptoType); + + String sigAlgo = SSOConstants.RSA_SHA1; + + // Get the private key + PrivateKey privateKey = crypto.getPrivateKey("realma", "realma"); + + // Create the signature + Signature signature = OpenSAMLUtil.buildSignature(); + signature.setCanonicalizationAlgorithm(SignatureConstants.ALGO_ID_C14N_EXCL_OMIT_COMMENTS); + signature.setSignatureAlgorithm(sigAlgo); + + BasicX509Credential signingCredential = new BasicX509Credential(issuerCerts[0], privateKey); + + signature.setSigningCredential(signingCredential); + + X509KeyInfoGeneratorFactory kiFactory = new X509KeyInfoGeneratorFactory(); + kiFactory.setEmitEntityCertificate(true); + + try { + KeyInfo keyInfo = kiFactory.newInstance().generate(signingCredential); + signature.setKeyInfo(keyInfo); + } catch (org.opensaml.security.SecurityException ex) { + throw new Exception( + "Error generating KeyInfo from signing credential", ex); + } + + SignableSAMLObject signableObject = (SignableSAMLObject) authnRequest; + signableObject.setSignature(signature); + signableObject.releaseDOM(); + signableObject.releaseChildrenDOM(true); + + } } http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/d6e13e52/systests/samlsso/src/test/resources/stsKeystoreA.properties ---------------------------------------------------------------------- diff --git a/systests/samlsso/src/test/resources/stsKeystoreA.properties b/systests/samlsso/src/test/resources/stsKeystoreA.properties new file mode 100644 index 0000000..bd9fb1b --- /dev/null +++ b/systests/samlsso/src/test/resources/stsKeystoreA.properties @@ -0,0 +1,6 @@ +org.apache.ws.security.crypto.provider=org.apache.ws.security.components.crypto.Merlin +org.apache.ws.security.crypto.merlin.keystore.type=jks +org.apache.ws.security.crypto.merlin.keystore.password=storepass +org.apache.ws.security.crypto.merlin.keystore.alias=realma +org.apache.ws.security.crypto.merlin.keystore.file=stsrealm_a.jks + http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/d6e13e52/systests/samlsso/src/test/resources/stsrealm_a.jks ---------------------------------------------------------------------- diff --git a/systests/samlsso/src/test/resources/stsrealm_a.jks b/systests/samlsso/src/test/resources/stsrealm_a.jks new file mode 100644 index 0000000..fde2928 Binary files /dev/null and b/systests/samlsso/src/test/resources/stsrealm_a.jks differ