[java-metadata-aggregator] branch master updated: MDA-242 - Review for thread safety
Ian Young
ian at iay.org.uk
Tue Jul 14 17:13:34 UTC 2020
This is an automated email from the git hooks/post-receive script.
iay pushed a commit to branch master
in repository java-metadata-aggregator.
View the commit online:
http://git.shibboleth.net/view/?p=java-metadata-aggregator.git;a=commit;h=8af154b0e37d7857f4ae08dea5104c28ef0acee0
The following commit(s) were added to refs/heads/master by this push:
new 8af154b MDA-242 - Review for thread safety
8af154b is described below
commit 8af154b0e37d7857f4ae08dea5104c28ef0acee0
Author: Ian Young <ian at iay.org.uk>
AuthorDate: Tue Jul 14 18:13:30 2020 +0100
MDA-242 - Review for thread safety
https://issues.shibboleth.net/jira/browse/MDA-242
Third and final tranche: util, validate, validate.string, validate.x509.
---
.../util/PathSegmentStringTransformer.java | 2 +
.../shibboleth/metadata/util/RegexFileFilter.java | 4 +-
.../metadata/util/SHA1StringTransformer.java | 2 +
.../metadata/validate/BaseValidator.java | 7 +++-
.../string/AcceptStringRegexValidator.java | 5 ++-
.../string/AcceptStringValueValidator.java | 5 ++-
.../validate/string/BaseStringRegexValidator.java | 15 ++++---
.../validate/string/BaseStringValueValidator.java | 14 ++++---
.../string/RejectStringRegexValidator.java | 5 ++-
.../string/RejectStringValueValidator.java | 5 ++-
.../validate/x509/AbstractX509Validator.java | 2 +
.../metadata/validate/x509/X509DSADetector.java | 14 ++++---
.../metadata/validate/x509/X509ROCAValidator.java | 2 +
.../validate/x509/X509RSAExponentValidator.java | 43 ++++++++++++++-----
.../validate/x509/X509RSAKeyLengthValidator.java | 18 ++++----
.../x509/X509RSAOpenSSLBlacklistValidator.java | 33 +++++++++------
.../x509/X509RSAExponentValidatorTest.java | 49 ++++++++++++++++++++++
17 files changed, 164 insertions(+), 61 deletions(-)
diff --git a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/util/PathSegmentStringTransformer.java b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/util/PathSegmentStringTransformer.java
index f139f3a..7554ffe 100644
--- a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/util/PathSegmentStringTransformer.java
+++ b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/util/PathSegmentStringTransformer.java
@@ -20,6 +20,7 @@ package net.shibboleth.metadata.util;
import java.util.function.Function;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.Immutable;
import com.google.common.net.UrlEscapers;
@@ -32,6 +33,7 @@ import com.google.common.net.UrlEscapers;
*
* @since 0.9.2
*/
+ at Immutable
public class PathSegmentStringTransformer implements Function<String, String> {
@Override
diff --git a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/util/RegexFileFilter.java b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/util/RegexFileFilter.java
index 6af5e0c..42766f3 100644
--- a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/util/RegexFileFilter.java
+++ b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/util/RegexFileFilter.java
@@ -22,7 +22,7 @@ import java.io.FileFilter;
import java.util.regex.Pattern;
import javax.annotation.Nonnull;
-import javax.annotation.concurrent.ThreadSafe;
+import javax.annotation.concurrent.Immutable;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
import net.shibboleth.utilities.java.support.logic.Constraint;
@@ -34,7 +34,7 @@ import net.shibboleth.utilities.java.support.primitive.StringSupport;
*
* @since 0.10.0
*/
- at ThreadSafe
+ at Immutable
public class RegexFileFilter implements FileFilter {
/**
diff --git a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/util/SHA1StringTransformer.java b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/util/SHA1StringTransformer.java
index 6400f31..37befed 100644
--- a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/util/SHA1StringTransformer.java
+++ b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/util/SHA1StringTransformer.java
@@ -20,6 +20,7 @@ package net.shibboleth.metadata.util;
import java.util.function.Function;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.Immutable;
import org.cryptacular.util.CodecUtil;
import org.cryptacular.util.HashUtil;
@@ -30,6 +31,7 @@ import org.cryptacular.util.HashUtil;
*
* @since 0.9.2
*/
+ at Immutable
public class SHA1StringTransformer implements Function<String, String> {
@Override
diff --git a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/BaseValidator.java b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/BaseValidator.java
index 4f89612..87fd19f 100644
--- a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/BaseValidator.java
+++ b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/BaseValidator.java
@@ -18,6 +18,8 @@
package net.shibboleth.metadata.validate;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.GuardedBy;
+import javax.annotation.concurrent.ThreadSafe;
import net.shibboleth.metadata.ErrorStatus;
import net.shibboleth.metadata.Item;
@@ -33,6 +35,7 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
*
* @since 0.9.0
*/
+ at ThreadSafe
public abstract class BaseValidator extends BaseIdentifiableInitializableComponent {
/**
@@ -45,7 +48,7 @@ public abstract class BaseValidator extends BaseIdentifiableInitializableCompone
*
* @since 0.10.0
*/
- @Nonnull
+ @Nonnull @GuardedBy("this")
private String message = "value rejected: '%s'";
/**
@@ -56,7 +59,7 @@ public abstract class BaseValidator extends BaseIdentifiableInitializableCompone
* @since 0.10.0
*/
@Nonnull
- public String getMessage() {
+ public final synchronized String getMessage() {
return message;
}
diff --git a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/AcceptStringRegexValidator.java b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/AcceptStringRegexValidator.java
index 2dc4ead..6b9a9b9 100644
--- a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/AcceptStringRegexValidator.java
+++ b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/AcceptStringRegexValidator.java
@@ -20,6 +20,7 @@ package net.shibboleth.metadata.validate.string;
import java.util.regex.Matcher;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.ThreadSafe;
import net.shibboleth.metadata.Item;
import net.shibboleth.metadata.validate.Validator;
@@ -32,6 +33,7 @@ import net.shibboleth.metadata.validate.Validator;
*
* @since 0.10.0
*/
+ at ThreadSafe
public class AcceptStringRegexValidator extends BaseStringRegexValidator implements Validator<String> {
@Override
@@ -39,9 +41,8 @@ public class AcceptStringRegexValidator extends BaseStringRegexValidator impleme
final Matcher matcher = getPattern().matcher(e);
if (matcher.matches()) {
return Action.DONE;
- } else {
- return Action.CONTINUE;
}
+ return Action.CONTINUE;
}
}
diff --git a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/AcceptStringValueValidator.java b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/AcceptStringValueValidator.java
index c2de229..910b916 100644
--- a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/AcceptStringValueValidator.java
+++ b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/AcceptStringValueValidator.java
@@ -18,6 +18,7 @@
package net.shibboleth.metadata.validate.string;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.ThreadSafe;
import net.shibboleth.metadata.Item;
import net.shibboleth.metadata.validate.Validator;
@@ -30,15 +31,15 @@ import net.shibboleth.metadata.validate.Validator;
*
* @since 0.10.0
*/
+ at ThreadSafe
public class AcceptStringValueValidator extends BaseStringValueValidator implements Validator<String> {
@Override
public Action validate(@Nonnull final String e, @Nonnull final Item<?> item, @Nonnull final String stageId) {
if (e.equals(getValue())) {
return Action.DONE;
- } else {
- return Action.CONTINUE;
}
+ return Action.CONTINUE;
}
}
diff --git a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/BaseStringRegexValidator.java b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/BaseStringRegexValidator.java
index 49f14a7..04a1cf7 100644
--- a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/BaseStringRegexValidator.java
+++ b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/BaseStringRegexValidator.java
@@ -20,6 +20,8 @@ package net.shibboleth.metadata.validate.string;
import java.util.regex.Pattern;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.GuardedBy;
+import javax.annotation.concurrent.ThreadSafe;
import net.shibboleth.metadata.validate.BaseValidator;
import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
@@ -30,14 +32,15 @@ import net.shibboleth.utilities.java.support.component.ComponentInitializationEx
*
* @since 0.10.0
*/
+ at ThreadSafe
public abstract class BaseStringRegexValidator extends BaseValidator {
/** Regular expression to be accepted by this validator. */
- @NonnullAfterInit
+ @NonnullAfterInit @GuardedBy("this")
private String regex;
/** Compiled regular expression to use in match operations. */
- @NonnullAfterInit
+ @NonnullAfterInit @GuardedBy("this")
private Pattern pattern;
/**
@@ -46,7 +49,7 @@ public abstract class BaseStringRegexValidator extends BaseValidator {
* @return Returns the regular expression.
*/
@NonnullAfterInit
- public String getRegex() {
+ public final synchronized String getRegex() {
return regex;
}
@@ -55,7 +58,7 @@ public abstract class BaseStringRegexValidator extends BaseValidator {
*
* @param r the regular expression to set.
*/
- public void setRegex(@Nonnull final String r) {
+ public synchronized void setRegex(@Nonnull final String r) {
throwSetterPreconditionExceptions();
regex = r;
}
@@ -65,7 +68,7 @@ public abstract class BaseStringRegexValidator extends BaseValidator {
*
* @return the compiled {@link Pattern}
*/
- protected Pattern getPattern() {
+ protected final synchronized Pattern getPattern() {
return pattern;
}
@@ -77,7 +80,7 @@ public abstract class BaseStringRegexValidator extends BaseValidator {
throw new ComponentInitializationException("regular expression to be matched can not be null");
}
- pattern = Pattern.compile(regex);
+ pattern = Pattern.compile(getRegex());
}
}
diff --git a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/BaseStringValueValidator.java b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/BaseStringValueValidator.java
index d4301cb..8c6b0b2 100644
--- a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/BaseStringValueValidator.java
+++ b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/BaseStringValueValidator.java
@@ -18,6 +18,8 @@
package net.shibboleth.metadata.validate.string;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.GuardedBy;
+import javax.annotation.concurrent.ThreadSafe;
import net.shibboleth.metadata.validate.BaseValidator;
import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
@@ -28,17 +30,18 @@ import net.shibboleth.utilities.java.support.component.ComponentInitializationEx
*
* @since 0.10.0
*/
+ at ThreadSafe
public abstract class BaseStringValueValidator extends BaseValidator {
/** Value to be accepted by this validator. */
- @NonnullAfterInit private String value;
+ @NonnullAfterInit @GuardedBy("this") private String value;
/**
* Returns the value.
*
* @return Returns the value.
*/
- @NonnullAfterInit public String getValue() {
+ @NonnullAfterInit public final synchronized String getValue() {
return value;
}
@@ -47,11 +50,12 @@ public abstract class BaseStringValueValidator extends BaseValidator {
*
* @param v the value to set.
*/
- public void setValue(@Nonnull final String v) {
+ public synchronized void setValue(@Nonnull final String v) {
value = v;
}
- @Override protected void doInitialize() throws ComponentInitializationException {
+ @Override
+ protected void doInitialize() throws ComponentInitializationException {
super.doInitialize();
if (getValue() == null) {
@@ -59,4 +63,4 @@ public abstract class BaseStringValueValidator extends BaseValidator {
}
}
-}
\ No newline at end of file
+}
diff --git a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/RejectStringRegexValidator.java b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/RejectStringRegexValidator.java
index ce3d530..d246038 100644
--- a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/RejectStringRegexValidator.java
+++ b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/RejectStringRegexValidator.java
@@ -20,6 +20,7 @@ package net.shibboleth.metadata.validate.string;
import java.util.regex.Matcher;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.ThreadSafe;
import net.shibboleth.metadata.Item;
import net.shibboleth.metadata.validate.Validator;
@@ -32,6 +33,7 @@ import net.shibboleth.metadata.validate.Validator;
*
* @since 0.10.0
*/
+ at ThreadSafe
public class RejectStringRegexValidator extends BaseStringRegexValidator implements Validator<String> {
@Override
@@ -40,8 +42,7 @@ public class RejectStringRegexValidator extends BaseStringRegexValidator impleme
if (matcher.matches()) {
addErrorMessage(e, item, stageId);
return Action.DONE;
- } else {
- return Action.CONTINUE;
}
+ return Action.CONTINUE;
}
}
diff --git a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/RejectStringValueValidator.java b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/RejectStringValueValidator.java
index d9ced43..117b758 100644
--- a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/RejectStringValueValidator.java
+++ b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/string/RejectStringValueValidator.java
@@ -18,6 +18,7 @@
package net.shibboleth.metadata.validate.string;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.ThreadSafe;
import net.shibboleth.metadata.Item;
import net.shibboleth.metadata.validate.Validator;
@@ -30,6 +31,7 @@ import net.shibboleth.metadata.validate.Validator;
*
* @since 0.10.0
*/
+ at ThreadSafe
public class RejectStringValueValidator extends BaseStringValueValidator implements Validator<String> {
@Override
@@ -37,9 +39,8 @@ public class RejectStringValueValidator extends BaseStringValueValidator impleme
if (e.equals(getValue())) {
addErrorMessage(e, item, stageId);
return Action.DONE;
- } else {
- return Action.CONTINUE;
}
+ return Action.CONTINUE;
}
}
diff --git a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/AbstractX509Validator.java b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/AbstractX509Validator.java
index 0196206..638a0ea 100644
--- a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/AbstractX509Validator.java
+++ b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/AbstractX509Validator.java
@@ -20,6 +20,7 @@ package net.shibboleth.metadata.validate.x509;
import java.security.cert.X509Certificate;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.ThreadSafe;
import net.shibboleth.metadata.Item;
import net.shibboleth.metadata.pipeline.StageProcessingException;
@@ -34,6 +35,7 @@ import net.shibboleth.metadata.validate.Validator;
*
* @since 0.9.0
*/
+ at ThreadSafe
public abstract class AbstractX509Validator extends BaseValidator implements Validator<X509Certificate> {
/**
diff --git a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509DSADetector.java b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509DSADetector.java
index 9a80962..f322da5 100644
--- a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509DSADetector.java
+++ b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509DSADetector.java
@@ -21,6 +21,7 @@ import java.security.PublicKey;
import java.security.cert.X509Certificate;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.GuardedBy;
import javax.annotation.concurrent.ThreadSafe;
import net.shibboleth.metadata.Item;
@@ -54,6 +55,7 @@ public class X509DSADetector extends BaseValidator implements Validator<X509Cert
* {@link net.shibboleth.metadata.validate.Validator.Action} to return when a DSA key is detected. Default:
* {@link net.shibboleth.metadata.validate.Validator.Action#DONE}.
*/
+ @Nonnull @GuardedBy("this")
private Action action = Action.DONE;
/**
@@ -61,14 +63,14 @@ public class X509DSADetector extends BaseValidator implements Validator<X509Cert
*
* Default: <code>true</code>.
*/
- private boolean error = true;
+ @GuardedBy("this") private boolean error = true;
/**
* Returns the {@link net.shibboleth.metadata.validate.Validator.Action} to be returned if a DSA key is detected.
*
* @return the {@link net.shibboleth.metadata.validate.Validator.Action} to be returned
*/
- public Action getAction() {
+ public final synchronized Action getAction() {
return action;
}
@@ -77,7 +79,7 @@ public class X509DSADetector extends BaseValidator implements Validator<X509Cert
*
* @param newAction the {@link net.shibboleth.metadata.validate.Validator.Action} to be returned
*/
- public void setAction(@Nonnull final Action newAction) {
+ public synchronized void setAction(@Nonnull final Action newAction) {
throwSetterPreconditionExceptions();
action = newAction;
}
@@ -87,7 +89,7 @@ public class X509DSADetector extends BaseValidator implements Validator<X509Cert
*
* @param newValue whether an {@link net.shibboleth.metadata.ErrorStatus} should be added on failure
*/
- public void setError(final boolean newValue) {
+ public synchronized void setError(final boolean newValue) {
throwSetterPreconditionExceptions();
error = newValue;
}
@@ -97,7 +99,7 @@ public class X509DSADetector extends BaseValidator implements Validator<X509Cert
*
* @return <code>true</code> if an {@link net.shibboleth.metadata.ErrorStatus} is being added on failure.
*/
- public boolean isError() {
+ public final synchronized boolean isError() {
return error;
}
@@ -107,7 +109,7 @@ public class X509DSADetector extends BaseValidator implements Validator<X509Cert
final PublicKey key = cert.getPublicKey();
if ("DSA".equals(key.getAlgorithm())) {
addStatus(error, "certificate contains a DSA key", item, stageId);
- return action;
+ return getAction();
}
return Action.CONTINUE;
}
diff --git a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509ROCAValidator.java b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509ROCAValidator.java
index 4f49233..634e371 100644
--- a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509ROCAValidator.java
+++ b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509ROCAValidator.java
@@ -23,6 +23,7 @@ import java.security.cert.X509Certificate;
import java.security.interfaces.RSAPublicKey;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.Immutable;
import javax.annotation.concurrent.ThreadSafe;
import net.shibboleth.metadata.Item;
@@ -44,6 +45,7 @@ public class X509ROCAValidator extends AbstractX509Validator {
* @see <a href="https://github.com/crocs-muni/roca/blob/master/java/BrokenKey.java">original source code
* (dual MIT and Apache 2 licensed)</a>
*/
+ @Immutable
private static class BrokenKey {
// Checkstyle: JavadocVariable|JavadocMethod|ConstantName OFF
diff --git a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509RSAExponentValidator.java b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509RSAExponentValidator.java
index 7ae486c..733922b 100644
--- a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509RSAExponentValidator.java
+++ b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509RSAExponentValidator.java
@@ -23,6 +23,7 @@ import java.security.cert.X509Certificate;
import java.security.interfaces.RSAPublicKey;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.GuardedBy;
import javax.annotation.concurrent.ThreadSafe;
import net.shibboleth.metadata.Item;
@@ -44,9 +45,11 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
public class X509RSAExponentValidator extends AbstractX509Validator {
/** The RSA public exponent value below which an error should result. Default: 5. */
+ @Nonnull @GuardedBy("this")
private BigInteger errorBoundary = BigInteger.valueOf(5);
/** The RSA public exponent value below which a warning should result. Default: 0 (disabled). */
+ @Nonnull @GuardedBy("this")
private BigInteger warningBoundary = BigInteger.ZERO;
/**
@@ -54,18 +57,27 @@ public class X509RSAExponentValidator extends AbstractX509Validator {
*
* @return the RSA public exponent below which an error will result.
*/
- public long getErrorBoundary() {
- return errorBoundary.longValue();
+ public final synchronized BigInteger getErrorBoundary() {
+ return errorBoundary;
}
+ /**
+ * Set the RSA public exponent below which an error should result.
+ *
+ * @param length the RSA public exponent below which an error should result
+ */
+ public synchronized void setErrorBoundary(@Nonnull final BigInteger length) {
+ Constraint.isGreaterThanOrEqual(0, length.compareTo(BigInteger.ZERO), "boundary value must not be negative");
+ errorBoundary = length;
+ }
+
/**
* Set the RSA public exponent below which an error should result.
*
* @param length the RSA public exponent below which an error should result
*/
public void setErrorBoundary(final long length) {
- Constraint.isGreaterThanOrEqual(0, length, "boundary value must not be negative");
- errorBoundary = BigInteger.valueOf(length);
+ setErrorBoundary(BigInteger.valueOf(length));
}
/**
@@ -73,8 +85,8 @@ public class X509RSAExponentValidator extends AbstractX509Validator {
*
* @return the RSA public exponent below which a warning will result.
*/
- public long getWarningBoundary() {
- return warningBoundary.longValue();
+ public final synchronized BigInteger getWarningBoundary() {
+ return warningBoundary;
}
/**
@@ -82,9 +94,18 @@ public class X509RSAExponentValidator extends AbstractX509Validator {
*
* @param length the RSA public exponent below which a warning should result
*/
- public void setWarningBoundary(final long length) {
- Constraint.isGreaterThanOrEqual(0, length, "boundary value must not be negative");
- warningBoundary = BigInteger.valueOf(length);
+ public synchronized void setWarningBoundary(@Nonnull final BigInteger length) {
+ Constraint.isGreaterThanOrEqual(0, length.compareTo(BigInteger.ZERO), "boundary value must not be negative");
+ warningBoundary = length;
+ }
+
+ /**
+ * Set the RSA public exponent below which a warning should result.
+ *
+ * @param length the RSA public exponent below which a warning should result
+ */
+ public synchronized void setWarningBoundary(final long length) {
+ setWarningBoundary(BigInteger.valueOf(length));
}
@Override
@@ -96,10 +117,10 @@ public class X509RSAExponentValidator extends AbstractX509Validator {
final BigInteger exponent = rsaKey.getPublicExponent();
if (!exponent.testBit(0)) {
addError("RSA public exponent of " + exponent + " must be odd", item, stageId);
- } else if (exponent.compareTo(errorBoundary) < 0) {
+ } else if (exponent.compareTo(getErrorBoundary()) < 0) {
addError("RSA public exponent of " + exponent + " is less than required " + errorBoundary,
item, stageId);
- } else if (exponent.compareTo(warningBoundary) < 0) {
+ } else if (exponent.compareTo(getWarningBoundary()) < 0) {
addWarning("RSA public exponent of " + exponent + " is less than recommended " + warningBoundary,
item, stageId);
}
diff --git a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509RSAKeyLengthValidator.java b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509RSAKeyLengthValidator.java
index 6eb0880..8e76f3a 100644
--- a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509RSAKeyLengthValidator.java
+++ b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509RSAKeyLengthValidator.java
@@ -22,6 +22,7 @@ import java.security.cert.X509Certificate;
import java.security.interfaces.RSAPublicKey;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.GuardedBy;
import javax.annotation.concurrent.ThreadSafe;
import net.shibboleth.metadata.Item;
@@ -42,17 +43,17 @@ import net.shibboleth.metadata.Item;
public class X509RSAKeyLengthValidator extends AbstractX509Validator {
/** The RSA key length below which an error should result. Default: 2048. */
- private int errorBoundary = 2048;
+ @GuardedBy("this") private int errorBoundary = 2048;
/** The RSA key length below which a warning should result. Default: 0 (disabled). */
- private int warningBoundary;
+ @GuardedBy("this") private int warningBoundary;
/**
* Get the RSA key length below which an error will result.
*
* @return the RSA key length below which an error will result.
*/
- public int getErrorBoundary() {
+ public final synchronized int getErrorBoundary() {
return errorBoundary;
}
@@ -61,7 +62,7 @@ public class X509RSAKeyLengthValidator extends AbstractX509Validator {
*
* @param length the RSA key length below which an error should result
*/
- public void setErrorBoundary(final int length) {
+ public synchronized void setErrorBoundary(final int length) {
errorBoundary = length;
}
@@ -70,7 +71,7 @@ public class X509RSAKeyLengthValidator extends AbstractX509Validator {
*
* @return the RSA key length below which a warning will result.
*/
- public int getWarningBoundary() {
+ public final synchronized int getWarningBoundary() {
return warningBoundary;
}
@@ -79,11 +80,10 @@ public class X509RSAKeyLengthValidator extends AbstractX509Validator {
*
* @param length the RSA key length below which a warning should result
*/
- public void setWarningBoundary(final int length) {
+ public synchronized void setWarningBoundary(final int length) {
warningBoundary = length;
}
- /** {@inheritDoc} */
@Override
public void doValidate(@Nonnull final X509Certificate cert, @Nonnull final Item<?> item,
@Nonnull final String stageId) {
@@ -91,10 +91,10 @@ public class X509RSAKeyLengthValidator extends AbstractX509Validator {
if ("RSA".equals(key.getAlgorithm())) {
final RSAPublicKey rsaKey = (RSAPublicKey) key;
final int keyLen = rsaKey.getModulus().bitLength();
- if (keyLen < errorBoundary) {
+ if (keyLen < getErrorBoundary()) {
addError("RSA key length of " + keyLen + " bits is less than required " + errorBoundary,
item, stageId);
- } else if (keyLen < warningBoundary) {
+ } else if (keyLen < getWarningBoundary()) {
addWarning("RSA key length of " + keyLen + " bits is less than recommended " + warningBoundary,
item, stageId);
}
diff --git a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509RSAOpenSSLBlacklistValidator.java b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509RSAOpenSSLBlacklistValidator.java
index 88a29c9..4de252c 100644
--- a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509RSAOpenSSLBlacklistValidator.java
+++ b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/validate/x509/X509RSAOpenSSLBlacklistValidator.java
@@ -32,7 +32,7 @@ import java.util.HashSet;
import java.util.Set;
import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
+import javax.annotation.concurrent.GuardedBy;
import javax.annotation.concurrent.ThreadSafe;
import org.apache.commons.codec.binary.Hex;
@@ -40,6 +40,7 @@ import org.springframework.core.io.Resource;
import net.shibboleth.metadata.Item;
import net.shibboleth.metadata.pipeline.StageProcessingException;
+import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
import net.shibboleth.utilities.java.support.logic.Constraint;
@@ -54,17 +55,19 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
public class X509RSAOpenSSLBlacklistValidator extends AbstractX509Validator {
/** Sequence of bytes put on the front of the string to be hashed. */
- private final byte[] openSSLprefix = {
+ private static final byte[] OPEN_SSL_PREFIX = {
'M', 'o', 'd', 'u', 'l', 'u', 's', '=',
};
/** Resource that provides the blacklist. */
+ @NonnullAfterInit @GuardedBy("this")
private Resource blacklistResource;
/** Restrict checking to a given key size. Default: no restriction (0). */
- private int keySize;
+ @GuardedBy("this") private int keySize;
/** Set of digest values blacklisted by this validator. */
+ @Nonnull @GuardedBy("this")
private final Set<String> blacklistedValues = new HashSet<>();
/**
@@ -72,7 +75,7 @@ public class X509RSAOpenSSLBlacklistValidator extends AbstractX509Validator {
*
* @return resource that provides the blacklist
*/
- @Nullable public Resource getBlacklistResource() {
+ @NonnullAfterInit public final synchronized Resource getBlacklistResource() {
return blacklistResource;
}
@@ -91,7 +94,7 @@ public class X509RSAOpenSSLBlacklistValidator extends AbstractX509Validator {
*
* @param size restricted key size, or 0 for no restriction
*/
- public void setKeySize(final int size) {
+ public synchronized void setKeySize(final int size) {
keySize = size;
}
@@ -100,7 +103,7 @@ public class X509RSAOpenSSLBlacklistValidator extends AbstractX509Validator {
*
* @return restricted key size for this blacklist, or 0 if no restriction
*/
- public int getKeySize() {
+ public final synchronized int getKeySize() {
return keySize;
}
@@ -128,7 +131,7 @@ public class X509RSAOpenSSLBlacklistValidator extends AbstractX509Validator {
// Now construct the thing we want to hash
final ByteArrayOutputStream bb = new ByteArrayOutputStream();
try {
- bb.write(openSSLprefix);
+ bb.write(OPEN_SSL_PREFIX);
for (final char c : encodedModulus) {
bb.write((byte) c);
}
@@ -154,18 +157,26 @@ public class X509RSAOpenSSLBlacklistValidator extends AbstractX509Validator {
}
}
- /** {@inheritDoc} */
@Override
public void doValidate(@Nonnull final X509Certificate cert, @Nonnull final Item<?> item,
@Nonnull final String stageId) throws StageProcessingException {
throwComponentStateExceptions();
final PublicKey key = cert.getPublicKey();
+
if ("RSA".equals(key.getAlgorithm())) {
final RSAPublicKey rsaKey = (RSAPublicKey) key;
final BigInteger modulus = rsaKey.getModulus();
- if (keySize == 0 || keySize == modulus.bitLength()) {
+
+ final Set<String> values;
+ final int keySz;
+ synchronized (this) {
+ values = blacklistedValues;
+ keySz = keySize;
+ }
+
+ if (keySz == 0 || keySz == modulus.bitLength()) {
final String value = openSSLDigest(modulus);
- if (blacklistedValues.contains(value)) {
+ if (values.contains(value)) {
addError("RSA modulus included in key blacklist (" + value + ")",
item, stageId);
}
@@ -173,7 +184,6 @@ public class X509RSAOpenSSLBlacklistValidator extends AbstractX509Validator {
}
}
- /** {@inheritDoc} */
@Override
protected void doDestroy() {
blacklistResource = null;
@@ -182,7 +192,6 @@ public class X509RSAOpenSSLBlacklistValidator extends AbstractX509Validator {
super.doDestroy();
}
- /** {@inheritDoc} */
@Override
protected void doInitialize() throws ComponentInitializationException {
super.doInitialize();
diff --git a/aggregator-pipeline/src/test/java/net/shibboleth/metadata/validate/x509/X509RSAExponentValidatorTest.java b/aggregator-pipeline/src/test/java/net/shibboleth/metadata/validate/x509/X509RSAExponentValidatorTest.java
index ed999d4..e1131af 100644
--- a/aggregator-pipeline/src/test/java/net/shibboleth/metadata/validate/x509/X509RSAExponentValidatorTest.java
+++ b/aggregator-pipeline/src/test/java/net/shibboleth/metadata/validate/x509/X509RSAExponentValidatorTest.java
@@ -18,11 +18,13 @@
package net.shibboleth.metadata.validate.x509;
+import java.math.BigInteger;
import java.security.cert.X509Certificate;
import net.shibboleth.metadata.Item;
import net.shibboleth.metadata.MockItem;
import net.shibboleth.metadata.validate.Validator;
+import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
import org.testng.Assert;
import org.testng.annotations.Test;
@@ -93,4 +95,51 @@ public class X509RSAExponentValidatorTest extends BaseX509ValidatorTest {
// do not initialize
Assert.assertNull(val.getId(), "unset ID should be null");
}
+
+ public void testErrorBoundaryLongZero() throws Exception {
+ final var stage = new X509RSAExponentValidator();
+ stage.setErrorBoundary(0L);
+ }
+
+ @Test(expectedExceptions = ConstraintViolationException.class)
+ public void testErrorBoundaryLongNegative() throws Exception {
+ final var stage = new X509RSAExponentValidator();
+ stage.setErrorBoundary(-1L);
+ }
+
+ @Test
+ public void testWarningBoundaryLongZero() throws Exception {
+ final var stage = new X509RSAExponentValidator();
+ stage.setWarningBoundary(0L);
+ }
+
+ @Test(expectedExceptions = ConstraintViolationException.class)
+ public void testWarningBoundaryLongNegative() throws Exception {
+ final var stage = new X509RSAExponentValidator();
+ stage.setWarningBoundary(-1L);
+ }
+
+ public void testErrorBoundaryBigZero() throws Exception {
+ final var stage = new X509RSAExponentValidator();
+ stage.setErrorBoundary(BigInteger.ZERO);
+ }
+
+ @Test(expectedExceptions = ConstraintViolationException.class)
+ public void testErrorBoundaryBigNegative() throws Exception {
+ final var stage = new X509RSAExponentValidator();
+ stage.setErrorBoundary(BigInteger.valueOf(-1));
+ }
+
+ @Test
+ public void testWarningBoundaryBigZero() throws Exception {
+ final var stage = new X509RSAExponentValidator();
+ stage.setWarningBoundary(BigInteger.ZERO);
+ }
+
+ @Test(expectedExceptions = ConstraintViolationException.class)
+ public void testWarningBoundaryBigNegative() throws Exception {
+ final var stage = new X509RSAExponentValidator();
+ stage.setWarningBoundary(BigInteger.valueOf(-1));
+ }
+
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list