[ https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=769469&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769469 ]
ASF GitHub Bot logged work on SSHD-1266: ---------------------------------------- Author: ASF GitHub Bot Created on: 12/May/22 08:30 Start Date: 12/May/22 08:30 Worklog Time Spent: 10m Work Description: vukzeka commented on code in PR #222: URL: https://github.com/apache/mina-sshd/pull/222#discussion_r871096735 ########## sshd-core/src/main/java/org/apache/sshd/certificate/OpenSshCertificateBuilder.java: ########## @@ -267,4 +275,38 @@ public OpenSshCertificate sign(KeyPair caKeypair, String signatureAlgorithm) thr return cert; } + + /** + * Validate if certificate options are correct + * + * @param options Options + */ + 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(option -> option.getName()) Review Comment: Done ########## sshd-core/src/main/java/org/apache/sshd/certificate/OpenSshCertificateBuilder.java: ########## @@ -267,4 +275,38 @@ public OpenSshCertificate sign(KeyPair caKeypair, String signatureAlgorithm) thr return cert; } + + /** + * Validate if certificate options are correct + * + * @param options Options + */ + 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(option -> option.getName()) + .collect(Collectors.toSet()); + if (!duplicates.isEmpty()) { + throw new IllegalArgumentException("Duplicate option: " + String.join(",", duplicates)); Review Comment: Done Issue Time Tracking ------------------- Worklog Id: (was: 769469) Time Spent: 3h 50m (was: 3h 40m) > OpenSSH certificate is not properly encoded when critical options are included > ------------------------------------------------------------------------------ > > Key: SSHD-1266 > URL: https://issues.apache.org/jira/browse/SSHD-1266 > Project: MINA SSHD > Issue Type: Bug > Affects Versions: 2.8.0 > Reporter: Zeljko Vukovic > Priority: Minor > Time Spent: 3h 50m > Remaining Estimate: 0h > > If critical options are included when OpenSSH certificate is created same > can't be read with OpenSSH. > > In oder to reproduce issue we can use existing test > [https://github.com/apache/mina-sshd/blob/master/sshd-core/src/test/java/org/apache/sshd/certificates/GenerateOpenSSHClientCertificateTest.java#L152] > and just add critical options (as in the example bellow) > {code:java} > final OpenSshCertificate signedCert = > OpenSshCertificateBuilder.userCertificate() > .serial(0L) > .publicKey(clientPublicKey) > .id("user01") > .principals(Collections.singletonList("user01")) > > .criticalOptions(Arrays.asList( > new > OpenSshCertificate.CertificateOption("force-command", "wget url"), > new > OpenSshCertificate.CertificateOption("source-address", "127.0.0.1/32"))) > > .extensions(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"))) > .sign(caKeypair, signatureAlgorithm); {code} > > Once we check such certificate we get following error > {code:java} > > ssh-keygen -L -f /path/to/cert.pub > Type: ecdsa-sha2-nistp256-cert-...@openssh.com user certificate > Public key: ECDSA-CERT > SHA256:0ITcONLKI/H/FNpXZVZMaEYB0STXD4BQNBkSSuDpk5U > Signing CA: ECDSA SHA256:KPz5LunqQBL9hWJx5eDk11T16anJCn40d/g480PfuKw > (using ecdsa-sha2-nistp384) > Key ID: "user01" > Serial: 0 > Valid: forever > Principals: > user01 > Critical Options: > show_options: buffer error: string is too large {code} > and similar for the other cert types(RSA, EC, Ed25519). > I was tracing this issue and it looks like related to this code > [https://github.com/apache/mina-sshd/blob/master/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java#L840] > but I was not able to figure out what exactly. > > Interesting is that parsing certificate is working as expected > [https://github.com/apache/mina-sshd/blob/master/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java#L370] > from code but also even if I create certificate directly with ssh-keygen > {code:java} > ssh-keygen -t rsa -b 4096 -f user_ca -C user_ca > ssh-keygen -f user-key -b 4096 -t rsa > ssh-keygen -s user_ca -I certN -n user -O source-address="127.0.0.1/32" -O > force-command="wget url" -V +10d user-key.pub {code} > > [~alex.sher...@gmail.com] / [~twolf] please if any hints what to check(it > looks to me that there is something wrong with encoding certificate option > data > [https://github.com/apache/mina-sshd/blob/master/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java#L838-L845] > , like these tuples should be written somehow differently) I am more than > open to support and create PR. > This is working as expected for extensions as these are all empty(do not have > data) but once we include critical options which have data than there is > mentioned failure > ([https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys#L221-L268] > ). > > -- This message was sent by Atlassian Jira (v8.20.7#820007) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org