tomaswolf commented on issue #928: URL: https://github.com/apache/mina-sshd/issues/928#issuecomment-6234762719
The container image builds fine. - The `"$@"` in the CMD is a nice touch enabling passing additional options to the `sshd` inside the container. Given that we can, if needed, override LogLevel from outside it would be OK to leave is by default at VERBOSE in sshd_config. - I would also pin the liboqs version. A simple way of doing so is to check out the expected commit by SHA1 (5a1a854b0dc9f2141bdc771c555ee60c37950183) after having cloned. (Works even with depth 1 since it should be the commit that was fetched.) The pure ML-KEM implementation I had prepared interoperates fine with this oqs-openssh. The `OqsOpenSshInteropTest` could be improved a little by making `pureMlKem()` a parameterized test and testing all three KEXes (mlkem512-sha256, mlkem768-sha256, and mlkem1024-sha384). Testing the hybrid is perhaps not really needed; the existing `OpenSshMlKemTest` tests that already against stock OpenSSH. Since this gives us a container with an OpenSSH version that does pure ML-KEM and pure ML-DSA I would change my suggested approach: do the pure ML-DSA first, hybrid later. I had only suggested to start with the hybrid because I knew already how to do interoperability tests. But that's no longer a valid reason. Do you want to prepare a PR for getting this container image in? One commit with the stuff in `./docker/oqs-openssh` and the changes to the GH workflows. (Locally I added also `sshd_config text eol=lf` to .gitattributes; it helps if someone want to build this image locally and happens to be on Windows.) Final code in PRs should not contain comments that read like AI instructions or "notes to self" from the AI. (Like [this](https://github.com/etnt/mina-sshd/blob/a523aae7102327577a65056801d5bc2754d3e136/sshd-test/src/test/java/org/apache/sshd/client/kex/OqsOpenSshInteropTest.java#L69).) (A new release 3.0.0-M7 is currently on its way; I'd merge that PR once that release is out.) Once we have that container built and pushed, we could in a second PR/commit add the test class, and then in a third commit I'd push the pure ML-KEM implementation. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
