This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/ws-wss4j.git
commit a85fd33f2d41f92edabb1d629946a87f524d7fd7 Author: Colm O hEigeartaigh <cohei...@apache.org> AuthorDate: Mon Dec 16 10:15:42 2019 +0000 Fixing a few trivial bugs --- .../org/apache/wss4j/common/crypto/Merlin.java | 5 +- .../apache/wss4j/common/crypto/MerlinDevice.java | 5 +- .../wss4j/common/saml/bean/ConditionsBean.java | 10 ++-- .../org/apache/wss4j/dom/util/WSSecurityUtil.java | 2 +- .../wss4j/dom/validate/SamlAssertionValidator.java | 54 +++++++++++----------- .../stax/validate/SamlTokenValidatorImpl.java | 46 +++++++++--------- 6 files changed, 60 insertions(+), 62 deletions(-) diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Merlin.java b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Merlin.java index a2919bc..ddb2a8d 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Merlin.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Merlin.java @@ -282,10 +282,7 @@ public class Merlin extends CryptoBase { loadCacerts = loadCacerts.trim(); } if (Boolean.valueOf(loadCacerts)) { - String cacertsPath = System.getProperty("java.home") + "/lib/security/cacerts"; - if (cacertsPath != null) { - cacertsPath = cacertsPath.trim(); - } + String cacertsPath = (System.getProperty("java.home") + "/lib/security/cacerts").trim(); try (InputStream is = Files.newInputStream(Paths.get(cacertsPath))) { String cacertsPasswd = properties.getProperty(prefix + TRUSTSTORE_PASSWORD, "changeit"); if (cacertsPasswd != null) { diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/MerlinDevice.java b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/MerlinDevice.java index d719d26..02f460c 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/MerlinDevice.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/MerlinDevice.java @@ -147,10 +147,7 @@ public class MerlinDevice extends Merlin { loadCACerts = false; } } else if (Boolean.valueOf(loadCacerts)) { - String cacertsPath = System.getProperty("java.home") + "/lib/security/cacerts"; - if (cacertsPath != null) { - cacertsPath = cacertsPath.trim(); - } + String cacertsPath = (System.getProperty("java.home") + "/lib/security/cacerts").trim(); try (InputStream is = Files.newInputStream(Paths.get(cacertsPath))) { String cacertsPasswd = properties.getProperty(prefix + TRUSTSTORE_PASSWORD, "changeit"); if (cacertsPasswd != null) { diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/bean/ConditionsBean.java b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/bean/ConditionsBean.java index f97d041..b9665d2 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/bean/ConditionsBean.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/bean/ConditionsBean.java @@ -58,7 +58,7 @@ public class ConditionsBean { this.notBefore = notBefore; this.notAfter = notAfter; } - + /** * Constructor ConditionsBean creates a new ConditionsBean instance. * @@ -85,7 +85,7 @@ public class ConditionsBean { public ConditionsBean( int tokenPeriodMinutes ) { - this.tokenPeriodSeconds = tokenPeriodMinutes * 60; + this.tokenPeriodSeconds = tokenPeriodMinutes * 60L; } /** @@ -105,7 +105,7 @@ public class ConditionsBean { public void setNotBefore(DateTime notBefore) { this.notBefore = notBefore; } - + /** * Set the notBefore instance * @@ -136,7 +136,7 @@ public class ConditionsBean { public void setNotAfter(DateTime notAfter) { this.notAfter = notAfter; } - + /** * Set the notAfter instance * @@ -165,7 +165,7 @@ public class ConditionsBean { * @param tokenPeriodMinutes the tokenPeriodMinutes to set */ public void setTokenPeriodMinutes(int tokenPeriodMinutes) { - this.tokenPeriodSeconds = tokenPeriodMinutes * 60; + this.tokenPeriodSeconds = tokenPeriodMinutes * 60L; } /** diff --git a/ws-security-dom/src/main/java/org/apache/wss4j/dom/util/WSSecurityUtil.java b/ws-security-dom/src/main/java/org/apache/wss4j/dom/util/WSSecurityUtil.java index 1bee80e..28876bb 100644 --- a/ws-security-dom/src/main/java/org/apache/wss4j/dom/util/WSSecurityUtil.java +++ b/ws-security-dom/src/main/java/org/apache/wss4j/dom/util/WSSecurityUtil.java @@ -619,7 +619,7 @@ public final class WSSecurityUtil { } else { try { int parsedAction = Integer.parseInt(single[i]); - if (wssConfig.getAction(parsedAction) == null) { + if (wssConfig == null || wssConfig.getAction(parsedAction) == null) { throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, "empty", new Object[] {"Unknown action defined: " + single[i]} ); diff --git a/ws-security-dom/src/main/java/org/apache/wss4j/dom/validate/SamlAssertionValidator.java b/ws-security-dom/src/main/java/org/apache/wss4j/dom/validate/SamlAssertionValidator.java index 93142ab..60dd6ed 100644 --- a/ws-security-dom/src/main/java/org/apache/wss4j/dom/validate/SamlAssertionValidator.java +++ b/ws-security-dom/src/main/java/org/apache/wss4j/dom/validate/SamlAssertionValidator.java @@ -147,36 +147,38 @@ public class SamlAssertionValidator extends SignatureTrustValidator { boolean signed = samlAssertion.isSigned(); boolean requiredMethodFound = false; boolean standardMethodFound = false; - for (String method : methods) { - if (OpenSAMLUtil.isMethodHolderOfKey(method)) { - if (samlAssertion.getSubjectKeyInfo() == null) { - LOG.warn("There is no Subject KeyInfo to match the holder-of-key subject conf method"); - throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, "noKeyInSAMLToken"); - } + if (methods != null) { + for (String method : methods) { + if (OpenSAMLUtil.isMethodHolderOfKey(method)) { + if (samlAssertion.getSubjectKeyInfo() == null) { + LOG.warn("There is no Subject KeyInfo to match the holder-of-key subject conf method"); + throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, "noKeyInSAMLToken"); + } - // The assertion must have been signed for HOK - if (!signed) { - LOG.warn("A holder-of-key assertion must be signed"); - throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, "invalidSAMLsecurity"); + // The assertion must have been signed for HOK + if (!signed) { + LOG.warn("A holder-of-key assertion must be signed"); + throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, "invalidSAMLsecurity"); + } + standardMethodFound = true; } - standardMethodFound = true; - } - if (method != null) { - if (method.equals(requiredSubjectConfirmationMethod)) { - requiredMethodFound = true; - } - if (SAML2Constants.CONF_BEARER.equals(method) - || SAML1Constants.CONF_BEARER.equals(method)) { - standardMethodFound = true; - if (requireBearerSignature && !signed) { - LOG.warn("A Bearer Assertion was not signed"); - throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, - "invalidSAMLsecurity"); + if (method != null) { + if (method.equals(requiredSubjectConfirmationMethod)) { + requiredMethodFound = true; + } + if (SAML2Constants.CONF_BEARER.equals(method) + || SAML1Constants.CONF_BEARER.equals(method)) { + standardMethodFound = true; + if (requireBearerSignature && !signed) { + LOG.warn("A Bearer Assertion was not signed"); + throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, + "invalidSAMLsecurity"); + } + } else if (SAML2Constants.CONF_SENDER_VOUCHES.equals(method) + || SAML1Constants.CONF_SENDER_VOUCHES.equals(method)) { + standardMethodFound = true; } - } else if (SAML2Constants.CONF_SENDER_VOUCHES.equals(method) - || SAML1Constants.CONF_SENDER_VOUCHES.equals(method)) { - standardMethodFound = true; } } } diff --git a/ws-security-stax/src/main/java/org/apache/wss4j/stax/validate/SamlTokenValidatorImpl.java b/ws-security-stax/src/main/java/org/apache/wss4j/stax/validate/SamlTokenValidatorImpl.java index c3f9e73..bc3ddf1 100644 --- a/ws-security-stax/src/main/java/org/apache/wss4j/stax/validate/SamlTokenValidatorImpl.java +++ b/ws-security-stax/src/main/java/org/apache/wss4j/stax/validate/SamlTokenValidatorImpl.java @@ -169,33 +169,35 @@ public class SamlTokenValidatorImpl extends SignatureTokenValidatorImpl implemen boolean signed = samlAssertion.isSigned(); boolean requiredMethodFound = false; boolean standardMethodFound = false; - for (String method : methods) { - // The assertion must have been signed for HOK - if (OpenSAMLUtil.isMethodHolderOfKey(method)) { - if (!signed) { - LOG.warn("A holder-of-key assertion must be signed"); - throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, - "invalidSAMLsecurity"); - } - standardMethodFound = true; - } - - if (method != null) { - if (method.equals(requiredSubjectConfirmationMethod)) { - requiredMethodFound = true; - } - if (SAML2Constants.CONF_BEARER.equals(method) - || SAML1Constants.CONF_BEARER.equals(method)) { - standardMethodFound = true; - if (requireBearerSignature && !signed) { - LOG.warn("A Bearer Assertion was not signed"); + if (methods != null) { + for (String method : methods) { + // The assertion must have been signed for HOK + if (OpenSAMLUtil.isMethodHolderOfKey(method)) { + if (!signed) { + LOG.warn("A holder-of-key assertion must be signed"); throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, "invalidSAMLsecurity"); } - } else if (SAML2Constants.CONF_SENDER_VOUCHES.equals(method) - || SAML1Constants.CONF_SENDER_VOUCHES.equals(method)) { standardMethodFound = true; } + + if (method != null) { + if (method.equals(requiredSubjectConfirmationMethod)) { + requiredMethodFound = true; + } + if (SAML2Constants.CONF_BEARER.equals(method) + || SAML1Constants.CONF_BEARER.equals(method)) { + standardMethodFound = true; + if (requireBearerSignature && !signed) { + LOG.warn("A Bearer Assertion was not signed"); + throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, + "invalidSAMLsecurity"); + } + } else if (SAML2Constants.CONF_SENDER_VOUCHES.equals(method) + || SAML1Constants.CONF_SENDER_VOUCHES.equals(method)) { + standardMethodFound = true; + } + } } }