[java-shib-shared] branch main updated: Fix null and annotation bugs, add record-based KeyStrategy API.
Scott Cantor
cantor.2 at osu.edu
Mon Nov 7 17:36:12 UTC 2022
This is an automated email from the git hooks/post-receive script.
scantor pushed a commit to branch main
in repository java-shib-shared.
View the commit online:
http://git.shibboleth.net/view/?p=java-shib-shared.git;a=commit;h=e808c309d35c42d013c25b41388d3cd5112babac
The following commit(s) were added to refs/heads/main by this push:
new e808c309 Fix null and annotation bugs, add record-based KeyStrategy API.
e808c309 is described below
commit e808c309d35c42d013c25b41388d3cd5112babac
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Nov 7 12:36:09 2022 -0500
Fix null and annotation bugs, add record-based KeyStrategy API.
---
.../net/shibboleth/shared/security/DataSealer.java | 23 ++++++++-------
.../shared/security/DataSealerKeyStrategy.java | 34 ++++++++++++++++++++++
.../security/impl/BasicKeystoreKeyStrategy.java | 13 ++++++---
.../impl/BasicKeystoreKeyStrategyTool.java | 13 +++++----
.../shared/security/impl/ScriptedKeyStrategy.java | 18 +++++++++---
.../impl/SelfSignedCertificateGenerator.java | 33 ++++++++++++---------
.../shibboleth/shared/security/DataSealerTest.java | 1 +
.../shared/security/TestResourceConverter.java | 14 ++++-----
8 files changed, 106 insertions(+), 43 deletions(-)
diff --git a/shib-security/src/main/java/net/shibboleth/shared/security/DataSealer.java b/shib-security/src/main/java/net/shibboleth/shared/security/DataSealer.java
index 8daed103..824d4700 100644
--- a/shib-security/src/main/java/net/shibboleth/shared/security/DataSealer.java
+++ b/shib-security/src/main/java/net/shibboleth/shared/security/DataSealer.java
@@ -52,6 +52,7 @@ import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.logic.Constraint;
import net.shibboleth.shared.logic.ConstraintViolationException;
import net.shibboleth.shared.primitive.StringSupport;
+import net.shibboleth.shared.security.DataSealerKeyStrategy.NamedKey;
/**
@@ -171,15 +172,17 @@ public class DataSealer extends AbstractInitializableComponent {
public void setNodePrefix(@Nullable @NotEmpty final String prefix) {
checkSetterPreconditions();
- nodePrefix = StringSupport.trimOrNull(prefix);
- if (nodePrefix != null) {
- if (nodePrefix.length() > PREFIX_LEN) {
+ String s = StringSupport.trimOrNull(prefix);
+ if (s != null) {
+ if (s.length() > PREFIX_LEN) {
throw new ConstraintViolationException(
"DataSealer nodePrefix cannot be longer than " + Integer.toString(PREFIX_LEN) + " characters");
- } else if (nodePrefix.length() < PREFIX_LEN) {
- nodePrefix = nodePrefix.concat(new String("X").repeat(PREFIX_LEN - nodePrefix.length()));
+ } else if (s.length() < PREFIX_LEN) {
+ s = s.concat(new String("X").repeat(PREFIX_LEN - s.length()));
}
}
+
+ nodePrefix = s;
}
/** {@inheritDoc} */
@@ -198,7 +201,7 @@ public class DataSealer extends AbstractInitializableComponent {
if (!lockedAtStartup) {
// Before we finish initialization, make sure that things are working.
- testEncryption(keyStrategy.getDefaultKey().getSecond());
+ testEncryption(keyStrategy.getDefaultKeyRecord().key());
}
if (nodePrefix != null) {
@@ -391,10 +394,10 @@ public class DataSealer extends AbstractInitializableComponent {
random.nextBytes(iv);
final GCMParameterSpec params = new GCMParameterSpec(128, iv);
- final Pair<String,SecretKey> defaultKey = keyStrategy.getDefaultKey();
+ final NamedKey defaultKey = keyStrategy.getDefaultKeyRecord();
- cipher.init(Cipher.ENCRYPT_MODE, defaultKey.getSecond(), params);
- cipher.updateAAD(defaultKey.getFirst().getBytes());
+ cipher.init(Cipher.ENCRYPT_MODE, defaultKey.key(), params);
+ cipher.updateAAD(defaultKey.name().getBytes());
try (final ByteArrayOutputStream byteStream = new ByteArrayOutputStream();
final GZIPOutputStream compressedStream = new GZIPOutputStream(byteStream);
@@ -425,7 +428,7 @@ public class DataSealer extends AbstractInitializableComponent {
try (final ByteArrayOutputStream finalByteStream = new ByteArrayOutputStream();
final DataOutputStream finalDataStream = new DataOutputStream(finalByteStream)) {
- finalDataStream.writeUTF(defaultKey.getFirst());
+ finalDataStream.writeUTF(defaultKey.name());
finalDataStream.write(iv);
finalDataStream.write(encryptedData, 0, outputLen);
finalDataStream.flush();
diff --git a/shib-security/src/main/java/net/shibboleth/shared/security/DataSealerKeyStrategy.java b/shib-security/src/main/java/net/shibboleth/shared/security/DataSealerKeyStrategy.java
index 4329498e..245d845d 100644
--- a/shib-security/src/main/java/net/shibboleth/shared/security/DataSealerKeyStrategy.java
+++ b/shib-security/src/main/java/net/shibboleth/shared/security/DataSealerKeyStrategy.java
@@ -24,6 +24,7 @@ import javax.crypto.SecretKey;
import net.shibboleth.shared.annotation.constraint.NotEmpty;
import net.shibboleth.shared.collection.Pair;
+import net.shibboleth.shared.logic.Constraint;
/**
* Interface for acquiring secret keys on behalf of the {@link DataSealer} class,
@@ -41,9 +42,24 @@ public interface DataSealerKeyStrategy {
*
* @return the key
* @throws KeyException if the key cannot be returned
+ *
+ * @deprecated
*/
+ @Deprecated(since="9.0.0", forRemoval=true)
@Nonnull Pair<String,SecretKey> getDefaultKey() throws KeyException;
+ /**
+ * Get an immutable record of the default named key.
+ *
+ * @return default key record
+ *
+ * @throws KeyException if the key is unobtainable
+ */
+ @Nonnull default NamedKey getDefaultKeyRecord() throws KeyException {
+ final Pair<String,SecretKey> result = getDefaultKey();
+ return new NamedKey(result.getFirst(), result.getSecond());
+ }
+
/**
* Get a specifically named key.
*
@@ -53,5 +69,23 @@ public interface DataSealerKeyStrategy {
* @throws KeyException if the key cannot be returned, does not exist, etc.
*/
@Nonnull SecretKey getKey(@Nonnull @NotEmpty final String name) throws KeyException;
+
+ /**
+ * Encapsulates a named key managed by a strategy.
+ *
+ * @param name key name
+ * @param key key value
+ *
+ * @since 9.0.0
+ */
+ record NamedKey(@Nonnull @NotEmpty String name, @Nonnull SecretKey key) {
+
+ /** Constructor. */
+ public NamedKey {
+ Constraint.isTrue(name != null && name.length() > 0, "Name cannot be empty or null");
+ Constraint.isNotNull(key, "Key cannot be null");
+ }
+
+ }
}
\ No newline at end of file
diff --git a/shib-security/src/main/java/net/shibboleth/shared/security/impl/BasicKeystoreKeyStrategy.java b/shib-security/src/main/java/net/shibboleth/shared/security/impl/BasicKeystoreKeyStrategy.java
index cec6f4cd..ff276d3b 100644
--- a/shib-security/src/main/java/net/shibboleth/shared/security/impl/BasicKeystoreKeyStrategy.java
+++ b/shib-security/src/main/java/net/shibboleth/shared/security/impl/BasicKeystoreKeyStrategy.java
@@ -288,20 +288,25 @@ public class BasicKeystoreKeyStrategy extends AbstractInitializableComponent imp
}
/** {@inheritDoc} */
- @Override
@Nonnull public Pair<String,SecretKey> getDefaultKey() throws KeyException {
+
+ final NamedKey keyrec = getDefaultKeyRecord();
+ return new Pair<>(keyrec.name(), keyrec.key());
+ }
+
+ /** {@inheritDoc} */
+ @Nonnull public NamedKey getDefaultKeyRecord() throws KeyException {
checkComponentActive();
synchronized(this) {
if (defaultKey != null) {
- return new Pair<>(currentAlias, defaultKey);
+ return new NamedKey(currentAlias, defaultKey);
}
throw new KeyException("Passwords not supplied, keystore is locked");
}
}
-
+
/** {@inheritDoc} */
- @Override
@Nonnull public SecretKey getKey(@Nonnull @NotEmpty final String name) throws KeyException {
synchronized(this) {
if (defaultKey != null && name.equals(currentAlias)) {
diff --git a/shib-security/src/main/java/net/shibboleth/shared/security/impl/BasicKeystoreKeyStrategyTool.java b/shib-security/src/main/java/net/shibboleth/shared/security/impl/BasicKeystoreKeyStrategyTool.java
index 37b11959..208bc7a9 100644
--- a/shib-security/src/main/java/net/shibboleth/shared/security/impl/BasicKeystoreKeyStrategyTool.java
+++ b/shib-security/src/main/java/net/shibboleth/shared/security/impl/BasicKeystoreKeyStrategyTool.java
@@ -146,13 +146,15 @@ public class BasicKeystoreKeyStrategyTool {
// Load keystore or create empty instance.
final KeyStore ks = KeyStore.getInstance(args.keystoreType);
- try (final FileInputStream ksIn = args.keystoreFile.exists() ? new FileInputStream(args.keystoreFile) : null) {
- ks.load(ksIn, args.keystorePassword.toCharArray());
+ try (final FileInputStream ksIn =
+ args.keystoreFile != null && args.keystoreFile.exists() ?
+ new FileInputStream(args.keystoreFile) : null) {
+ ks.load(ksIn, args.keystorePassword != null ? args.keystorePassword.toCharArray() : null);
}
// Load key versioning properties.
final Properties versionInfo = new Properties();
- if (args.versionFile.exists()) {
+ if (args.versionFile != null && args.versionFile.exists()) {
try (final FileInputStream versionIn = new FileInputStream(args.versionFile)) {
versionInfo.load(versionIn);
}
@@ -179,7 +181,8 @@ public class BasicKeystoreKeyStrategyTool {
final KeyGenerator keyGenerator = KeyGenerator.getInstance(args.keyType);
keyGenerator.init(args.keySize);
final SecretKey newKey = keyGenerator.generateKey();
- ks.setKeyEntry(newKeyAlias, newKey, args.keystorePassword.toCharArray(), null);
+ ks.setKeyEntry(newKeyAlias, newKey,
+ args.keystorePassword != null ? args.keystorePassword.toCharArray() : null, null);
// Remove older keys maintaining the key count.
int oldVersion = currentVersion - args.keyCount;
@@ -196,7 +199,7 @@ public class BasicKeystoreKeyStrategyTool {
// Save keystore back, and then the properties.
try (final FileOutputStream ksOut = new FileOutputStream(args.keystoreFile)) {
- ks.store(ksOut, args.keystorePassword.toCharArray());
+ ks.store(ksOut, args.keystorePassword != null ? args.keystorePassword.toCharArray() : null);
}
try (final FileOutputStream versionOut = new FileOutputStream(args.versionFile)) {
diff --git a/shib-security/src/main/java/net/shibboleth/shared/security/impl/ScriptedKeyStrategy.java b/shib-security/src/main/java/net/shibboleth/shared/security/impl/ScriptedKeyStrategy.java
index c7bf8644..6ba0daeb 100644
--- a/shib-security/src/main/java/net/shibboleth/shared/security/impl/ScriptedKeyStrategy.java
+++ b/shib-security/src/main/java/net/shibboleth/shared/security/impl/ScriptedKeyStrategy.java
@@ -191,6 +191,7 @@ public class ScriptedKeyStrategy extends AbstractInitializableComponent implemen
} else {
internalTaskTimer = updateTaskTimer;
}
+ assert(internalTaskTimer != null);
internalTaskTimer.schedule(updateTask, updateInterval.toMillis(), updateInterval.toMillis());
}
}
@@ -201,7 +202,7 @@ public class ScriptedKeyStrategy extends AbstractInitializableComponent implemen
if (updateTask != null) {
updateTask.cancel();
updateTask = null;
- if (updateTaskTimer == null) {
+ if (updateTaskTimer == null && internalTaskTimer != null) {
internalTaskTimer.cancel();
}
internalTaskTimer = null;
@@ -209,13 +210,20 @@ public class ScriptedKeyStrategy extends AbstractInitializableComponent implemen
super.doDestroy();
}
+
/** {@inheritDoc} */
@Nonnull public Pair<String,SecretKey> getDefaultKey() throws KeyException {
+ final NamedKey keyrec = getDefaultKeyRecord();
+ return new Pair<>(keyrec.name(), keyrec.key());
+ }
+
+ /** {@inheritDoc} */
+ @Nonnull public NamedKey getDefaultKeyRecord() throws KeyException {
checkComponentActive();
synchronized(this) {
if (defaultKey != null) {
- return new Pair<>(currentAlias, defaultKey);
+ return new NamedKey(currentAlias, defaultKey);
}
throw new KeyException("Default key unavailable");
}
@@ -245,11 +253,13 @@ public class ScriptedKeyStrategy extends AbstractInitializableComponent implemen
log.debug("Loaded key '{}' from external script", name);
return (SecretKey) result;
} else if (result instanceof Pair && ((Pair<?,?>) result).getSecond() instanceof SecretKey) {
+ final SecretKey key = (SecretKey) ((Pair<?,?>) result).getSecond();
+ assert(key != null);
synchronized(this) {
- keyCache.put(name, (SecretKey) ((Pair<?,?>) result).getSecond());
+ keyCache.put(name, key);
}
log.debug("Loaded key '{}' from external script", name);
- return (SecretKey) ((Pair<?,?>) result).getSecond();
+ return key;
} else {
throw new KeyException("Script did not return SecretKey or Pair<String,SecretKey> result.");
}
diff --git a/shib-security/src/main/java/net/shibboleth/shared/security/impl/SelfSignedCertificateGenerator.java b/shib-security/src/main/java/net/shibboleth/shared/security/impl/SelfSignedCertificateGenerator.java
index 830f7611..bc43bff1 100644
--- a/shib-security/src/main/java/net/shibboleth/shared/security/impl/SelfSignedCertificateGenerator.java
+++ b/shib-security/src/main/java/net/shibboleth/shared/security/impl/SelfSignedCertificateGenerator.java
@@ -202,21 +202,24 @@ public class SelfSignedCertificateGenerator {
// Check all the files to prevent overwrite.
- if (args.privateKeyFile != null) {
- if (!args.privateKeyFile.createNewFile()) {
- throw new IOException("Private key file exists: " + args.privateKeyFile.getAbsolutePath());
+ File f = args.privateKeyFile;
+ if (f != null) {
+ if (!f.createNewFile()) {
+ throw new IOException("Private key file exists: " + f.getAbsolutePath());
}
}
- if (args.certificateFile != null) {
- if (!args.certificateFile.createNewFile()) {
- throw new IOException("Certificate file exists: " + args.certificateFile.getAbsolutePath());
+ f = args.certificateFile;
+ if (f != null) {
+ if (!f.createNewFile()) {
+ throw new IOException("Certificate file exists: " + f.getAbsolutePath());
}
}
- if (args.keystoreFile != null) {
- if (!args.keystoreFile.createNewFile()) {
- throw new IOException("KeyStore file exists: " + args.keystoreFile.getAbsolutePath());
+ f = args.keystoreFile;
+ if (f != null) {
+ if (!f.createNewFile()) {
+ throw new IOException("KeyStore file exists: " + f.getAbsolutePath());
}
}
@@ -244,11 +247,13 @@ public class SelfSignedCertificateGenerator {
if (args.keystoreFile != null) {
final KeyStore store = KeyStore.getInstance(args.keystoreType);
store.load(null, null);
- store.setKeyEntry(args.hostname, keypair.getPrivate(), args.keystorePassword.toCharArray(),
+ final String password = args.keystorePassword;
+ assert(password != null);
+ store.setKeyEntry(args.hostname, keypair.getPrivate(), password.toCharArray(),
new X509Certificate[] {certificate});
try (final FileOutputStream keystoreOut = new FileOutputStream(args.keystoreFile)) {
- store.store(keystoreOut, args.keystorePassword.toCharArray());
+ store.store(keystoreOut, password.toCharArray());
keystoreOut.flush();
}
}
@@ -256,11 +261,13 @@ public class SelfSignedCertificateGenerator {
/** Validates the settings. */
protected void validate() {
- if (args.hostname == null || args.hostname.length() == 0) {
+ final String hostname = args.hostname;
+ if (hostname == null || hostname.length() == 0) {
throw new IllegalArgumentException("A non-empty hostname is required");
}
- if (args.keystoreFile != null && (args.keystorePassword == null || args.keystorePassword.length() == 0)) {
+ final String password = args.keystorePassword;
+ if (args.keystoreFile != null && (password == null || password.length() == 0)) {
throw new IllegalArgumentException("Keystore password cannot be null if a keystore file is given");
}
}
diff --git a/shib-security/src/test/java/net/shibboleth/shared/security/DataSealerTest.java b/shib-security/src/test/java/net/shibboleth/shared/security/DataSealerTest.java
index d1b5df4e..f4edba97 100644
--- a/shib-security/src/test/java/net/shibboleth/shared/security/DataSealerTest.java
+++ b/shib-security/src/test/java/net/shibboleth/shared/security/DataSealerTest.java
@@ -38,6 +38,7 @@ import org.testng.annotations.Test;
/**
* Test for {@link DataSealer}.
*/
+ at SuppressWarnings("javadoc")
public class DataSealerTest {
private Resource keystoreResource;
diff --git a/shib-security/src/test/java/net/shibboleth/shared/security/TestResourceConverter.java b/shib-security/src/test/java/net/shibboleth/shared/security/TestResourceConverter.java
index 502daf21..32df4949 100644
--- a/shib-security/src/test/java/net/shibboleth/shared/security/TestResourceConverter.java
+++ b/shib-security/src/test/java/net/shibboleth/shared/security/TestResourceConverter.java
@@ -58,7 +58,7 @@ public final class TestResourceConverter implements net.shibboleth.shared.resour
* @param springResource the input
* @return a {@link Resource} which reflects what the Spring one does
*/
- public static net.shibboleth.shared.resource.Resource of(Resource springResource) {
+ @Nonnull public static net.shibboleth.shared.resource.Resource of(@Nonnull Resource springResource) {
if (springResource instanceof net.shibboleth.shared.resource.Resource) {
return (net.shibboleth.shared.resource.Resource) springResource;
}
@@ -86,17 +86,17 @@ public final class TestResourceConverter implements net.shibboleth.shared.resour
}
/** {@inheritDoc} */
- @Override public URL getURL() throws IOException {
+ @Override @Nonnull public URL getURL() throws IOException {
return springResource.getURL();
}
/** {@inheritDoc} */
- @Override public URI getURI() throws IOException {
+ @Override @Nonnull public URI getURI() throws IOException {
return springResource.getURI();
}
/** {@inheritDoc} */
- @Override public File getFile() throws IOException {
+ @Override @Nonnull public File getFile() throws IOException {
return springResource.getFile();
}
@@ -111,8 +111,8 @@ public final class TestResourceConverter implements net.shibboleth.shared.resour
}
/** {@inheritDoc} */
- @Override public net.shibboleth.shared.resource.Resource createRelativeResource(
- String relativePath) throws IOException {
+ @Override @Nonnull public net.shibboleth.shared.resource.Resource createRelativeResource(
+ @Nonnull String relativePath) throws IOException {
return of(springResource.createRelative(relativePath));
}
@@ -123,7 +123,7 @@ public final class TestResourceConverter implements net.shibboleth.shared.resour
}
/** {@inheritDoc} */
- @Override public String getDescription() {
+ @Override @Nonnull public String getDescription() {
return springResource.getDescription();
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list