Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply upstream refactor to sshkey.c #160

Merged
merged 1 commit into from
Jul 22, 2024

Conversation

geedo0
Copy link

@geedo0 geedo0 commented Jul 15, 2024

In PR #159 we resolved a massive merge conflict in sshkey.c by simply taking the upstream version and removing all PQ SSH key support from the trunk. This broke the build/tests and obviously the ability to use PQ SSH keys. This PR restores PQ SSH key support for "pure" PQ algorithms. A follow-up PR will bring back support for hybrid-PQ. This was done to break up the PRs and do things more incrementally.

At a high-level, there was a major refactor of sshkey upstream which moved algorithm implementations to an OOP abstraction via the sshkey_impl struct. This allowed them to get rid of all the switch statements, encapsulate algorithm specific code, and standardize on a single factory method to get the right implementation. Some other minor things like the function signatures for sign/verify were updated to accept new optional arguments.

Getting OQS over to this new system was a matter of defining these sshkey_impl structs and factoring out the relevant OQS bits to the proper functions. This involved a number of changes/renames/deletions to the templates. I also performed a 3-way merge to pull-in changes that weren't directly impacted by the refactor.

There is no support for hybrid SSH Key support for now. This is to make sure that the baseline refactor is in a good shape before committing to a certain path. To support hybrid, I propose implementing generic hybrid versions of these classes. The only tricky thing is figuring out how to expose the RSA/EC implementations to the ssh-oqs file (probably declaring externs?). I considered doing this "on the outside", but that seems to go against what the refactor aimed to do and adds a lot more complexity to the sshkey source than seems necessary.

What's working now?

  • build_openssh.sh now compiles and installs successfully 🎉
  • CI passes as long as tests for the missing hybrid SSH key support is skipped.
  • Upstream OpenSSH CI and make tests target fail, but these are to be pre-existing defects.

Related to Issue #135

In PR open-quantum-safe#159 we resolved a massive merge conflict in `sshkey.c` by simply taking the upstream version and removing all PQ SSH key support from the trunk. This broke the build/tests and obviously the ability to use PQ SSH keys. This PR restores PQ SSH key support for "pure" PQ algorithms. A follow-up PR will bring back support for hybrid-PQ. This was done to break up the PRs and do things more incrementally.

At a high-level, there was a major refactor of `sshkey` upstream which moved algorithm implementations to an OOP abstraction via the `sshkey_impl` struct. This allowed them to get rid of all the switch statements, encapsulate algorithm specific code, and standardize on a single factory method to get the right implementation. Some other minor things like the function signatures for sign/verify were updated to accept new optional arguments.

Getting OQS over to this new system was a matter of defining these `sshkey_impl` structs and factoring out the relevant OQS bits to the proper functions. This involved a number of changes/renames/deletions to the templates. I also performed a 3-way merge to pull-in changes that weren't directly impacted by the refactor.

There is no support for hybrid SSH Key support for now. This is to make sure that the baseline refactor is in a good shape before committing to a certain path. To support hybrid, I propose implementing generic hybrid versions of these classes. The only tricky thing is figuring out how to expose the RSA/EC implementations to the `ssh-oqs` file (probably declaring externs?). I considered doing this "on the outside", but that seems to go against what the refactor aimed to do and adds a lot more complexity to the sshkey source than seems necessary.

What's working now?
- `build_openssh.sh` now compiles and installs successfully 🎉
- `make tests` runs through a lot of tests before failing now. This is my next line of inquiry to chase down.

Signed-off-by: Gerardo Ravago <[email protected]>
@geedo0 geedo0 marked this pull request as ready for review July 15, 2024 18:21
@geedo0
Copy link
Author

geedo0 commented Jul 22, 2024

