Author: coheigea Date: Tue Oct 18 15:20:45 2016 New Revision: 1765462 URL: http://svn.apache.org/viewvc?rev=1765462&view=rev Log: WSS-593 - Some tidy up of the PR. This closes #5.
Modified: webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CertificateStore.java webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Crypto.java webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CryptoBase.java webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Merlin.java webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/MerlinAKI.java webservices/wss4j/branches/2_1_x-fixes/ws-security-dom/src/main/java/org/apache/wss4j/dom/action/EncryptionAction.java webservices/wss4j/branches/2_1_x-fixes/ws-security-dom/src/main/java/org/apache/wss4j/dom/handler/WSHandler.java webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/impl/securityToken/SamlSecurityTokenImpl.java webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/impl/securityToken/X509SecurityTokenImpl.java webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/setup/ConfigurationConverter.java webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/setup/OutboundWSSec.java Modified: webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CertificateStore.java URL: http://svn.apache.org/viewvc/webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CertificateStore.java?rev=1765462&r1=1765461&r2=1765462&view=diff ============================================================================== --- webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CertificateStore.java (original) +++ webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CertificateStore.java Tue Oct 18 15:20:45 2016 @@ -157,7 +157,7 @@ public class CertificateStore extends Cr * @param subjectCertConstraints A set of constraints on the Subject DN of the certificates * @throws WSSecurityException if the certificate chain is invalid */ - public void verifyTrust( + protected void verifyTrust( X509Certificate[] certs, boolean enableRevocation, Collection<Pattern> subjectCertConstraints @@ -276,7 +276,7 @@ public class CertificateStore extends Cr @Override public void verifyTrust(X509Certificate[] certs, boolean enableRevocation, Collection<Pattern> subjectCertConstraints, Collection<Pattern> issuerCertConstraints) throws WSSecurityException { - verifyTrust(certs,enableRevocation,subjectCertConstraints); + verifyTrust(certs, enableRevocation, subjectCertConstraints); if (!matchesIssuerDnPattern(certs[0], issuerCertConstraints)) { throw new WSSecurityException(WSSecurityException.ErrorCode.FAILED_AUTHENTICATION); } Modified: webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Crypto.java URL: http://svn.apache.org/viewvc/webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Crypto.java?rev=1765462&r1=1765461&r2=1765462&view=diff ============================================================================== --- webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Crypto.java (original) +++ webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Crypto.java Tue Oct 18 15:20:45 2016 @@ -197,36 +197,14 @@ public interface Crypto { * @param certs Certificate chain to validate * @param enableRevocation whether to enable CRL verification or not * @param subjectCertConstraints A set of constraints on the Subject DN of the certificates - * @throws WSSecurityException if the certificate chain is invalid - */ - void verifyTrust( - X509Certificate[] certs, boolean enableRevocation, - Collection<Pattern> subjectCertConstraints - ) throws WSSecurityException; - - /** - * Evaluate whether a given certificate chain should be trusted. - * - * @param certs Certificate chain to validate - * @param enableRevocation whether to enable CRL verification or not - * @param subjectCertConstraints A set of constraints on the Subject DN of the certificates * @param issuerCertConstraints A set of constraints on the Issuer DN of the certificates * @throws WSSecurityException if the certificate chain is invalid */ void verifyTrust( - X509Certificate[] certs, boolean enableRevocation, - Collection<Pattern> subjectCertConstraints,Collection<Pattern> issuerCertConstraints + X509Certificate[] certs, boolean enableRevocation, + Collection<Pattern> subjectCertConstraints,Collection<Pattern> issuerCertConstraints ) throws WSSecurityException; - - /** - * Evaluate whether a given public key should be trusted directly (located - * - * @param certs Certificate chain to validate - * @throws WSSecurityException if the certificate chain is invalid - */ - void verifyDirectTrust(X509Certificate[] certs) throws WSSecurityException; - /** * Evaluate whether a given public key should be trusted. * Modified: webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CryptoBase.java URL: http://svn.apache.org/viewvc/webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CryptoBase.java?rev=1765462&r1=1765461&r2=1765462&view=diff ============================================================================== --- webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CryptoBase.java (original) +++ webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CryptoBase.java Tue Oct 18 15:20:45 2016 @@ -296,11 +296,6 @@ public abstract class CryptoBase impleme return certs; } - @Override - public void verifyDirectTrust(X509Certificate[] certs) throws WSSecurityException { - verifyTrust(certs, true, null); - } - protected Object createBCX509Name(String s) { if (BC_509CLASS_CONS != null) { try { @@ -321,13 +316,16 @@ public abstract class CryptoBase impleme matchesSubjectDnPattern( final X509Certificate cert, final Collection<Pattern> subjectDNPatterns ) { - if (cert == null) { LOG.debug("The certificate is null so no constraints matching was possible"); return false; } String subjectName = cert.getSubjectX500Principal().getName(); - return matchesName(subjectName,subjectDNPatterns); + if (subjectDNPatterns == null || subjectDNPatterns.isEmpty()) { + LOG.warn("No Subject DN Certificate Constraints were defined. This could be a security issue"); + return true; + } + return matchesName(subjectName, subjectDNPatterns); } /** @@ -337,31 +335,26 @@ public abstract class CryptoBase impleme */ protected boolean matchesIssuerDnPattern( - final X509Certificate cert, final Collection<Pattern> issuerDNPatterns + final X509Certificate cert, final Collection<Pattern> issuerDNPatterns ) { - - if (cert == null) { - LOG.debug("The certificate is null so no constraints matching was possible"); - return false; - } + if (cert == null) { + LOG.debug("The certificate is null so no constraints matching was possible"); + return false; + } String issuerDn = cert.getIssuerDN().getName(); - return matchesName(issuerDn,issuerDNPatterns); + return matchesName(issuerDn, issuerDNPatterns); } /** * @return true if the provided name matches the constraints defined in the - * subject DNConstraints; false, otherwise. The certificate subject DN only - * has to match ONE of the subject cert constraints (not all). + * subject DNConstraints; false, otherwise. The certificate (subject) DN only + * has to match ONE of the (subject) cert constraints (not all). */ protected boolean matchesName( - final String name, final Collection<Pattern> patterns + final String name, final Collection<Pattern> patterns ) { - if (patterns == null || patterns.isEmpty()) { - LOG.warn("No Subject DN Certificate Constraints were defined. This could be a security issue"); - return true; - } - if (!patterns.isEmpty()) { + if (patterns != null && !patterns.isEmpty()) { if (name == null || name.isEmpty()) { LOG.debug("The name is null so no constraints matching was possible"); return false; Modified: webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Merlin.java URL: http://svn.apache.org/viewvc/webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Merlin.java?rev=1765462&r1=1765461&r2=1765462&view=diff ============================================================================== --- webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Merlin.java (original) +++ webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Merlin.java Tue Oct 18 15:20:45 2016 @@ -779,7 +779,7 @@ public class Merlin extends CryptoBase { * * @throws WSSecurityException if the certificate chain is invalid */ - public void verifyTrust( + protected void verifyTrust( X509Certificate[] certs, boolean enableRevocation, Collection<Pattern> subjectCertConstraints @@ -953,9 +953,10 @@ public class Merlin extends CryptoBase { } @Override - public void verifyTrust(X509Certificate[] certs, boolean enableRevocation, Collection<Pattern> subjectCertConstraints, + public void verifyTrust(X509Certificate[] certs, boolean enableRevocation, + Collection<Pattern> subjectCertConstraints, Collection<Pattern> issuerCertConstraints) throws WSSecurityException { - verifyTrust(certs,enableRevocation,subjectCertConstraints); + verifyTrust(certs, enableRevocation, subjectCertConstraints); if (!matchesIssuerDnPattern(certs[0], issuerCertConstraints)) { throw new WSSecurityException(WSSecurityException.ErrorCode.FAILED_AUTHENTICATION); } Modified: webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/MerlinAKI.java URL: http://svn.apache.org/viewvc/webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/MerlinAKI.java?rev=1765462&r1=1765461&r2=1765462&view=diff ============================================================================== --- webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/MerlinAKI.java (original) +++ webservices/wss4j/branches/2_1_x-fixes/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/MerlinAKI.java Tue Oct 18 15:20:45 2016 @@ -81,7 +81,7 @@ public class MerlinAKI extends Merlin { * @throws WSSecurityException if the certificate chain is invalid */ @Override - public void verifyTrust( + protected void verifyTrust( X509Certificate[] certs, boolean enableRevocation, Collection<Pattern> subjectCertConstraints Modified: webservices/wss4j/branches/2_1_x-fixes/ws-security-dom/src/main/java/org/apache/wss4j/dom/action/EncryptionAction.java URL: http://svn.apache.org/viewvc/webservices/wss4j/branches/2_1_x-fixes/ws-security-dom/src/main/java/org/apache/wss4j/dom/action/EncryptionAction.java?rev=1765462&r1=1765461&r2=1765462&view=diff ============================================================================== --- webservices/wss4j/branches/2_1_x-fixes/ws-security-dom/src/main/java/org/apache/wss4j/dom/action/EncryptionAction.java (original) +++ webservices/wss4j/branches/2_1_x-fixes/ws-security-dom/src/main/java/org/apache/wss4j/dom/action/EncryptionAction.java Tue Oct 18 15:20:45 2016 @@ -80,7 +80,7 @@ public class EncryptionAction implements cryptoType.setAlias(encryptionToken.getUser()); X509Certificate[] certs = crypto.getX509Certificates(cryptoType); if (certs != null && certs.length > 0) { - crypto.verifyTrust(certs, enableRevocation, null); + crypto.verifyTrust(certs, enableRevocation, null, null); } } if (encryptionToken.getParts().size() > 0) { Modified: webservices/wss4j/branches/2_1_x-fixes/ws-security-dom/src/main/java/org/apache/wss4j/dom/handler/WSHandler.java URL: http://svn.apache.org/viewvc/webservices/wss4j/branches/2_1_x-fixes/ws-security-dom/src/main/java/org/apache/wss4j/dom/handler/WSHandler.java?rev=1765462&r1=1765461&r2=1765462&view=diff ============================================================================== --- webservices/wss4j/branches/2_1_x-fixes/ws-security-dom/src/main/java/org/apache/wss4j/dom/handler/WSHandler.java (original) +++ webservices/wss4j/branches/2_1_x-fixes/ws-security-dom/src/main/java/org/apache/wss4j/dom/handler/WSHandler.java Tue Oct 18 15:20:45 2016 @@ -1318,39 +1318,15 @@ public abstract class WSHandler { String certConstraints = getString(WSHandlerConstants.SIG_SUBJECT_CERT_CONSTRAINTS, reqData.getMsgContext()); if (certConstraints != null) { - String[] certConstraintsList = certConstraints.split(","); - if (certConstraintsList != null) { - Collection<Pattern> subjectCertConstraints = - new ArrayList<>(certConstraintsList.length); - for (String certConstraint : certConstraintsList) { - try { - subjectCertConstraints.add(Pattern.compile(certConstraint.trim())); - } catch (PatternSyntaxException ex) { - LOG.debug(ex.getMessage(), ex); - throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, ex); - } - } - reqData.setSubjectCertConstraints(subjectCertConstraints); - } + Collection<Pattern> subjectCertConstraints = getCertConstraints(certConstraints); + reqData.setSubjectCertConstraints(subjectCertConstraints); } String issuerCertConstraintsStringValue = - getString(WSHandlerConstants.SIG_ISSUER_CERT_CONSTRAINTS, reqData.getMsgContext()); - if (issuerCertConstraintsStringValue != null) { - String[] issuerCertConstraintsList = issuerCertConstraintsStringValue.split(","); - if (issuerCertConstraintsList != null) { - Collection<Pattern> issuerCertConstraints = - new ArrayList<>(issuerCertConstraintsList.length); - for (String certConstraint : issuerCertConstraintsList) { - try { - issuerCertConstraints.add(Pattern.compile(certConstraint.trim())); - } catch (PatternSyntaxException ex) { - LOG.debug(ex.getMessage(), ex); - throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, ex); - } - } - reqData.setIssuerDNPatterns(issuerCertConstraints); - } - } + getString(WSHandlerConstants.SIG_ISSUER_CERT_CONSTRAINTS, reqData.getMsgContext()); + if (issuerCertConstraintsStringValue != null) { + Collection<Pattern> issuerCertConstraints = getCertConstraints(issuerCertConstraintsStringValue); + reqData.setIssuerDNPatterns(issuerCertConstraints); + } boolean expandXOP = decodeBooleanConfigValue( @@ -1358,6 +1334,25 @@ public abstract class WSHandler { ); reqData.setExpandXopIncludeForSignature(expandXOP); } + + private Collection<Pattern> getCertConstraints(String certConstraints) throws WSSecurityException { + String[] certConstraintsList = certConstraints.split(","); + if (certConstraintsList != null && certConstraintsList.length > 0) { + Collection<Pattern> certConstraintsCollection = + new ArrayList<>(certConstraintsList.length); + for (String certConstraint : certConstraintsList) { + try { + certConstraintsCollection.add(Pattern.compile(certConstraint.trim())); + } catch (PatternSyntaxException ex) { + LOG.debug(ex.getMessage(), ex); + throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, ex); + } + } + + return certConstraintsCollection; + } + return Collections.emptyList(); + } /* * Set and check the decryption specific parameters, if necessary Modified: webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/impl/securityToken/SamlSecurityTokenImpl.java URL: http://svn.apache.org/viewvc/webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/impl/securityToken/SamlSecurityTokenImpl.java?rev=1765462&r1=1765461&r2=1765462&view=diff ============================================================================== --- webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/impl/securityToken/SamlSecurityTokenImpl.java (original) +++ webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/impl/securityToken/SamlSecurityTokenImpl.java Tue Oct 18 15:20:45 2016 @@ -212,7 +212,7 @@ public class SamlSecurityTokenImpl exten issuerCertConstraints = securityProperties.getIssuerDNConstraints(); } - crypto.verifyTrust(x509Certificates, enableRevocation, subjectCertConstraints,issuerCertConstraints); + crypto.verifyTrust(x509Certificates, enableRevocation, subjectCertConstraints, issuerCertConstraints); } PublicKey publicKey = getPublicKey(); if (publicKey != null) { Modified: webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/impl/securityToken/X509SecurityTokenImpl.java URL: http://svn.apache.org/viewvc/webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/impl/securityToken/X509SecurityTokenImpl.java?rev=1765462&r1=1765461&r2=1765462&view=diff ============================================================================== --- webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/impl/securityToken/X509SecurityTokenImpl.java (original) +++ webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/impl/securityToken/X509SecurityTokenImpl.java Tue Oct 18 15:20:45 2016 @@ -119,7 +119,7 @@ public abstract class X509SecurityTokenI subjectCertConstraints = securityProperties.getSubjectCertConstraints(); issuerCertConstraints = securityProperties.getIssuerDNConstraints(); } - getCrypto().verifyTrust(x509Certificates, enableRevocation, subjectCertConstraints,issuerCertConstraints); + getCrypto().verifyTrust(x509Certificates, enableRevocation, subjectCertConstraints, issuerCertConstraints); } } Modified: webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/setup/ConfigurationConverter.java URL: http://svn.apache.org/viewvc/webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/setup/ConfigurationConverter.java?rev=1765462&r1=1765461&r2=1765462&view=diff ============================================================================== --- webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/setup/ConfigurationConverter.java (original) +++ webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/setup/ConfigurationConverter.java Tue Oct 18 15:20:45 2016 @@ -20,6 +20,7 @@ package org.apache.wss4j.stax.setup; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Properties; @@ -535,37 +536,15 @@ public final class ConfigurationConverte String certConstraints = getString(ConfigurationConstants.SIG_SUBJECT_CERT_CONSTRAINTS, config); if (certConstraints != null) { - String[] certConstraintsList = certConstraints.split(","); - if (certConstraintsList != null) { - Collection<Pattern> subjectCertConstraints = - new ArrayList<>(certConstraintsList.length); - for (String certConstraint : certConstraintsList) { - try { - subjectCertConstraints.add(Pattern.compile(certConstraint.trim())); - } catch (PatternSyntaxException ex) { - LOG.error(ex.getMessage(), ex); - } - } - properties.setSubjectCertConstraints(subjectCertConstraints); - } + Collection<Pattern> subjectCertConstraints = getCertConstraints(certConstraints); + properties.setSubjectCertConstraints(subjectCertConstraints); } // Subject Cert Constraints String issuerCertConstraintsString = - getString(ConfigurationConstants.SIG_ISSUER_CERT_CONSTRAINTS, config); + getString(ConfigurationConstants.SIG_ISSUER_CERT_CONSTRAINTS, config); if (issuerCertConstraintsString != null) { - String[] certConstraintsList = issuerCertConstraintsString.split(","); - if (certConstraintsList != null) { - Collection<Pattern> issuerCertConstraints = - new ArrayList<>(certConstraintsList.length); - for (String certConstraint : certConstraintsList) { - try { - issuerCertConstraints.add(Pattern.compile(certConstraint.trim())); - } catch (PatternSyntaxException ex) { - LOG.error(ex.getMessage(), ex); - } - } - properties.setSubjectCertConstraints(issuerCertConstraints); - } + Collection<Pattern> issuerCertConstraints = getCertConstraints(certConstraints); + properties.setIssuerDNConstraints(issuerCertConstraints); } properties.setUtTTL(decodeTimeToLive(config, false)); @@ -627,6 +606,24 @@ public final class ConfigurationConverte } } + private static Collection<Pattern> getCertConstraints(String certConstraints) { + String[] certConstraintsList = certConstraints.split(","); + if (certConstraintsList != null && certConstraintsList.length > 0) { + Collection<Pattern> certConstraintsCollection = + new ArrayList<>(certConstraintsList.length); + for (String certConstraint : certConstraintsList) { + try { + certConstraintsCollection.add(Pattern.compile(certConstraint.trim())); + } catch (PatternSyntaxException ex) { + LOG.error(ex.getMessage(), ex); + } + } + + return certConstraintsCollection; + } + return Collections.emptyList(); + } + private static void configureParts(Object secureParts, WSSSecurityProperties properties, String digestAlgo, boolean required, boolean signature) { if (secureParts != null) { Modified: webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/setup/OutboundWSSec.java URL: http://svn.apache.org/viewvc/webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/setup/OutboundWSSec.java?rev=1765462&r1=1765461&r2=1765462&view=diff ============================================================================== --- webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/setup/OutboundWSSec.java (original) +++ webservices/wss4j/branches/2_1_x-fixes/ws-security-stax/src/main/java/org/apache/wss4j/stax/setup/OutboundWSSec.java Tue Oct 18 15:20:45 2016 @@ -405,7 +405,7 @@ public class OutboundWSSec { // Check for Revocation if (securityProperties.isEnableRevocation() && x509Certificates != null) { Crypto crypto = securityProperties.getEncryptionCrypto(); - crypto.verifyTrust(x509Certificates, true, null); + crypto.verifyTrust(x509Certificates, true, null, null); } // Create a new outbound EncryptedKey token for the cert