This is an automated email from the ASF dual-hosted git repository.
twolf pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit 1e27daaecc2ce3d18b06f66e67f7220dfc65102c
Author: Thomas Wolf <tw...@apache.org>
AuthorDate: Sat Feb 22 14:53:25 2025 +0100
OpenSshCertificate: access options and extensions as maps
A Map<String, String> interface is much more convenient for working with
these options than a List<CertificateOption>. Using sorted maps also
internally for storing these options we can automatically fulfill the
lexicographic ordering and uniqueness requirements.
---
.../common/config/keys/OpenSshCertificate.java | 42 +++++-
.../common/config/keys/OpenSshCertificateImpl.java | 142 ++++++++++++++++++++-
.../certificate/OpenSshCertificateBuilder.java | 108 ++++++++--------
.../sshd/server/auth/pubkey/UserAuthPublicKey.java | 4 +-
.../certificates/OpenSSHCertificateParserTest.java | 10 +-
.../common/auth/PublicKeyAuthenticationTest.java | 3 +-
6 files changed, 235 insertions(+), 74 deletions(-)
diff --git
a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/OpenSshCertificate.java
b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/OpenSshCertificate.java
index d345a3aac..f9f0cef30 100644
---
a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/OpenSshCertificate.java
+++
b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/OpenSshCertificate.java
@@ -20,11 +20,13 @@ package org.apache.sshd.common.config.keys;
import java.security.PrivateKey;
import java.security.PublicKey;
+import java.time.Instant;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
+import java.util.SortedMap;
import java.util.concurrent.TimeUnit;
import org.apache.sshd.common.util.ValidateUtils;
@@ -45,8 +47,7 @@ public interface OpenSshCertificate extends SshPublicKey,
PrivateKey {
/** User key certificate. */
USER,
/** Host key certificate. */
- HOST,
- ;
+ HOST;
public static final List<Type> VALUES =
Collections.unmodifiableList(Arrays.asList(values()));
@@ -147,17 +148,33 @@ public interface OpenSshCertificate extends SshPublicKey,
PrivateKey {
/**
* Retrieves the critical options set in the certificate.
*
- * @return the critical options as a list, never {@code null} but possibly
empty
+ * @return the critical options as an unmodifiable list, never {@code
null} but possibly empty
+ * @see #getCriticalOptionsMap()
*/
List<CertificateOption> getCriticalOptions();
+ /**
+ * Retrieves the critical options set in the certificate.
+ *
+ * @return the critical options as an unmodifiable map, never {@code null}
but possibly empty
+ */
+ SortedMap<String, String> getCriticalOptionsMap();
+
/**
* Retrieves the extensions set in the certificate.
*
- * @return the extensions as a list, never {@code null} but possibly empty
+ * @return the extensions as an unmodifiable list, never {@code null} but
possibly empty
+ * @see #getExtensionsMap()
*/
List<CertificateOption> getExtensions();
+ /**
+ * Retrieves the extensions set in the certificate.
+ *
+ * @return the extensions as an unmodifiable map, never {@code null} but
possibly empty
+ */
+ SortedMap<String, String> getExtensionsMap();
+
/**
* Retrieves the "reserved" field of the certificate. OpenSSH currently
doesn't use it and ignores it.
*
@@ -210,8 +227,21 @@ public interface OpenSshCertificate extends SshPublicKey,
PrivateKey {
*/
static boolean isValidNow(OpenSshCertificate cert) {
long now = TimeUnit.MILLISECONDS.toSeconds(System.currentTimeMillis());
- return Long.compareUnsigned(cert.getValidAfter(), now) <= 0
- && Long.compareUnsigned(now, cert.getValidBefore()) < 0;
+ return Long.compareUnsigned(cert.getValidAfter(), now) <= 0 &&
Long.compareUnsigned(now, cert.getValidBefore()) < 0;
+ }
+
+ /**
+ * Determines whether the given {@link OpenSshCertificate} is valid at the
given {@link Instant}.
+ *
+ * @param cert to check
+ * @return {@code true} if the certificate is valid according to its
timestamps, {@code false} otherwise
+ */
+ static boolean isValidAt(OpenSshCertificate cert, Instant time) {
+ long at = time.getEpochSecond();
+ if (at < 0) {
+ return false;
+ }
+ return Long.compareUnsigned(cert.getValidAfter(), at) <= 0 &&
Long.compareUnsigned(at, cert.getValidBefore()) < 0;
}
/**
diff --git
a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/OpenSshCertificateImpl.java
b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/OpenSshCertificateImpl.java
index e38ced610..f2c182b54 100644
---
a/sshd-common/src/main/java/org/apache/sshd/common/config/keys/OpenSshCertificateImpl.java
+++
b/sshd-common/src/main/java/org/apache/sshd/common/config/keys/OpenSshCertificateImpl.java
@@ -20,14 +20,19 @@ package org.apache.sshd.common.config.keys;
import java.security.PublicKey;
import java.time.Instant;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
import java.util.List;
+import java.util.Map;
+import java.util.SortedMap;
+import java.util.TreeMap;
import java.util.concurrent.TimeUnit;
import org.apache.sshd.common.util.GenericUtils;
import org.apache.sshd.common.util.NumberUtils;
+import org.apache.sshd.common.util.ValidateUtils;
import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
/**
@@ -49,8 +54,8 @@ public class OpenSshCertificateImpl implements
OpenSshCertificate {
private long validAfter = OpenSshCertificate.MIN_EPOCH;
// match ssh-keygen behavior where the default would be forever
private long validBefore = OpenSshCertificate.INFINITY;
- private List<CertificateOption> criticalOptions;
- private List<CertificateOption> extensions;
+ private SortedMap<String, String> criticalOptions;
+ private SortedMap<String, String> extensions;
private String reserved;
private PublicKey caPubKey;
private byte[] message;
@@ -112,12 +117,32 @@ public class OpenSshCertificateImpl implements
OpenSshCertificate {
@Override
public List<CertificateOption> getCriticalOptions() {
- return criticalOptions == null ? Collections.emptyList() :
criticalOptions;
+ if (criticalOptions == null || criticalOptions.isEmpty()) {
+ return Collections.emptyList();
+ }
+ List<CertificateOption> list = new ArrayList<>(criticalOptions.size());
+ criticalOptions.forEach((k, v) -> list.add(new CertificateOption(k,
v)));
+ return Collections.unmodifiableList(list);
+ }
+
+ @Override
+ public SortedMap<String, String> getCriticalOptionsMap() {
+ return criticalOptions == null ? Collections.emptySortedMap() :
Collections.unmodifiableSortedMap(criticalOptions);
}
@Override
public List<CertificateOption> getExtensions() {
- return extensions == null ? Collections.emptyList() : extensions;
+ if (extensions == null || extensions.isEmpty()) {
+ return Collections.emptyList();
+ }
+ List<CertificateOption> list = new ArrayList<>(extensions.size());
+ extensions.forEach((k, v) -> list.add(new CertificateOption(k, v)));
+ return Collections.unmodifiableList(list);
+ }
+
+ @Override
+ public SortedMap<String, String> getExtensionsMap() {
+ return extensions == null ? Collections.emptySortedMap() :
Collections.unmodifiableSortedMap(extensions);
}
@Override
@@ -236,12 +261,117 @@ public class OpenSshCertificateImpl implements
OpenSshCertificate {
}
}
+ /**
+ * Sets the critical options of the certificate, overriding any options
set earlier.
+ *
+ * @param criticalOptions to set; may be {@code null} or empty to remove
all previously set options
+ */
public void setCriticalOptions(List<CertificateOption> criticalOptions) {
- this.criticalOptions = criticalOptions;
+ if (criticalOptions == null || criticalOptions.isEmpty()) {
+ this.criticalOptions = null;
+ } else {
+ this.criticalOptions = new TreeMap<>();
+ for (CertificateOption option : criticalOptions) {
+ String data = option.getData();
+ this.criticalOptions.put(option.getName(), data == null ? "" :
data);
+ }
+ }
+ }
+
+ /**
+ * Sets the critical options of the certificate, overriding any options
set earlier.
+ *
+ * @param criticalOptions to set; may be {@code null} or empty to remove
all previously set options
+ */
+ public void setCriticalOptions(Map<String, String> criticalOptions) {
+ if (criticalOptions == null || criticalOptions.isEmpty()) {
+ this.criticalOptions = null;
+ } else {
+ this.criticalOptions = new TreeMap<>();
+ criticalOptions.forEach((k, v) -> {
+ String data = v == null ? "" : v;
+ this.criticalOptions.put(k, data);
+ });
+ }
+ }
+
+ /**
+ * Adds a critical option to the certificate, or removes it if {@code
value == null}. To add an option with an empty
+ * value, use an empty string as value. If the certificate already has an
option with the given name it is replaced.
+ *
+ * @param name of the option to set
+ * @param value of the option
+ * @return {@code true} if the map did not contain the name; {@code
false} if it did
+ */
+ public boolean addCriticalOption(String name, String value) {
+ String key = ValidateUtils.checkNotNullAndNotEmpty(name, "Critical
option name must be set");
+ if (value == null) {
+ if (criticalOptions == null) {
+ return true;
+ }
+ return criticalOptions.remove(key) == null;
+ }
+ if (criticalOptions == null) {
+ criticalOptions = new TreeMap<>();
+ }
+ return criticalOptions.put(key, value) == null;
}
+ /**
+ * Sets the extensions of the certificate, overriding any extensions set
earlier.
+ *
+ * @param extensions to set; may be {@code null} or empty to remove all
previously set extensions
+ */
public void setExtensions(List<CertificateOption> extensions) {
- this.extensions = extensions;
+ if (extensions == null || extensions.isEmpty()) {
+ this.extensions = null;
+ } else {
+ this.extensions = new TreeMap<>();
+ for (CertificateOption option : extensions) {
+ String data = option.getData();
+ this.extensions.put(option.getName(), data == null ? "" :
data);
+ }
+ }
+ }
+
+ /**
+ * Sets the extensions of the certificate, overriding any extensions set
earlier.
+ *
+ * @param extensions to set; may be {@code null} or empty to remove all
previously set extensions
+ */
+ public void setExtensions(Map<String, String> extensions) {
+ if (extensions == null || extensions.isEmpty()) {
+ this.extensions = null;
+ } else {
+ this.extensions = new TreeMap<>();
+ extensions.forEach((k, v) -> {
+ String data = v == null ? "" : v;
+ this.extensions.put(k, data);
+ });
+ }
+ }
+
+ /**
+ * Adds an extension to the certificate, or removes it if {@code value ==
null}. To add an extension with an empty
+ * value, use an empty string as value. If the certificate already has an
extension with the given name it is
+ * replaced.
+ *
+ * @param name of the extension to set
+ * @param value of the extension
+ * @return {@code true} if the map did not contain the name; {@code
false} if it did
+ */
+ public boolean addExtension(String name, String value) {
+ String key = ValidateUtils.checkNotNullAndNotEmpty(name, "Extension
name must be set");
+ if (value == null) {
+ if (extensions == null) {
+ return true;
+ }
+ return extensions.remove(key) == null;
+ }
+ if (extensions == null) {
+ extensions = new TreeMap<>();
+ }
+ return extensions.put(key, value) == null;
}
public void setReserved(String reserved) {
diff --git
a/sshd-core/src/main/java/org/apache/sshd/certificate/OpenSshCertificateBuilder.java
b/sshd-core/src/main/java/org/apache/sshd/certificate/OpenSshCertificateBuilder.java
index e8e6597e3..5d5f51ed8 100644
---
a/sshd-core/src/main/java/org/apache/sshd/certificate/OpenSshCertificateBuilder.java
+++
b/sshd-core/src/main/java/org/apache/sshd/certificate/OpenSshCertificateBuilder.java
@@ -25,18 +25,15 @@ import java.security.SecureRandom;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.Collections;
-import java.util.Comparator;
-import java.util.HashSet;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.Set;
-import java.util.stream.Collectors;
import org.apache.sshd.common.BaseBuilder;
import org.apache.sshd.common.NamedFactory;
import org.apache.sshd.common.config.keys.KeyUtils;
import org.apache.sshd.common.config.keys.OpenSshCertificate;
+import org.apache.sshd.common.config.keys.OpenSshCertificate.CertificateOption;
import org.apache.sshd.common.config.keys.OpenSshCertificate.Type;
import org.apache.sshd.common.config.keys.OpenSshCertificateImpl;
import org.apache.sshd.common.keyprovider.KeyPairProvider;
@@ -68,10 +65,8 @@ public class OpenSshCertificateBuilder {
protected long serial;
protected String id;
protected Collection<String> principals;
- // criticalOptions and extensions must be lexically ordered by "name" if
they appear in the
- // sequence. Each named option may only appear once in a certificate.
- protected List<OpenSshCertificate.CertificateOption> criticalOptions;
- protected List<OpenSshCertificate.CertificateOption> extensions;
+ protected final Map<String, String> criticalOptions = new HashMap<>();
+ protected final Map<String, String> extensions = new HashMap<>();
// match ssh-keygen behavior where the default would be forever
protected long validAfter = OpenSshCertificate.MIN_EPOCH;
// match ssh-keygen behavior where the default would be forever
@@ -111,15 +106,59 @@ public class OpenSshCertificateBuilder {
return this;
}
+ public OpenSshCertificateBuilder criticalOption(String name, String data) {
+ String key = ValidateUtils.checkNotNullAndNotEmpty(name, "Critical
option name must be set");
+ this.criticalOptions.put(key, data == null ? "" : data);
+ return this;
+ }
+
public OpenSshCertificateBuilder
criticalOptions(List<OpenSshCertificate.CertificateOption> criticalOptions) {
- validateOptions(criticalOptions);
- this.criticalOptions = lexicallyOrderOptions(criticalOptions);
+ this.criticalOptions.clear();
+ for (CertificateOption option : criticalOptions) {
+ String key =
ValidateUtils.checkNotNullAndNotEmpty(option.getName(), "Critical option name
must be set");
+ String value = option.getData();
+ String prev = this.criticalOptions.put(key, value == null ? "" :
value);
+ if (prev != null) {
+ throw new IllegalArgumentException("Duplicate certificate
option " + key);
+ }
+ }
+ return this;
+ }
+
+ public OpenSshCertificateBuilder criticalOptions(Map<String, String>
criticalOptions) {
+ this.criticalOptions.clear();
+ criticalOptions.forEach((k, v) -> {
+ String key = ValidateUtils.checkNotNullAndNotEmpty(k, "Critical
option name must be set");
+ this.criticalOptions.put(key, v == null ? "" : v);
+ });
+ return this;
+ }
+
+ public OpenSshCertificateBuilder extension(String name, String data) {
+ String key = ValidateUtils.checkNotNullAndNotEmpty(name, "Extension
name must be set");
+ this.extensions.put(key, data == null ? "" : data);
return this;
}
public OpenSshCertificateBuilder
extensions(List<OpenSshCertificate.CertificateOption> extensions) {
- validateOptions(extensions);
- this.extensions = lexicallyOrderOptions(extensions);
+ this.extensions.clear();
+ for (CertificateOption option : extensions) {
+ String key =
ValidateUtils.checkNotNullAndNotEmpty(option.getName(), "Extension name must be
set");
+ String value = option.getData();
+ String prev = this.extensions.put(key, value == null ? "" : value);
+ if (prev != null) {
+ throw new IllegalArgumentException("Duplicate certificate
extension " + key);
+ }
+ }
+ return this;
+ }
+
+ public OpenSshCertificateBuilder extensions(Map<String, String>
extensions) {
+ this.extensions.clear();
+ extensions.forEach((k, v) -> {
+ String key = ValidateUtils.checkNotNullAndNotEmpty(k, "Extension
name must be set");
+ this.extensions.put(key, v == null ? "" : v);
+ });
return this;
}
@@ -228,11 +267,11 @@ public class OpenSshCertificateBuilder {
if (principals != null && !principals.isEmpty()) {
cert.setPrincipals(new ArrayList<>(principals));
}
- if (criticalOptions != null && !criticalOptions.isEmpty()) {
- cert.setCriticalOptions(new ArrayList<>(criticalOptions));
+ if (!criticalOptions.isEmpty()) {
+ cert.setCriticalOptions(criticalOptions);
}
- if (extensions != null && !extensions.isEmpty()) {
- cert.setExtensions(new ArrayList<>(extensions));
+ if (!extensions.isEmpty()) {
+ cert.setExtensions(extensions);
}
cert.setValidAfter(validAfter);
cert.setValidBefore(validBefore);
@@ -277,39 +316,4 @@ public class OpenSshCertificateBuilder {
return cert;
}
-
- /**
- * Validates that there are no duplicate options.
- *
- * @param options the options to check
- * @throws IllegalArgumentException if there are duplicates
- */
- private void validateOptions(List<OpenSshCertificate.CertificateOption>
options) {
- if (options != null && !options.isEmpty()) {
- // check if any duplicates
- Set<String> names = new HashSet<>();
- Set<String> duplicates = options.stream().filter(option ->
!names.add(option.getName()))
- .map(OpenSshCertificate.CertificateOption::getName)
- .collect(Collectors.toSet());
- if (!duplicates.isEmpty()) {
- throw new IllegalArgumentException("Duplicate option: " +
duplicates);
- }
- }
- }
-
- /**
- * Lexically orders certificate options by name.
- *
- * @param options the options to order
- * @return a list containing the options in lexical order
- */
- private List<OpenSshCertificate.CertificateOption> lexicallyOrderOptions(
- List<OpenSshCertificate.CertificateOption> options) {
- if (options != null && !options.isEmpty()) {
- return options.stream()
-
.sorted(Comparator.comparing(OpenSshCertificate.CertificateOption::getName))
- .collect(Collectors.toList());
- }
- return Collections.emptyList();
- }
}
diff --git
a/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/UserAuthPublicKey.java
b/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/UserAuthPublicKey.java
index 3280170de..d16534373 100644
---
a/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/UserAuthPublicKey.java
+++
b/sshd-core/src/main/java/org/apache/sshd/server/auth/pubkey/UserAuthPublicKey.java
@@ -33,7 +33,6 @@ import org.apache.sshd.common.RuntimeSshException;
import org.apache.sshd.common.SshConstants;
import org.apache.sshd.common.config.keys.KeyUtils;
import org.apache.sshd.common.config.keys.OpenSshCertificate;
-import org.apache.sshd.common.config.keys.OpenSshCertificate.CertificateOption;
import org.apache.sshd.common.net.InetAddressRange;
import org.apache.sshd.common.signature.Signature;
import org.apache.sshd.common.signature.SignatureFactoriesManager;
@@ -204,8 +203,7 @@ public class UserAuthPublicKey extends AbstractUserAuth
implements SignatureFact
}
protected void verifyCertificateSources(ServerSession session,
OpenSshCertificate cert) throws CertificateException {
- String allowedSources = cert.getCriticalOptions().stream().filter(c ->
"source-address".equals(c.getName()))
- .map(CertificateOption::getData).findAny().orElse(null);
+ String allowedSources =
cert.getCriticalOptionsMap().get("source-address");
if (allowedSources == null) {
return;
}
diff --git
a/sshd-core/src/test/java/org/apache/sshd/certificates/OpenSSHCertificateParserTest.java
b/sshd-core/src/test/java/org/apache/sshd/certificates/OpenSSHCertificateParserTest.java
index a2e317d11..9c69e7816 100644
---
a/sshd-core/src/test/java/org/apache/sshd/certificates/OpenSSHCertificateParserTest.java
+++
b/sshd-core/src/test/java/org/apache/sshd/certificates/OpenSSHCertificateParserTest.java
@@ -98,11 +98,11 @@ public class OpenSSHCertificateParserTest extends
BaseTestSupport {
assertTrue(typedCert.getCriticalOptions().isEmpty());
assertEquals(
Arrays.asList(
- new
OpenSshCertificate.CertificateOption("permit-X11-forwarding"),
- new
OpenSshCertificate.CertificateOption("permit-agent-forwarding"),
- new
OpenSshCertificate.CertificateOption("permit-port-forwarding"),
- new
OpenSshCertificate.CertificateOption("permit-pty"),
- new
OpenSshCertificate.CertificateOption("permit-user-rc")),
+ new
OpenSshCertificate.CertificateOption("permit-X11-forwarding", ""),
+ new
OpenSshCertificate.CertificateOption("permit-agent-forwarding", ""),
+ new
OpenSshCertificate.CertificateOption("permit-port-forwarding", ""),
+ new
OpenSshCertificate.CertificateOption("permit-pty", ""),
+ new
OpenSshCertificate.CertificateOption("permit-user-rc", "")),
typedCert.getExtensions());
assertEquals(params.sigAlgorithm,
typedCert.getSignatureAlgorithm());
verifySignature(typedCert);
diff --git
a/sshd-core/src/test/java/org/apache/sshd/common/auth/PublicKeyAuthenticationTest.java
b/sshd-core/src/test/java/org/apache/sshd/common/auth/PublicKeyAuthenticationTest.java
index 67a7d5949..7d8a2db04 100644
---
a/sshd-core/src/test/java/org/apache/sshd/common/auth/PublicKeyAuthenticationTest.java
+++
b/sshd-core/src/test/java/org/apache/sshd/common/auth/PublicKeyAuthenticationTest.java
@@ -49,7 +49,6 @@ import org.apache.sshd.common.SshException;
import org.apache.sshd.common.config.keys.FilePasswordProvider;
import org.apache.sshd.common.config.keys.KeyUtils;
import org.apache.sshd.common.config.keys.OpenSshCertificate;
-import org.apache.sshd.common.config.keys.OpenSshCertificate.CertificateOption;
import org.apache.sshd.common.config.keys.OpenSshCertificateImpl;
import org.apache.sshd.common.keyprovider.KeyIdentityProvider;
import org.apache.sshd.common.keyprovider.KeyPairProvider;
@@ -593,7 +592,7 @@ public class PublicKeyAuthenticationTest extends
AuthenticationTestSupport {
.id("test-cert-ecdsa-sha2-nistp256") //
.validBefore(System.currentTimeMillis() +
TimeUnit.HOURS.toMillis(1)) //
.principals(Collections.singletonList("user01")) //
- .criticalOptions(Collections.singletonList(new
CertificateOption("source-address", sources)))
+ .criticalOption("source-address", sources)
.sign(caKeypair, "ecdsa-sha2-nistp256");
// Configure the ssh server
