vukzeka commented on code in PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#discussion_r870194796
##########
sshd-core/src/main/java/org/apache/sshd/certificate/OpenSshCertificateBuilder.java:
##########
@@ -104,7 +110,21 @@ public OpenSshCertificateBuilder
principals(Collection<String> principals) {
}
public OpenSshCertificateBuilder
criticalOptions(List<OpenSshCertificate.CertificateOption> criticalOptions) {
- this.criticalOptions = criticalOptions;
+ if (criticalOptions != null && !criticalOptions.isEmpty()) {
+ // check if any duplicates
+ Set<String> names = new HashSet<String>();
+ Set<String> duplicates = criticalOptions.stream().filter(option ->
!names.add(option.getName()))
+ .map(option -> option.getName())
+ .collect(Collectors.toSet());
+ if (!duplicates.isEmpty()) {
+ throw new IllegalArgumentException("Duplicate critical option:
" + String.join(",", duplicates));
+ }
+ // lexically order by "name"
+ List<OpenSshCertificate.CertificateOption> sortedCriticalOptions =
criticalOptions.stream()
+
.sorted(Comparator.comparing(OpenSshCertificate.CertificateOption::getName))
+ .collect(Collectors.toList());
+ this.criticalOptions = sortedCriticalOptions;
+ }
Review Comment:
@tomaswolf I was not aware that duplicate and sorting are applied for
extensions.
I just checked with ssh-keygen and is possible to add custom extensions and
these are not sorted and also duplicates are allowed
for example following
```
ssh-keygen -s user_ca -I certN -n user -O extension:test=value -O
extension:test=value -O extension:[email protected]=value -O
extension:[email protected]=value -O critical:b_critical_option=value -O
critical:a_critical_option=value -V +10d user-key.pub
```
would generate
``` Type: [email protected] user certificate
Public key: RSA-CERT
SHA256:c6uq2hsYBCROmuFrWz38sIk+0Hlx75l5kOqRo7J1wv8
Signing CA: RSA SHA256:qBnc5EM9d1dlZSfbtnPBh45cAJt5jM7LcLeGDbFOd38
(using rsa-sha2-512)
Key ID: "certN"
Serial: 0
Valid: from 2022-05-11T13:34:00 to 2022-05-21T13:35:41
Principals:
user
Critical Options:
b_critical_option UNKNOWN OPTION (len 9)
a_critical_option UNKNOWN OPTION (len 9)
Extensions:
permit-X11-forwarding
permit-agent-forwarding
permit-port-forwarding
permit-pty
permit-user-rc
test UNKNOWN OPTION (len 9)
test UNKNOWN OPTION (len 9)
[email protected] UNKNOWN OPTION (len 9)
[email protected] UNKNOWN OPTION (len 9)
```
So it looks to me
- duplicates are checked only for standard extensions
- ordering is applied only for standard extensions
- all custom extensions are added in the order as specified and custom
duplicates are allowed
@tomaswolf I can add like that if that sounds correct?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
