[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=770087=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-770087
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 13/May/22 09:23
Start Date: 13/May/22 09:23
Worklog Time Spent: 10m
Work Description: vukzeka commented on PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#issuecomment-1125839242
> Looks fine to me. I'll download, undo Lyor's maven formatter change,
rebase, run the tests locally, and if successful squash-merge this later.
Thanks @tomaswolf
Issue Time Tracking
---
Worklog Id: (was: 770087)
Time Spent: 4h 50m (was: 4h 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
> Fix For: 2.9.0
>
> Time Spent: 4h 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]
> ).
>
>
--
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=770058=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-770058
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 13/May/22 07:43
Start Date: 13/May/22 07:43
Worklog Time Spent: 10m
Work Description: tomaswolf merged PR #222:
URL: https://github.com/apache/mina-sshd/pull/222
Issue Time Tracking
---
Worklog Id: (was: 770058)
Time Spent: 4h 40m (was: 4.5h)
> 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: 4h 40m
> 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:
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=769765=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769765
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 12/May/22 17:06
Start Date: 12/May/22 17:06
Worklog Time Spent: 10m
Work Description: lgoldstein commented on PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#issuecomment-1125229695
Done
Issue Time Tracking
---
Worklog Id: (was: 769765)
Time Spent: 4.5h (was: 4h 20m)
> 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: 4.5h
> 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
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=769757=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769757
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 12/May/22 16:37
Start Date: 12/May/22 16:37
Worklog Time Spent: 10m
Work Description: lgoldstein commented on PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#issuecomment-1125204883
@tomaswolf
> please downgrade the formatter maven plug-in again. (Revert commit
https://github.com/apache/mina-sshd/commit/9f8aa3dbafe3934bad661d36a1d6bf20e80baada.)
It doesn't run with Java 1.8
On it... sorry (again)
Issue Time Tracking
---
Worklog Id: (was: 769757)
Time Spent: 4h 20m (was: 4h 10m)
> 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: 4h 20m
> 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
>
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=769474=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769474
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 12/May/22 08:37
Start Date: 12/May/22 08:37
Worklog Time Spent: 10m
Work Description: vukzeka commented on PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#issuecomment-1124693963
> Looks good, some minor comments.
>
> I notice that the tests add the options already in the correct order. Now
that we do sort I'd suggest to change the tests to add them in a wrong order.
>
> If you want to do a testcontainers test, I suggest to do a new PR once
this one is merged.
Hope is ok now.
Yes, I will add also one test for ssh-keygen later this week and that will
be separate PR.
Issue Time Tracking
---
Worklog Id: (was: 769474)
Time Spent: 4h 10m (was: 4h)
> 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: 4h 10m
> 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
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=769473=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769473
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 12/May/22 08:36
Start Date: 12/May/22 08:36
Worklog Time Spent: 10m
Work Description: vukzeka commented on PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#issuecomment-1124693099
> @vukzeka I just pushed a commit
[alex-sherwin@f5966b4](https://github.com/alex-sherwin/mina-sshd/commit/f5966b48e3ac2ab375269aae553c765cf62488cf)
>
> It shows how to use testcontainers to build an image w/ ssh-keygen and:
>
> 1. How you can use **ssh-keygen** to parse/view a OpenSSH Certificate file
and check the exit code
> 2. How you can use **ssh-keygen** to generate data and copy the contents
back to Java
>
> This test is _not_ using the testcontainers JUnit automatic test lifecycle
stuff (where you define the `GenericContainer` as a class-level property or
static prop with `@ClassRule` annotations to manage the container lifecycle,
but, testcontainers will still auto cleanup/reap the container and images after
JVM exit automagically).
>
> This is because all that stuff is geared towards long-running daemon
services, etc.
>
> Instead, the unit test itself is defining the docker image + container,
starting it, and using a custom wait strategy (which checks for exit, but does
not assert the exit code itself), so that the unit test function can inspect
the container exit code and logs and apply junit assertions to them.
>
> If you do add a test using this kind of technique, you should probably set
it up to be a parameterized test like **GenerateOpenSSHClientCertificateTest**
with all the cert type variations
Great, thanks @alex-sherwin
[This](https://github.com/alex-sherwin/mina-sshd/commit/f5966b48e3ac2ab375269aae553c765cf62488cf#diff-87ff30dcfbfe5636d9eb3413bb6530396eeea5409eae18163ac9561951bcedf2R113)
and
[this](https://github.com/alex-sherwin/mina-sshd/commit/f5966b48e3ac2ab375269aae553c765cf62488cf#diff-87ff30dcfbfe5636d9eb3413bb6530396eeea5409eae18163ac9561951bcedf2R137-R144)
is exactly what i was looking for.
Something like that would help to not store pre-generated certs on the repo
already test can
- generate keys using ssh-keygen
- generate certs using ssh-keygen
- parse them using code
- generate certs from code
- and validate certs back with ssh-keygen
and more important will be usable also for some other tests not related to
this functionality.
I will create some example later this week but will be separate from this
PR.
Issue Time Tracking
---
Worklog Id: (was: 769473)
Time Spent: 4h (was: 3h 50m)
> 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: 4h
> 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
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=769469=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
options) {
+if (options != null && !options.isEmpty()) {
+// check if any duplicates
+Set names = new HashSet<>();
+Set 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
options) {
+if (options != null && !options.isEmpty()) {
+// check if any duplicates
+Set names = new HashSet<>();
+Set 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
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=769451=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769451
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 12/May/22 07:40
Start Date: 12/May/22 07:40
Worklog Time Spent: 10m
Work Description: tomaswolf commented on code in PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#discussion_r871043739
##
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
options) {
+if (options != null && !options.isEmpty()) {
+// check if any duplicates
+Set names = new HashSet<>();
+Set 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));
+}
+}
+}
+
+/**
+ * Lexically order certificate options by "name"
+ *
+ * @param options Options
+ * @return Lexically ordered options
+ */
+private List lexicallyOrderOptions(
+List options) {
+if (options != null && !options.isEmpty()) {
+return options.stream()
+
.sorted(Comparator.comparing(OpenSshCertificate.CertificateOption::getName))
+.collect(Collectors.toList());
+}
+return options;
Review Comment:
I would return here `Collections.emptyList()`. That would ensure that the
builder stores a copy of the parameter the user passed in, and that subsequent
modifications of the original list can have no effect on the builder.
##
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
options) {
+if (options != null && !options.isEmpty()) {
+// check if any duplicates
+Set names = new HashSet<>();
+Set duplicates = options.stream().filter(option ->
!names.add(option.getName()))
+.map(option -> option.getName())
Review Comment:
Why not `.map(OpenSshCertificate.CertificateOption::getName)`?
##
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
options) {
+if (options != null && !options.isEmpty()) {
+// check if any duplicates
+Set names = new HashSet<>();
+Set 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:
`Set.toString()` will enumerate the content. So perhaps just `+ duplicates`
instead of the `String.join(...)`?
Issue Time Tracking
---
Worklog Id: (was: 769451)
Time Spent: 3h 40m (was: 3.5h)
> 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 40m
> 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}
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=769389=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769389
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 12/May/22 02:13
Start Date: 12/May/22 02:13
Worklog Time Spent: 10m
Work Description: alex-sherwin commented on PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#issuecomment-1124459535
@vukzeka I just pushed a commit
https://github.com/alex-sherwin/mina-sshd/commit/f5966b48e3ac2ab375269aae553c765cf62488cf
It shows how to use testcontainers to build an image w/ ssh-keygen and:
1. How you can use **ssh-keygen** to parse/view a OpenSSH Certificate file
and check the exit code
2. How you can use **ssh-keygen** to generate data and copy the contents
back to Java (this one is not really that useful for what we're discussing
here, I just created it to see how it would be done)
This test is *not* using the testcontainers JUnit automatic test lifecycle
stuff (where you define the `GenericContainer` as a class-level property or
static prop with `@ClassRule` annotations to manage the container lifecycle).
This is because all that stuff is geared towards long-running services, etc.
Instead, the unit test itself is defining the docker image + container,
starting it, and using a custom wait strategy (which checks for exit, but does
not assert the exit code itself), so that the unit test function can inspect
the container exit code and logs and apply junit assertions to them.
If you do add a test using this kind of technique, you should probably set
it up to be a parameterized test like **GenerateOpenSSHClientCertificateTest**
with all the cert type variations
Issue Time Tracking
---
Worklog Id: (was: 769389)
Time Spent: 3.5h (was: 3h 20m)
> 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: 3.5h
> 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
>
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=769365=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769365
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 12/May/22 00:18
Start Date: 12/May/22 00:18
Worklog Time Spent: 10m
Work Description: vukzeka commented on PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#issuecomment-1124402694
> As I noted on https://issues.apache.org/jira/browse/SSHD-1266
>
> The original PR's for the OpenSSH certificate support brought in support
for Testcontainers based Docker unit testing
>
> Perhaps this PR should include a new unit test which uses
Testcontainers/Docker to use the real OpenSSH _ssh-keygen_ binary to parse the
OpenSSH Certificate output from MINA?
>
> It could minimally use _ssh-keygen_ to parse the MINA generated OpenSSH
Certificate and check for a zero exit code
Hi @alex-sherwin
It was not in the "scope" but I was thinking same. Not only for this use
case but in general something like that would be useful.
I just saw test you were referring and I'm thinking that probably easiest
would be to have image with **ssh-keygen**, something like
```
FROM alpine:3.15.4
RUN apk update && \
apk add --no-cache openssh-keygen bash && \
rm -rf /var/cache/apk/*
CMD ["ssh-keygen"]
```
and then run same as **OneShotStartupCheckStrategy** supplying different
commands
`ssh-keygen COMMAND`
But question if you know, is there easy way to capture different outputs of
interest (generated files, exit code, stdout, stderror, etc.) with such
approach?
Issue Time Tracking
---
Worklog Id: (was: 769365)
Time Spent: 3h 20m (was: 3h 10m)
> 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 20m
> 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
>
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=769353=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769353
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 11/May/22 23:27
Start Date: 11/May/22 23:27
Worklog Time Spent: 10m
Work Description: vukzeka commented on code in PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#discussion_r870828179
##
sshd-core/src/test/java/org/apache/sshd/certificates/GenerateOpenSSHClientCertificateTest.java:
##
@@ -154,7 +150,12 @@ public void signCertificate() throws Exception {
.publicKey(clientPublicKey)
.id("user01")
.principals(Collections.singletonList("user01"))
+.criticalOptions(Arrays.asList(
+new
OpenSshCertificate.CertificateOption("force-command", "/path/to/script.sh"),
+new
OpenSshCertificate.CertificateOption("source-address", "127.0.0.1/32"),
+new
OpenSshCertificate.CertificateOption("verify-required")))
Review Comment:
ok, just regenerated certs but i have separated them from the old ones as
there is other test which is using same and it fails with dummy critical
options (source-address, force-command). There is sh script which can be used
to easily generate/regenerate if needed
Issue Time Tracking
---
Worklog Id: (was: 769353)
Time Spent: 3h 10m (was: 3h)
> 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 10m
> 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"
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=769316=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769316
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 11/May/22 21:35
Start Date: 11/May/22 21:35
Worklog Time Spent: 10m
Work Description: vukzeka commented on code in PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#discussion_r870774200
##
sshd-core/src/main/java/org/apache/sshd/certificate/OpenSshCertificateBuilder.java:
##
@@ -104,7 +110,21 @@ public OpenSshCertificateBuilder
principals(Collection principals) {
}
public OpenSshCertificateBuilder
criticalOptions(List criticalOptions) {
-this.criticalOptions = criticalOptions;
+if (criticalOptions != null && !criticalOptions.isEmpty()) {
+// check if any duplicates
+Set names = new HashSet();
+Set 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 sortedCriticalOptions =
criticalOptions.stream()
+
.sorted(Comparator.comparing(OpenSshCertificate.CertificateOption::getName))
+.collect(Collectors.toList());
+this.criticalOptions = sortedCriticalOptions;
+}
Review Comment:
I missed that part from OpenSSH documentation. And good catch for that bug.
I will apply now same logic for both extensions.
Issue Time Tracking
---
Worklog Id: (was: 769316)
Time Spent: 3h (was: 2h 50m)
> 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
> 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
>
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=769155=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769155
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 11/May/22 15:45
Start Date: 11/May/22 15:45
Worklog Time Spent: 10m
Work Description: tomaswolf commented on code in PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#discussion_r870480760
##
sshd-core/src/main/java/org/apache/sshd/certificate/OpenSshCertificateBuilder.java:
##
@@ -104,7 +110,21 @@ public OpenSshCertificateBuilder
principals(Collection principals) {
}
public OpenSshCertificateBuilder
criticalOptions(List criticalOptions) {
-this.criticalOptions = criticalOptions;
+if (criticalOptions != null && !criticalOptions.isEmpty()) {
+// check if any duplicates
+Set names = new HashSet();
+Set 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 sortedCriticalOptions =
criticalOptions.stream()
+
.sorted(Comparator.comparing(OpenSshCertificate.CertificateOption::getName))
+.collect(Collectors.toList());
+this.criticalOptions = sortedCriticalOptions;
+}
Review Comment:
Please see OpenSSH [bug
3198](https://bugzilla.mindrot.org/show_bug.cgi?id=3198), fixed in OpenSSH 8.4
via [this
commit](https://github.com/openssh/openssh-portable/commit/2d8a3b7e8b0408dfeb933ac5cfd3a58f5bac49af).
Issue Time Tracking
---
Worklog Id: (was: 769155)
Time Spent: 2h 50m (was: 2h 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: 2h 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
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=769115=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769115
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 11/May/22 14:40
Start Date: 11/May/22 14:40
Worklog Time Spent: 10m
Work Description: 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 principals) {
}
public OpenSshCertificateBuilder
criticalOptions(List criticalOptions) {
-this.criticalOptions = criticalOptions;
+if (criticalOptions != null && !criticalOptions.isEmpty()) {
+// check if any duplicates
+Set names = new HashSet();
+Set 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 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`.
Issue Time Tracking
---
Worklog Id: (was: 769115)
Time Spent: 2h 40m (was: 2.5h)
> 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: 2h 40m
> 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:
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=769114=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769114
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 11/May/22 14:40
Start Date: 11/May/22 14:40
Worklog Time Spent: 10m
Work Description: 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 principals) {
}
public OpenSshCertificateBuilder
criticalOptions(List criticalOptions) {
-this.criticalOptions = criticalOptions;
+if (criticalOptions != null && !criticalOptions.isEmpty()) {
+// check if any duplicates
+Set names = new HashSet();
+Set 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 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}}.
Issue Time Tracking
---
Worklog Id: (was: 769114)
Time Spent: 2.5h (was: 2h 20m)
> 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: 2.5h
> 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:
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=769013=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769013
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 11/May/22 11:41
Start Date: 11/May/22 11:41
Worklog Time Spent: 10m
Work Description: vukzeka commented on code in PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#discussion_r870197910
##
sshd-common/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java:
##
@@ -348,29 +348,36 @@ public List
getCertificateOptions() {
/**
* According to https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys#L222-L262;>PROTOCOL.certkeys:
+ *
"https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys#L221-L319;>PROTOCOL.certkeys:
*
* Critical Options is a set of bytes that is
*
- * [overall length][name(string)][data(string)]...
+ * [overall length][name(string)][[length of buffer][[length of
string][data(string)]]]...
*
- * Where each Critical Option is encoded as a name (string) and data
(string)
+ * Where each Certificate Option is encoded as a name (string) and buffer
of data (string packed in a buffer)
*
- * Then the entire name + data strings are added as bytes (which will get
a length prefix)
+ * Then the entire name (string) + data (buffer) are added as bytes (which
will get a length prefix)
*
* @param charset {@link Charset} to use for converting bytes to
characters
* @return the parsed result, never {@code null}, but possibly
empty
*/
public List
getCertificateOptions(Charset charset) {
-// pull out entire Critical Options section
-final ByteArrayBuffer optionBuffer = new ByteArrayBuffer(getBytes());
-
List list = new ArrayList<>();
-while (optionBuffer.available() > 0) {
-String name = optionBuffer.getString(charset);
-String data =
GenericUtils.trimToEmpty(optionBuffer.getString(charset));
-list.add(new OpenSshCertificate.CertificateOption(name,
data.length() > 0 ? data : null));
+if (available() > 0) {
+// pull out entire Certificate Options section
+final ByteArrayBuffer optionBuffer = new
ByteArrayBuffer(getBytes());
+
+while (optionBuffer.available() > 0) {
+String name = optionBuffer.getString(charset);
+String data = null;
+final ByteArrayBuffer dataBuffer = new
ByteArrayBuffer(optionBuffer.getBytes());
Review Comment:
@lgoldstein same as previous will apply all changes together
Issue Time Tracking
---
Worklog Id: (was: 769013)
Time Spent: 2h 20m (was: 2h 10m)
> 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: 2h 20m
> 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
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=769011=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769011
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 11/May/22 11:40
Start Date: 11/May/22 11:40
Worklog Time Spent: 10m
Work Description: 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 principals) {
}
public OpenSshCertificateBuilder
criticalOptions(List criticalOptions) {
-this.criticalOptions = criticalOptions;
+if (criticalOptions != null && !criticalOptions.isEmpty()) {
+// check if any duplicates
+Set names = new HashSet();
+Set 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 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:b...@b.com=value -O
extension:a...@a.com=value -O critical:b_critical_option=value -O
critical:a_critical_option=value -V +10d user-key.pub
```
would generate
``` Type: ssh-rsa-cert-...@openssh.com 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)
b...@b.com UNKNOWN OPTION (len 9)
a...@a.com 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?
Issue Time Tracking
---
Worklog Id: (was: 769011)
Time Spent: 2h 10m (was: 2h)
> 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: 2h 10m
> 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
>
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=769010=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769010
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 11/May/22 11:39
Start Date: 11/May/22 11:39
Worklog Time Spent: 10m
Work Description: 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 principals) {
}
public OpenSshCertificateBuilder
criticalOptions(List criticalOptions) {
-this.criticalOptions = criticalOptions;
+if (criticalOptions != null && !criticalOptions.isEmpty()) {
+// check if any duplicates
+Set names = new HashSet();
+Set 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 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:b...@b.com=value -O
extension:a...@a.com=value -O critical:b_critical_option=value -O
critical:a_critical_option=value -V +10d user-key.pub
```
would generate
```Type: ssh-rsa-cert-...@openssh.com 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:24:00 to 2022-05-21T13:25:15
Principals:
user
Critical Options: (none)
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)
b...@b.com UNKNOWN OPTION (len 9)
a...@a.com 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?
Issue Time Tracking
---
Worklog Id: (was: 769010)
Time Spent: 2h (was: 1h 50m)
> 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: 2h
> 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",
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=769008=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769008
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 11/May/22 11:38
Start Date: 11/May/22 11:38
Worklog Time Spent: 10m
Work Description: 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 principals) {
}
public OpenSshCertificateBuilder
criticalOptions(List criticalOptions) {
-this.criticalOptions = criticalOptions;
+if (criticalOptions != null && !criticalOptions.isEmpty()) {
+// check if any duplicates
+Set names = new HashSet();
+Set 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 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:b...@b.com=value -O
extension:a...@a.com=value -O critical:b_critical_option=value -O
critical:a_critical_option=value -V +10d user-key.pub`
would generate
`Type: ssh-rsa-cert-...@openssh.com 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:24:00 to 2022-05-21T13:25:15
Principals:
user
Critical Options: (none)
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)
b...@b.com UNKNOWN OPTION (len 9)
a...@a.com 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?
Issue Time Tracking
---
Worklog Id: (was: 769008)
Time Spent: 1h 50m (was: 1h 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: 1h 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")))
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=768923=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-768923
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 11/May/22 09:23
Start Date: 11/May/22 09:23
Worklog Time Spent: 10m
Work Description: tomaswolf commented on code in PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#discussion_r870072991
##
sshd-core/src/test/java/org/apache/sshd/certificates/GenerateOpenSSHClientCertificateTest.java:
##
@@ -154,7 +150,12 @@ public void signCertificate() throws Exception {
.publicKey(clientPublicKey)
.id("user01")
.principals(Collections.singletonList("user01"))
+.criticalOptions(Arrays.asList(
+new
OpenSshCertificate.CertificateOption("force-command", "/path/to/script.sh"),
+new
OpenSshCertificate.CertificateOption("source-address", "127.0.0.1/32"),
+new
OpenSshCertificate.CertificateOption("verify-required")))
Review Comment:
Just regenerate the certs with OpenSSH and replace the files in
test/resources. The Java code shows how they would need to be regenerated to
match.
Issue Time Tracking
---
Worklog Id: (was: 768923)
Time Spent: 1h 40m (was: 1.5h)
> 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: 1h 40m
> 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
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=768920=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-768920
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 11/May/22 09:19
Start Date: 11/May/22 09:19
Worklog Time Spent: 10m
Work Description: vukzeka commented on code in PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#discussion_r870069420
##
sshd-core/src/test/java/org/apache/sshd/certificates/GenerateOpenSSHClientCertificateTest.java:
##
@@ -154,7 +150,12 @@ public void signCertificate() throws Exception {
.publicKey(clientPublicKey)
.id("user01")
.principals(Collections.singletonList("user01"))
+.criticalOptions(Arrays.asList(
+new
OpenSshCertificate.CertificateOption("force-command", "/path/to/script.sh"),
+new
OpenSshCertificate.CertificateOption("source-address", "127.0.0.1/32"),
+new
OpenSshCertificate.CertificateOption("verify-required")))
Review Comment:
@tomaswolf i didn't add there as would be required to change all existing
certs as message is now different meaning signature and message check will not
pass.
Probably better would be to add separate test for these. I will see later
what would be best and will update
Issue Time Tracking
---
Worklog Id: (was: 768920)
Time Spent: 1.5h (was: 1h 20m)
> 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: 1.5h
> 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
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=768919=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-768919
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 11/May/22 09:18
Start Date: 11/May/22 09:18
Worklog Time Spent: 10m
Work Description: vukzeka commented on code in PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#discussion_r870067540
##
sshd-core/src/test/java/org/apache/sshd/certificates/GenerateOpenSSHClientCertificateTest.java:
##
@@ -154,7 +150,12 @@ public void signCertificate() throws Exception {
.publicKey(clientPublicKey)
.id("user01")
.principals(Collections.singletonList("user01"))
+.criticalOptions(Arrays.asList(
+new
OpenSshCertificate.CertificateOption("force-command", "/path/to/script.sh"),
+new
OpenSshCertificate.CertificateOption("source-address", "127.0.0.1/32"),
+new
OpenSshCertificate.CertificateOption("verify-required")))
.extensions(Arrays.asList(
+new
OpenSshCertificate.CertificateOption("no-touch-required"),
Review Comment:
hi @tomaswolf
Based on specs I believe no-touch-required should be used only with sk keys.
I added it here for completeness of all available critical
options/extensions as in the code **CertificateOptions** are not constants so
we do not have enumeration.
Issue Time Tracking
---
Worklog Id: (was: 768919)
Time Spent: 1h 20m (was: 1h 10m)
> 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: 1h 20m
> 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
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=768918=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-768918
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 11/May/22 09:12
Start Date: 11/May/22 09:12
Worklog Time Spent: 10m
Work Description: vukzeka commented on code in PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#discussion_r870062043
##
sshd-common/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java:
##
@@ -348,29 +348,36 @@ public List
getCertificateOptions() {
/**
* According to https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys#L222-L262;>PROTOCOL.certkeys:
+ *
"https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys#L221-L319;>PROTOCOL.certkeys:
*
* Critical Options is a set of bytes that is
*
- * [overall length][name(string)][data(string)]...
+ * [overall length][name(string)][[length of buffer][[length of
string][data(string)]]]...
*
- * Where each Critical Option is encoded as a name (string) and data
(string)
+ * Where each Certificate Option is encoded as a name (string) and buffer
of data (string packed in a buffer)
*
- * Then the entire name + data strings are added as bytes (which will get
a length prefix)
+ * Then the entire name (string) + data (buffer) are added as bytes (which
will get a length prefix)
*
* @param charset {@link Charset} to use for converting bytes to
characters
* @return the parsed result, never {@code null}, but possibly
empty
*/
public List
getCertificateOptions(Charset charset) {
-// pull out entire Critical Options section
-final ByteArrayBuffer optionBuffer = new ByteArrayBuffer(getBytes());
-
List list = new ArrayList<>();
-while (optionBuffer.available() > 0) {
-String name = optionBuffer.getString(charset);
-String data =
GenericUtils.trimToEmpty(optionBuffer.getString(charset));
-list.add(new OpenSshCertificate.CertificateOption(name,
data.length() > 0 ? data : null));
+if (available() > 0) {
+// pull out entire Certificate Options section
+final ByteArrayBuffer optionBuffer = new
ByteArrayBuffer(getBytes());
Review Comment:
@lgoldstein same as previous comment
Issue Time Tracking
---
Worklog Id: (was: 768918)
Time Spent: 1h 10m (was: 1h)
> 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: 1h 10m
> 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
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=768915=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-768915
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 11/May/22 09:10
Start Date: 11/May/22 09:10
Worklog Time Spent: 10m
Work Description: vukzeka commented on code in PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#discussion_r870060409
##
sshd-common/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java:
##
@@ -348,29 +348,36 @@ public List
getCertificateOptions() {
/**
* According to https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys#L222-L262;>PROTOCOL.certkeys:
+ *
"https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys#L221-L319;>PROTOCOL.certkeys:
*
* Critical Options is a set of bytes that is
*
- * [overall length][name(string)][data(string)]...
+ * [overall length][name(string)][[length of buffer][[length of
string][data(string)]]]...
*
- * Where each Critical Option is encoded as a name (string) and data
(string)
+ * Where each Certificate Option is encoded as a name (string) and buffer
of data (string packed in a buffer)
*
- * Then the entire name + data strings are added as bytes (which will get
a length prefix)
+ * Then the entire name (string) + data (buffer) are added as bytes (which
will get a length prefix)
*
* @param charset {@link Charset} to use for converting bytes to
characters
* @return the parsed result, never {@code null}, but possibly
empty
*/
public List
getCertificateOptions(Charset charset) {
-// pull out entire Critical Options section
-final ByteArrayBuffer optionBuffer = new ByteArrayBuffer(getBytes());
-
List list = new ArrayList<>();
-while (optionBuffer.available() > 0) {
-String name = optionBuffer.getString(charset);
-String data =
GenericUtils.trimToEmpty(optionBuffer.getString(charset));
-list.add(new OpenSshCertificate.CertificateOption(name,
data.length() > 0 ? data : null));
+if (available() > 0) {
+// pull out entire Certificate Options section
+final ByteArrayBuffer optionBuffer = new
ByteArrayBuffer(getBytes());
Review Comment:
hi @lgoldstein
Thanks for taking time for review.
Actually this was copy-paste from existing code, see
https://github.com/apache/mina-sshd/blob/master/sshd-common/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java#L366
In getCertificateOptions I provided it correctly but somehow didn't align
these two.
Issue Time Tracking
---
Worklog Id: (was: 768915)
Time Spent: 1h (was: 50m)
> 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: 1h
> 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
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=768617=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-768617
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 10/May/22 17:48
Start Date: 10/May/22 17:48
Worklog Time Spent: 10m
Work Description: alex-sherwin commented on PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#issuecomment-1122693618
As noted on https://issues.apache.org/jira/browse/SSHD-1266
The original PR's for the OpenSSH certificate support brought in support for
Testcontainers based Docker unit testing
Perhaps this PR should include a new unit test which uses
Testcontainers/Docker to use the real OpenSSH *ssh-keygen* binary to parse the
OpenSSH Certificate output from MINA
Issue Time Tracking
---
Worklog Id: (was: 768617)
Time Spent: 50m (was: 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: 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
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=768603=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-768603
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 10/May/22 17:12
Start Date: 10/May/22 17:12
Worklog Time Spent: 10m
Work Description: tomaswolf commented on PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#issuecomment-1122660236
@lgoldstein : please downgrade then formatter maven plug-in again. (Revert
commit 9f8aa3dba.) It doesn't run with Java 1.8.
Issue Time Tracking
---
Worklog Id: (was: 768603)
Time Spent: 40m (was: 0.5h)
> 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: 40m
> 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)
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=768600=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-768600
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 10/May/22 17:11
Start Date: 10/May/22 17:11
Worklog Time Spent: 10m
Work Description: tomaswolf commented on code in PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#discussion_r869384180
##
sshd-core/src/main/java/org/apache/sshd/certificate/OpenSshCertificateBuilder.java:
##
@@ -104,7 +110,21 @@ public OpenSshCertificateBuilder
principals(Collection principals) {
}
public OpenSshCertificateBuilder
criticalOptions(List criticalOptions) {
-this.criticalOptions = criticalOptions;
+if (criticalOptions != null && !criticalOptions.isEmpty()) {
+// check if any duplicates
+Set names = new HashSet();
+Set 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 sortedCriticalOptions =
criticalOptions.stream()
+
.sorted(Comparator.comparing(OpenSshCertificate.CertificateOption::getName))
+.collect(Collectors.toList());
+this.criticalOptions = sortedCriticalOptions;
+}
Review Comment:
Thanks for adding this; I also noticed that. The same duplicate detection
and sorting is also needed for the extensions.
##
sshd-core/src/test/java/org/apache/sshd/certificates/GenerateOpenSSHClientCertificateTest.java:
##
@@ -154,7 +150,12 @@ public void signCertificate() throws Exception {
.publicKey(clientPublicKey)
.id("user01")
.principals(Collections.singletonList("user01"))
+.criticalOptions(Arrays.asList(
+new
OpenSshCertificate.CertificateOption("force-command", "/path/to/script.sh"),
+new
OpenSshCertificate.CertificateOption("source-address", "127.0.0.1/32"),
+new
OpenSshCertificate.CertificateOption("verify-required")))
Review Comment:
Please also add critical options in the
`GenerateOpenSshClientCertificateOracleTest`. That one tests against
pre-existing certificates generated using OpenSSH.
##
sshd-core/src/test/java/org/apache/sshd/certificates/GenerateOpenSSHClientCertificateTest.java:
##
@@ -154,7 +150,12 @@ public void signCertificate() throws Exception {
.publicKey(clientPublicKey)
.id("user01")
.principals(Collections.singletonList("user01"))
+.criticalOptions(Arrays.asList(
+new
OpenSshCertificate.CertificateOption("force-command", "/path/to/script.sh"),
+new
OpenSshCertificate.CertificateOption("source-address", "127.0.0.1/32"),
+new
OpenSshCertificate.CertificateOption("verify-required")))
.extensions(Arrays.asList(
+new
OpenSshCertificate.CertificateOption("no-touch-required"),
Review Comment:
A bit off-topic, but I wonder: should non-sk keys even accept this option?
Issue Time Tracking
---
Worklog Id: (was: 768600)
Time Spent: 0.5h (was: 20m)
> 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: 0.5h
> 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"))
>
>
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=768565=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-768565
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 10/May/22 16:04
Start Date: 10/May/22 16:04
Worklog Time Spent: 10m
Work Description: lgoldstein commented on code in PR #222:
URL: https://github.com/apache/mina-sshd/pull/222#discussion_r869427309
##
sshd-common/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java:
##
@@ -348,29 +348,36 @@ public List
getCertificateOptions() {
/**
* According to https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys#L222-L262;>PROTOCOL.certkeys:
+ *
"https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys#L221-L319;>PROTOCOL.certkeys:
*
* Critical Options is a set of bytes that is
*
- * [overall length][name(string)][data(string)]...
+ * [overall length][name(string)][[length of buffer][[length of
string][data(string)]]]...
*
- * Where each Critical Option is encoded as a name (string) and data
(string)
+ * Where each Certificate Option is encoded as a name (string) and buffer
of data (string packed in a buffer)
*
- * Then the entire name + data strings are added as bytes (which will get
a length prefix)
+ * Then the entire name (string) + data (buffer) are added as bytes (which
will get a length prefix)
*
* @param charset {@link Charset} to use for converting bytes to
characters
* @return the parsed result, never {@code null}, but possibly
empty
*/
public List
getCertificateOptions(Charset charset) {
-// pull out entire Critical Options section
-final ByteArrayBuffer optionBuffer = new ByteArrayBuffer(getBytes());
-
List list = new ArrayList<>();
-while (optionBuffer.available() > 0) {
-String name = optionBuffer.getString(charset);
-String data =
GenericUtils.trimToEmpty(optionBuffer.getString(charset));
-list.add(new OpenSshCertificate.CertificateOption(name,
data.length() > 0 ? data : null));
+if (available() > 0) {
+// pull out entire Certificate Options section
+final ByteArrayBuffer optionBuffer = new
ByteArrayBuffer(getBytes());
Review Comment:
No need for *final* keyword
##
sshd-common/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java:
##
@@ -348,29 +348,36 @@ public List
getCertificateOptions() {
/**
* According to https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys#L222-L262;>PROTOCOL.certkeys:
+ *
"https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys#L221-L319;>PROTOCOL.certkeys:
*
* Critical Options is a set of bytes that is
*
- * [overall length][name(string)][data(string)]...
+ * [overall length][name(string)][[length of buffer][[length of
string][data(string)]]]...
*
- * Where each Critical Option is encoded as a name (string) and data
(string)
+ * Where each Certificate Option is encoded as a name (string) and buffer
of data (string packed in a buffer)
*
- * Then the entire name + data strings are added as bytes (which will get
a length prefix)
+ * Then the entire name (string) + data (buffer) are added as bytes (which
will get a length prefix)
*
* @param charset {@link Charset} to use for converting bytes to
characters
* @return the parsed result, never {@code null}, but possibly
empty
*/
public List
getCertificateOptions(Charset charset) {
-// pull out entire Critical Options section
-final ByteArrayBuffer optionBuffer = new ByteArrayBuffer(getBytes());
-
List list = new ArrayList<>();
-while (optionBuffer.available() > 0) {
-String name = optionBuffer.getString(charset);
-String data =
GenericUtils.trimToEmpty(optionBuffer.getString(charset));
-list.add(new OpenSshCertificate.CertificateOption(name,
data.length() > 0 ? data : null));
+if (available() > 0) {
+// pull out entire Certificate Options section
+final ByteArrayBuffer optionBuffer = new
ByteArrayBuffer(getBytes());
Review Comment:
Why can't it be `Buffer optionBuffer = new ...` - are you using any
*ByteArrayBuffer* methods that are not available via the *Buffer* class ?
Hint: recommend to use the "weakest" variable type you need and not what you
know its full type is.
##
sshd-common/src/main/java/org/apache/sshd/common/util/buffer/Buffer.java:
##
@@ -348,29 +348,36 @@ public List
getCertificateOptions() {
/**
* According to
[jira] [Work logged] (SSHD-1266) OpenSSH certificate is not properly encoded when critical options are included
[
https://issues.apache.org/jira/browse/SSHD-1266?focusedWorklogId=768549=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-768549
]
ASF GitHub Bot logged work on SSHD-1266:
Author: ASF GitHub Bot
Created on: 10/May/22 15:13
Start Date: 10/May/22 15:13
Worklog Time Spent: 10m
Work Description: vukzeka opened a new pull request, #222:
URL: https://github.com/apache/mina-sshd/pull/222
- fixed bug with CertificateOption data encoding
- Improved CertificateOption docs
- Added critical option ordering and duplicate check
- updated tests to include critical options
Issue Time Tracking
---
Worklog Id: (was: 768549)
Remaining Estimate: 0h
Time Spent: 10m
> 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: Critical
> Time Spent: 10m
> 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
