tomaswolf commented on code in PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#discussion_r870397695
##########
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:
I based my comment of the [OpenSSH
documentation](https://github.com/openssh/openssh-portable/blob/0086a286ea6bbd11ca9b664ac3bb12b27443d6eb/PROTOCOL.certkeys#L274):
> The encoding and ordering of extensions in this field is identical to that
of the critical options, as is the requirement that each name appear only once.
I had not checked what the OpenSSH code actually does. It appears that
[auth-options.c](https://github.com/openssh/openssh-portable/blob/0086a286ea6bbd11ca9b664ac3bb12b27443d6eb/auth-options.c#L72)
does not enforce _any_ ordering or key uniqueness requirements.
[ssh-keygen.c](https://github.com/openssh/openssh-portable/blob/0086a286ea6bbd11ca9b664ac3bb12b27443d6eb/ssh-keygen.c#L1601)
appears to only sort, but not check for duplicates,
[except](https://github.com/openssh/openssh-portable/blob/0086a286ea6bbd11ca9b664ac3bb12b27443d6eb/ssh-keygen.c#L1964)
for `force-command` and `source-address`.
--
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: dev-unsubscr...@mina.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org