Can we merge the PR (I don't have permissions to do this) or are we waiting on more feedback or testing? Squash merge is fine/preferred here since this isn't an upstream merge. I have a number of other changes that depend on this one and can't get the PR's submitted until this one is merged.

@dstebila
Copy link
Member

Can we merge the PR (I don't have permissions to do this) or are we waiting on more feedback or testing? Squash merge is fine/preferred here since this isn't an upstream merge. I have a number of other changes that depend on this one and can't get the PR's submitted until this one is merged.

Was just hoping to get one more set of eyes on this before merging.

Copy link
Member

@baentsch baentsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest merging as-is even though there are rough edges (#ifdef close comments not merging starts, say)--with the understanding this work will move to functional equivalence with the 8.x variant--as discussed, ideally with some interop test between old and new such as to make acceptance of this substantial piece of work reasonably doable without diving into all details.

@dstebila dstebila merged commit 1173ecc into open-quantum-safe:OQS-v9 Jul 22, 2024
3 checks passed
@geedo0
Copy link
Author

geedo0 commented Jul 22, 2024

I'd suggest merging as-is even though there are rough edges (#ifdef close comments not merging starts, say)--with the understanding this work will move to functional equivalence with the 8.x variant--as discussed, ideally with some interop test between old and new such as to make acceptance of this substantial piece of work reasonably doable without diving into all details.

My plan is to present more in depth validation as this gets closer to done (e.g. the next PR on hybrid SSH Keys is a better checkpoint for that). I wanted to avoid some hassle on temporarily working around things when we can tell a much cleaner story around validation with a complete implementation. Hope that's alright and makes sense.

geedo0 added a commit to geedo0/openssh that referenced this pull request Jul 23, 2024
This finishes the work in PR open-quantum-safe#160 which applied the upstream `sshkey.c` refactor to the OQS fork by adding support for hybrid SSH keys. More importantly, this brings the `OQS-v9` branch up to parity with `OQS-v8` in terms of supported algorithms and functionality. Therefore, we can do more in depth and thorough validation to increase confidence in cutting over to this newer branch.

Speaking to the code changes for hybrid SSH key support, this works by adding logic to `ssh-oqs` which branches on hybrid SSH key implementations to handle the classical portion of the key and combine it with the PQ portion as-appropriate. The main trick is to introduce a small lookup table for the RSA/ECDSA implementation and exposing the symbols to `ssh-oqs` via an extern declaration. One notable oddity is that upstream OpenSSH multiplexes the underlying EC curves by placing a generic implementation behind the P-256 struct and allowing the implementation to fork based on the `bits` or `key->type` parameters. Depending on the context, this is how `sshkey` does things so I followed their convention.

Related to issue open-quantum-safe#135

Asserted that Circle CI jobs pass. These tests run through a subset of the OpenSSH unit tests that have been documented to pass against the OQS fork and skip tests that depend on missing/broken functionality. This demonstrates internal consistency and parity with the testing bar set by `OQS-v8`.

Performed interop testing between `OQS-v8` and `OQS-v9` to assert that we have no regressions from pulling in 2 years of upstream changes and re-implementing PQ+Hybrid SSH Keys. This was done by modifying `try_connection.py` which tests all PQ+Hybrid signatures and key exchanges by connecting the built SSH client to the SSHD server and explicitly specifying each algorithm. By adding CLI flags to override this test to use an SSH or SSHD binary from somewhere else, we can perform thorough interop testing between an `OQS-v8` server and `OQS-v9` client or vice versa. Detailed process/commands outlined below.

```
git clone [email protected]:open-quantum-safe/openssh.git oqs-openssh-clean
cd oqs-openssh-clean
git checkout OQS-v8
./oqs-scripts/clone_liboqs.sh
./oqs-scripts/build_liboqs.sh
./oqs-scripts/build_openssh.sh

python3 oqs-test/try_connection.py --sshd `readlink -f ../oqs-openssh-clean/sshd` doall
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-rsa3072-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp256-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp521-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-dilithium2.
...

python3 oqs-test/try_connection.py --ssh `readlink -f ../oqs-openssh-clean/ssh` doall
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-rsa3072-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp256-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp521-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-dilithium2.
...
```
@geedo0 geedo0 mentioned this pull request Jul 23, 2024
geedo0 added a commit to geedo0/openssh that referenced this pull request Jul 23, 2024
This finishes the work in PR open-quantum-safe#160 which applied the upstream `sshkey.c` refactor to the OQS fork by adding support for hybrid SSH keys. More importantly, this brings the `OQS-v9` branch up to parity with `OQS-v8` in terms of supported algorithms and functionality. Therefore, we can do more in depth and thorough validation to increase confidence in cutting over to this newer branch.

Speaking to the code changes for hybrid SSH key support, this works by adding logic to `ssh-oqs` which branches on hybrid SSH key implementations to handle the classical portion of the key and combine it with the PQ portion as-appropriate. The main trick is to introduce a small lookup table for the RSA/ECDSA implementation and exposing the symbols to `ssh-oqs` via an extern declaration. One notable oddity is that upstream OpenSSH multiplexes the underlying EC curves by placing a generic implementation behind the P-256 struct and allowing the implementation to fork based on the `bits` or `key->type` parameters. Depending on the context, this is how `sshkey` does things so I followed their convention.

Related to issue open-quantum-safe#135

Asserted that Circle CI jobs pass. These tests run through a subset of the OpenSSH unit tests that have been documented to pass against the OQS fork and skip tests that depend on missing/broken functionality. This demonstrates internal consistency and parity with the testing bar set by `OQS-v8`.

Performed interop testing between `OQS-v8` and `OQS-v9` to assert that we have no regressions from pulling in 2 years of upstream changes and re-implementing PQ+Hybrid SSH Keys. This was done by modifying `try_connection.py` which tests all PQ+Hybrid signatures and key exchanges by connecting the built SSH client to the SSHD server and explicitly specifying each algorithm. By adding CLI flags to override this test to use an SSH or SSHD binary from somewhere else, we can perform thorough interop testing between an `OQS-v8` server and `OQS-v9` client or vice versa. Detailed process/commands outlined below.

```
git clone [email protected]:open-quantum-safe/openssh.git oqs-openssh-clean
cd oqs-openssh-clean
git checkout OQS-v8
./oqs-scripts/clone_liboqs.sh
./oqs-scripts/build_liboqs.sh
./oqs-scripts/build_openssh.sh

python3 oqs-test/try_connection.py --sshd `readlink -f ../oqs-openssh-clean/sshd` doall
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-rsa3072-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp256-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp521-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-dilithium2.
...

python3 oqs-test/try_connection.py --ssh `readlink -f ../oqs-openssh-clean/ssh` doall
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-rsa3072-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp256-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp521-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-dilithium2.
...
```

Signed-off-by: Gerardo Ravago <[email protected]>
geedo0 added a commit to geedo0/openssh that referenced this pull request Jul 23, 2024
This finishes the work in PR open-quantum-safe#160 which applied the upstream `sshkey.c` refactor to the OQS fork by adding support for hybrid SSH keys. More importantly, this brings the `OQS-v9` branch up to parity with `OQS-v8` in terms of supported algorithms and functionality. Therefore, we can do more in depth and thorough validation to increase confidence in cutting over to this newer branch.

Speaking to the code changes for hybrid SSH key support, this works by adding logic to `ssh-oqs` which branches on hybrid SSH key implementations to handle the classical portion of the key and combine it with the PQ portion as-appropriate. The main trick is to introduce a small lookup table for the RSA/ECDSA implementation and exposing the symbols to `ssh-oqs` via an extern declaration. One notable oddity is that upstream OpenSSH multiplexes the underlying EC curves by placing a generic implementation behind the P-256 struct and allowing the implementation to fork based on the `bits` or `key->type` parameters. Depending on the context, this is how `sshkey` does things so I followed their convention.

Related to issue open-quantum-safe#135

Asserted that Circle CI jobs pass. These tests run through a subset of the OpenSSH unit tests that have been documented to pass against the OQS fork and skip tests that depend on missing/broken functionality. This demonstrates internal consistency and parity with the testing bar set by `OQS-v8`.

Performed interop testing between `OQS-v8` and `OQS-v9` to assert that we have no regressions from pulling in 2 years of upstream changes and re-implementing PQ+Hybrid SSH Keys. This was done by modifying `try_connection.py` which tests all PQ+Hybrid signatures and key exchanges by connecting the built SSH client to the SSHD server and explicitly specifying each algorithm. By adding CLI flags to override this test to use an SSH or SSHD binary from somewhere else, we can perform thorough interop testing between an `OQS-v8` server and `OQS-v9` client or vice versa. Detailed process/commands outlined below.

```
git clone [email protected]:open-quantum-safe/openssh.git oqs-openssh-clean
cd oqs-openssh-clean
git checkout OQS-v8
./oqs-scripts/clone_liboqs.sh
./oqs-scripts/build_liboqs.sh
./oqs-scripts/build_openssh.sh

python3 oqs-test/try_connection.py --sshd `readlink -f ../oqs-openssh-clean/sshd` doall
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-rsa3072-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp256-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp521-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-dilithium2.
...

python3 oqs-test/try_connection.py --ssh `readlink -f ../oqs-openssh-clean/ssh` doall
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-rsa3072-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp256-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp521-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-dilithium2.
...
```
geedo0 added a commit to geedo0/openssh that referenced this pull request Jul 23, 2024
This finishes the work in PR open-quantum-safe#160 which applied the upstream `sshkey.c` refactor to the OQS fork by adding support for hybrid SSH keys. More importantly, this brings the `OQS-v9` branch up to parity with `OQS-v8` in terms of supported algorithms and functionality. Therefore, we can do more in depth and thorough validation to increase confidence in cutting over to this newer branch.

Speaking to the code changes for hybrid SSH key support, this works by adding logic to `ssh-oqs` which branches on hybrid SSH key implementations to handle the classical portion of the key and combine it with the PQ portion as-appropriate. The main trick is to introduce a small lookup table for the RSA/ECDSA implementation and exposing the symbols to `ssh-oqs` via an extern declaration. One notable oddity is that upstream OpenSSH multiplexes the underlying EC curves by placing a generic implementation behind the P-256 struct and allowing the implementation to fork based on the `bits` or `key->type` parameters. Depending on the context, this is how `sshkey` does things so I followed their convention.

Related to issue open-quantum-safe#135

Asserted that Circle CI jobs pass. These tests run through a subset of the OpenSSH unit tests that have been documented to pass against the OQS fork and skip tests that depend on missing/broken functionality. This demonstrates internal consistency and parity with the testing bar set by `OQS-v8`.

Performed interop testing between `OQS-v8` and `OQS-v9` to assert that we have no regressions from pulling in 2 years of upstream changes and re-implementing PQ+Hybrid SSH Keys. This was done by modifying `try_connection.py` which tests all PQ+Hybrid signatures and key exchanges by connecting the built SSH client to the SSHD server and explicitly specifying each algorithm. By adding CLI flags to override this test to use an SSH or SSHD binary from somewhere else, we can perform thorough interop testing between an `OQS-v8` server and `OQS-v9` client or vice versa. Detailed process/commands outlined below.

```
git clone [email protected]:open-quantum-safe/openssh.git oqs-openssh-clean
cd oqs-openssh-clean
git checkout OQS-v8
./oqs-scripts/clone_liboqs.sh
./oqs-scripts/build_liboqs.sh
./oqs-scripts/build_openssh.sh

python3 oqs-test/try_connection.py --sshd `readlink -f ../oqs-openssh-clean/sshd` doall
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-rsa3072-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp256-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp521-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-dilithium2.
...

python3 oqs-test/try_connection.py --ssh `readlink -f ../oqs-openssh-clean/ssh` doall
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-rsa3072-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp256-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp521-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-dilithium2.
...
```

Signed-off-by: Gerardo Ravago <[email protected]>
geedo0 added a commit to geedo0/openssh that referenced this pull request Aug 6, 2024
This finishes the work in PR open-quantum-safe#160 which applied the upstream `sshkey.c` refactor to the OQS fork by adding support for hybrid SSH keys. More importantly, this brings the `OQS-v9` branch up to parity with `OQS-v8` in terms of supported algorithms and functionality. Therefore, we can do more in depth and thorough validation to increase confidence in cutting over to this newer branch.

Speaking to the code changes for hybrid SSH key support, this works by adding logic to `ssh-oqs` which branches on hybrid SSH key implementations to handle the classical portion of the key and combine it with the PQ portion as-appropriate. The main trick is to introduce a small lookup table for the RSA/ECDSA implementation and exposing the symbols to `ssh-oqs` via an extern declaration. One notable oddity is that upstream OpenSSH multiplexes the underlying EC curves by placing a generic implementation behind the P-256 struct and allowing the implementation to fork based on the `bits` or `key->type` parameters. Depending on the context, this is how `sshkey` does things so I followed their convention.

Related to issue open-quantum-safe#135

Asserted that Circle CI jobs pass. These tests run through a subset of the OpenSSH unit tests that have been documented to pass against the OQS fork and skip tests that depend on missing/broken functionality. This demonstrates internal consistency and parity with the testing bar set by `OQS-v8`.

Performed interop testing between `OQS-v8` and `OQS-v9` to assert that we have no regressions from pulling in 2 years of upstream changes and re-implementing PQ+Hybrid SSH Keys. This was done by modifying `try_connection.py` which tests all PQ+Hybrid signatures and key exchanges by connecting the built SSH client to the SSHD server and explicitly specifying each algorithm. By adding CLI flags to override this test to use an SSH or SSHD binary from somewhere else, we can perform thorough interop testing between an `OQS-v8` server and `OQS-v9` client or vice versa. Detailed process/commands outlined below.

```
git clone [email protected]:open-quantum-safe/openssh.git oqs-openssh-clean
cd oqs-openssh-clean
git checkout OQS-v8
./oqs-scripts/clone_liboqs.sh
./oqs-scripts/build_liboqs.sh
./oqs-scripts/build_openssh.sh

python3 oqs-test/try_connection.py --sshd `readlink -f ../oqs-openssh-clean/sshd` doall
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-rsa3072-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp256-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp521-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-dilithium2.
...

python3 oqs-test/try_connection.py --ssh `readlink -f ../oqs-openssh-clean/ssh` doall
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-rsa3072-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp256-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp521-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-dilithium2.
...
```

Signed-off-by: Gerardo Ravago <[email protected]>
geedo0 added a commit to geedo0/openssh that referenced this pull request Aug 6, 2024
This finishes the work in PR open-quantum-safe#160 which applied the upstream `sshkey.c` refactor to the OQS fork by adding support for hybrid SSH keys. More importantly, this brings the `OQS-v9` branch up to parity with `OQS-v8` in terms of supported algorithms and functionality. Therefore, we can do more in depth and thorough validation to increase confidence in cutting over to this newer branch.

Speaking to the code changes for hybrid SSH key support, this works by adding logic to `ssh-oqs` which branches on hybrid SSH key implementations to handle the classical portion of the key and combine it with the PQ portion as-appropriate. The main trick is to introduce a small lookup table for the RSA/ECDSA implementation and exposing the symbols to `ssh-oqs` via an extern declaration. One notable oddity is that upstream OpenSSH multiplexes the underlying EC curves by placing a generic implementation behind the P-256 struct and allowing the implementation to fork based on the `bits` or `key->type` parameters. Depending on the context, this is how `sshkey` does things so I followed their convention.

Related to issue open-quantum-safe#135

Asserted that Circle CI jobs pass. These tests run through a subset of the OpenSSH unit tests that have been documented to pass against the OQS fork and skip tests that depend on missing/broken functionality. This demonstrates internal consistency and parity with the testing bar set by `OQS-v8`.

Performed interop testing between `OQS-v8` and `OQS-v9` to assert that we have no regressions from pulling in 2 years of upstream changes and re-implementing PQ+Hybrid SSH Keys. This was done by modifying `try_connection.py` which tests all PQ+Hybrid signatures and key exchanges by connecting the built SSH client to the SSHD server and explicitly specifying each algorithm. By adding CLI flags to override this test to use an SSH or SSHD binary from somewhere else, we can perform thorough interop testing between an `OQS-v8` server and `OQS-v9` client or vice versa. Detailed process/commands outlined below.

```
git clone [email protected]:open-quantum-safe/openssh.git oqs-openssh-clean
cd oqs-openssh-clean
git checkout OQS-v8
./oqs-scripts/clone_liboqs.sh
./oqs-scripts/build_liboqs.sh
./oqs-scripts/build_openssh.sh

python3 oqs-test/try_connection.py --sshd `readlink -f ../oqs-openssh-clean/sshd` doall
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-rsa3072-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp256-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp521-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-dilithium2.
...

python3 oqs-test/try_connection.py --ssh `readlink -f ../oqs-openssh-clean/ssh` doall
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-rsa3072-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp256-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp521-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-dilithium2.
...
```

Signed-off-by: Gerardo Ravago <[email protected]>
geedo0 added a commit to geedo0/openssh that referenced this pull request Aug 6, 2024
This finishes the work in PR open-quantum-safe#160 which applied the upstream `sshkey.c` refactor to the OQS fork by adding support for hybrid SSH keys. More importantly, this brings the `OQS-v9` branch up to parity with `OQS-v8` in terms of supported algorithms and functionality. Therefore, we can do more in depth and thorough validation to increase confidence in cutting over to this newer branch.

Speaking to the code changes for hybrid SSH key support, this works by adding logic to `ssh-oqs` which branches on hybrid SSH key implementations to handle the classical portion of the key and combine it with the PQ portion as-appropriate. The main trick is to introduce a small lookup table for the RSA/ECDSA implementation and exposing the symbols to `ssh-oqs` via an extern declaration. One notable oddity is that upstream OpenSSH multiplexes the underlying EC curves by placing a generic implementation behind the P-256 struct and allowing the implementation to fork based on the `bits` or `key->type` parameters. Depending on the context, this is how `sshkey` does things so I followed their convention.

Related to issue open-quantum-safe#135

Asserted that Circle CI jobs pass. These tests run through a subset of the OpenSSH unit tests that have been documented to pass against the OQS fork and skip tests that depend on missing/broken functionality. This demonstrates internal consistency and parity with the testing bar set by `OQS-v8`.

Performed interop testing between `OQS-v8` and `OQS-v9` to assert that we have no regressions from pulling in 2 years of upstream changes and re-implementing PQ+Hybrid SSH Keys. This was done by modifying `try_connection.py` which tests all PQ+Hybrid signatures and key exchanges by connecting the built SSH client to the SSHD server and explicitly specifying each algorithm. By adding CLI flags to override this test to use an SSH or SSHD binary from somewhere else, we can perform thorough interop testing between an `OQS-v8` server and `OQS-v9` client or vice versa. Detailed process/commands outlined below.

```
git clone [email protected]:open-quantum-safe/openssh.git oqs-openssh-clean
cd oqs-openssh-clean
git checkout OQS-v8
./oqs-scripts/clone_liboqs.sh
./oqs-scripts/build_liboqs.sh
./oqs-scripts/build_openssh.sh

python3 oqs-test/try_connection.py --sshd `readlink -f ../oqs-openssh-clean/sshd` doall
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-rsa3072-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp256-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp521-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-dilithium2.
...

python3 oqs-test/try_connection.py --ssh `readlink -f ../oqs-openssh-clean/ssh` doall
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-rsa3072-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp256-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp521-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-dilithium2.
...
```

Signed-off-by: gcr <[email protected]>
geedo0 added a commit that referenced this pull request Aug 13, 2024
This finishes the work in PR #160 which applied the upstream `sshkey.c` refactor to the OQS fork by adding support for hybrid SSH keys. More importantly, this brings the `OQS-v9` branch up to parity with `OQS-v8` in terms of supported algorithms and functionality. Therefore, we can do more in depth and thorough validation to increase confidence in cutting over to this newer branch.

Speaking to the code changes for hybrid SSH key support, this works by adding logic to `ssh-oqs` which branches on hybrid SSH key implementations to handle the classical portion of the key and combine it with the PQ portion as-appropriate. The main trick is to introduce a small lookup table for the RSA/ECDSA implementation and exposing the symbols to `ssh-oqs` via an extern declaration. One notable oddity is that upstream OpenSSH multiplexes the underlying EC curves by placing a generic implementation behind the P-256 struct and allowing the implementation to fork based on the `bits` or `key->type` parameters. Depending on the context, this is how `sshkey` does things so I followed their convention.

Related to issue #135

Asserted that Circle CI jobs pass. These tests run through a subset of the OpenSSH unit tests that have been documented to pass against the OQS fork and skip tests that depend on missing/broken functionality. This demonstrates internal consistency and parity with the testing bar set by `OQS-v8`.

Performed interop testing between `OQS-v8` and `OQS-v9` to assert that we have no regressions from pulling in 2 years of upstream changes and re-implementing PQ+Hybrid SSH Keys. This was done by modifying `try_connection.py` which tests all PQ+Hybrid signatures and key exchanges by connecting the built SSH client to the SSHD server and explicitly specifying each algorithm. By adding CLI flags to override this test to use an SSH or SSHD binary from somewhere else, we can perform thorough interop testing between an `OQS-v8` server and `OQS-v9` client or vice versa. Detailed process/commands outlined below.

```
git clone [email protected]:open-quantum-safe/openssh.git oqs-openssh-clean
cd oqs-openssh-clean
git checkout OQS-v8
./oqs-scripts/clone_liboqs.sh
./oqs-scripts/build_liboqs.sh
./oqs-scripts/build_openssh.sh

python3 oqs-test/try_connection.py --sshd `readlink -f ../oqs-openssh-clean/sshd` doall
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-rsa3072-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp256-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp521-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-dilithium2.
...

python3 oqs-test/try_connection.py --ssh `readlink -f ../oqs-openssh-clean/ssh` doall
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-rsa3072-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp256-falcon512.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-ecdsa-nistp521-falcon1024.
Success! Key Exchange Algorithm: frodokem-640-aes-sha256. Signature Algorithm: ssh-dilithium2.
...
```

Signed-off-by: gcr <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants