Skip to content

Upgrade to OpenSSH v10.0#179

Merged
dstebila merged 490 commits intoopen-quantum-safe:OQS-v9from
andrewyounkers:feature/openssh-10.0.P2-uplift
Sep 9, 2025
Merged

Upgrade to OpenSSH v10.0#179
dstebila merged 490 commits intoopen-quantum-safe:OQS-v9from
andrewyounkers:feature/openssh-10.0.P2-uplift

Conversation

@andrewyounkers
Copy link

This is an attempt to follow the process taken to upgrade OpenSSH to v9.7 (PR #159) and merge in all the commits between the tip of OQS-v9 and the V_10_0_P2 tag in upstream OpenSSH.

To take in these commits, I followed the instructions for merging upstream code and executed git merge V_10_0_P2 in my feature branch. When resolving the conflicts resulting from the merge I followed psuedo-algorithm defined in the 9.7 uplift to make a first pass.

Merge conflict callouts:

  • kex.c was refactored as the kex-names.c file was introduced. I resolved this by accepting the incoming changes and removing the functions that were moved to kex-names.c. In kex-names.c the OQS_TEMPLATE_FRAGMENT_ADD_KEX_ALGS logic needed to be added in the same way as in kex.c to the new location. Lastly, upstream references to KEX_NTRUPRIME_SNTRUP761_SHA512 and KEX_ML_KEM_768_SHA256 were removed to maintain consistency with previous behavior in the repo.
  • sshd.c was refactored as two new files were introduced: sshd-session.c (9_8_P1) and sshd-auth.c(10_0_P1). To resolve this conflict I took in the incoming changes since it didn't seem to modify any OQS behavior. During the 9.8 uplift the sshd-session.c needed changes in the function list_hostkey_types needed to have the OQS specific cases added back, along with the logic for OQS_TEMPLATE_FRAGMENT_POINT_TO_KEX_GEN. During the 10.0 uplift, this behavior was moved to the sshd-auth.c file.
  • In OpenSSH v9.9, the upstream added support for MLKEM768X25519_SHA256. The following files had conflicts due to these upstream changes: kex.h, test_kex.c, sshconnect2.c, ssh-keyscan.c, monitor.c, ssh_api.c, and kexgen.c. In all of these files the resolution was to keep the OQS changes. This removed the references to the upstreams mlkem768 and was done to keep parity with how the upstreams sntrup761 changes were handled in oqs-ssh. In defines.h USE_MLKEM768X25519 was set to 0.
  • ssh-ecdsa.c and ssh-rsa.c had changes in behavior where the resolution was to keep a combination of the changes. This allowed the new behavior to be taken in and kept the calls to oqs_utils_is_ALG_hybrid.
  • For the remaining files the resolution was to simply take the incoming changes: servconf.c, misc.c, and readconf.c. Files README.md, my-proposal.h, authfile.c , and .gitignore the resolution was to keep the OQS behavior.
  • .git_allowed_signers removed for the reasons explained in the 9.7 uplift PR

Other changes:

  • generate.py received updates along with adding in the necessary fragment files. This updates the automation for adding new algorithms to look in the kex-names.c and sshd-auth.c files now, and removes references to sshd.c. The fragment for updating the Readme was modified to prevent overwriting the finalized mlkem codepoints with the incorrect format.
  • configure and config.h.in files were added by the upstream during this uplift. To maintain consistency with the current behavior of oqs-ssh to rely on using autoconf, these files were removed.

To test these changes were working properly I tested by running build_openssh.sh and fixing compiler errors that were present. Once I was able to successfully run that script, I executed the tests located at oqs-test/run_tests.sh.

I encountered a few testing failures during the 9.8 portion of the uplift, where the OQS logic had not been added to the new files kex-names.c and sshd-session.c after the merge. Adding back the OQS changes to the logic that was moved fixed these failures, and then I used the same strategy when handling the 10.0 commits for the sshd-auth.c file. All testing ran by run_tests.sh is presently passing.

djmdjm and others added 30 commits September 9, 2024 16:46
The previous commit was incorrect (or at least insufficient), the
ML-KEM code is actually using compound literals, so test for them.
I can't find a reliable way to detect the features the ML-KEM code
requires in configure. Give up for now and use VLA support (that we
can detect) as a proxy for "old compiler" and turn off ML-KEM if
it isn't supported.
used for C89 compilers
OpenBSD-Commit-ID: fa18dccdd9753dd287e62ecab189b3de45672521
OpenSSH 9.8, which incorrectly required that sshd was started with an
absolute path in inetd mode. bz3717, patch from Colin Wilson

OpenBSD-Commit-ID: 25c57f22764897242d942853f8cccc5e991ea058
In Fedora systems, %{?rhel} is empty. In RHEL systems, %{?fedora} is
empty. Therefore, the original code always sets without_openssl to 1.
verification fails. Prevents restrictive key options being incorrectly
applied to subsequent keys in authorized_keys. bz3733, ok markus@

OpenBSD-Commit-ID: ba3776d9da4642443c19dbc015a1333622eb5a4e
prompts. Helps the user know what's going on when ssh-keygen is invoked via
other tools. Requested in GHPR503

OpenBSD-Commit-ID: 613b0bb6cf845b7e787d69a5b314057ceda6a8b6
string tokeniser, making it possible to use shell-like quoting in Match
directives, particularly "Match exec". ok markus@

OpenBSD-Commit-ID: 0877309650b76f624b2194c35dbacaf065e769a5
too; ok markus@

OpenBSD-Commit-ID: b74b5b0385f2e0379670e2b869318a65b0bc3923
If set, this will terminate the connection at the first authentication
request (this is the earliest we can evaluate sshd_config Match blocks)

ok markus@

OpenBSD-Commit-ID: 43cc2533984074c44d0d2f92eb93f661e7a0b09c
PerSourcePenalties

This allows penalising connection sources that have had connections
dropped by the RefuseConnection option. ok markus@

OpenBSD-Commit-ID: 3c8443c427470bb3eac1880aa075cb4864463cb6
options.

This allows writing Match conditions that trigger for invalid username.
E.g.

PerSourcePenalties refuseconnection:90s
Match invalid-user
 RefuseConnection yes

Will effectively penalise bots try to guess passwords for bogus accounts,
at the cost of implicitly revealing which accounts are invalid.

feedback markus@

OpenBSD-Commit-ID: 93d3a46ca04bbd9d84a94d1e1d9d3a21073fbb07
OpenBSD-Commit-ID: 2c84a9b517283e9711e2812c1f268081dcb02081
implementation in SUPERCOP 20201130 to the "compact" implementation in
SUPERCOP 20240808. The new version is substantially faster. Thanks to Daniel
J Bernstein for pointing out the new implementation (and of course for
writing it).

tested in snaps/ok deraadt@

OpenBSD-Commit-ID: bf1a77924c125ecdbf03e2f3df8ad13bd3dafdcb
Simpler and removes some code with the old-style BSD license.
OpenBSD-Commit-ID: d899c13b0e8061d209298eaf58fe53e3643e967c
OpenBSD-Commit-ID: 1c81f37b138b8b66abba811fec836388a0f3e6da
relies on using -fwrapv to provide defined over/underflow behaviour, but we
use -ftrapv to catch integer errors and abort the program. ok dtucker@

OpenBSD-Commit-ID: 8933369b33c17b5f02479503d0a92d87bc3a574b
key values need to be static to persist across invocations;
spotted by the Qualys Security Advisory team.
OpenBSD-Commit-ID: 303417285f1a73b9cb7a2ae78d3f493bbbe31f98
@andrewyounkers
Copy link
Author

tagging @dstebila as discussed in the OQS status meeting on 7/29

@andrewyounkers andrewyounkers force-pushed the feature/openssh-10.0.P2-uplift branch from 5b4ebe7 to 3a4a115 Compare August 15, 2025 18:30
Update SNTRUP761 codepoints in the kex-names.c and sshd-session.c files
introduced with OPENSSH 9.8P1.

Signed-off-by: Andrew Younkers <ayounkers44@gmail.com>
With the kex-names.c file being introduced, logic has moved
from kex.c to kex-names.c. This commit cleans up duplicate
functions left behind in kex.c

Signed-off-by: Andrew Younkers <ayounkers44@gmail.com>
When merging commits from the upstream with tag 'V_9_8_p1',
commit 6849957 added autogenerated configure files. This commit
updates those files after running `autoreconf` with OQS enabled.

Signed-off-by: Andrew Younkers <ayounkers44@gmail.com>
When functionality was moved from sshd.c to sshd-session.c the OQS
specific logic removed needed to be added to the new location. The
`list_hostkey_types` function and `OQS_TEMPLATE_FRAGMENT_POINT_TO_KEX_GEN`
logic were updated to resolve this. Similar changes were needed in kex-names.c
to take in the `OQS_TEMPLATE_FRAGMENT_ADD_KEX_ALGS` that was removed from kex.c

Signed-off-by: Andrew Younkers <ayounkers44@gmail.com>
The generate.py script was updated for the introduced 'kex-names.c'
and 'sshd-session.c` files. OQS template subdirectories and requesite
templates were added for these files as well. There was also a change to
the README fragment to prevent adding a '-' character to the MLKEM algs,
avoiding confusion to the old codepoint references being added back when
the generate script is run.

Signed-off-by: Andrew Younkers <ayounkers44@gmail.com>
Additional clean-up from merge conflict resolution. The kex
defintions were resolved to use the existing implementation of
'KEX_ML_KEM_768_X25519_SHA256' instead of what openssh enabled
in V_9_9_P1. The 'kex-names.c' file had to be updated to reflect
this change. The ssh-rsa.c file was modified to remove a old call
to 'oqs_utils_is_rsa_hybrid' as the key parameter is no longer
available in that function.

Signed-off-by: Andrew Younkers <ayounkers44@gmail.com>
When uplifting to OpenSSH 9_8_P1, the upstream added in pre-populated
config files to the repo. This commit removes these files and relies
on the previous behavior of just using 'configure.ac' and 'autoconf'
to configure ssh.

Signed-off-by: Andrew Younkers <ayounkers44@gmail.com>
When uplifing to OpenSSH 10_0, logic was moved from the
'sshd-session.c' file to the new 'sshd-auth.c' file. As
a result of this the OQS specific logic needed to be added
back to the moved functions.

Signed-off-by: Andrew Younkers <ayounkers44@gmail.com>
With OQS template info being moved from 'sshd-session.c' to 'sshd-auth.c'
the algorithm generation logic needs to be updated to modify the template
information in the updated location.

Signed-off-by: Andrew Younkers <ayounkers44@gmail.com>
While upgrading to 10_0_P2, the OQS template information got
removed from myproposal.h causing failures during the sshd
syntax test. This commit reverts the file to contain the OQS
template fragment again.

Signed-off-by: Andrew Younkers <ayounkers44@gmail.com>
This commit includes the reference to the header file 'atomicio.h'
in 'sshd.c' that got dropped when resolving merge conflicts. It also
cleans up upstream mlkem768 behavior that was introduced during the uplift.
This was done to match the behavior in oqs-ssh for handling upstream sntrup761.

Signed-off-by: Andrew Younkers <ayounkers44@gmail.com>
Signed-off-by: Andrew Younkers <ayounkers44@gmail.com>
@andrewyounkers andrewyounkers force-pushed the feature/openssh-10.0.P2-uplift branch from 3a4a115 to 29c35bd Compare August 15, 2025 18:40
@dstebila
Copy link
Member

@ryjones This PR merges in lots of commits from openssh upstream, plus patches from @andrewyounkers to make those updates work for OQS-OpenSSH. Andrew has done DCO sign-off on the commits containing code he wrote, but there are no DCO sign-offs on the commits being merged in from openssh upstream. Is there anything we need to do here? Or can we just merge as-is?

@ryjones
Copy link

ryjones commented Aug 27, 2025

@ryjones This PR merges in lots of commits from openssh upstream, plus patches from @andrewyounkers to make those updates work for OQS-OpenSSH. Andrew has done DCO sign-off on the commits containing code he wrote, but there are no DCO sign-offs on the commits being merged in from openssh upstream. Is there anything we need to do here? Or can we just merge as-is?

@hartm

@hartm
Copy link

hartm commented Aug 27, 2025

Technically speaking, @andrewyounkers can count as the contributor here since he is doing the upstreaming, so he can sign the DCO for these commits. This should be the same as pulling in any (acceptably licensed) code.

Are you all OK rebasing this or do you want the full commit history?

@dstebila
Copy link
Member

Technically speaking, @andrewyounkers can count as the contributor here since he is doing the upstreaming, so he can sign the DCO for these commits. This should be the same as pulling in any (acceptably licensed) code.

Are you all OK rebasing this or do you want the full commit history?

I think we want the full commit history to make future pulls from upstream easier, but @andrewyounkers has done that process much more recently than me, so I defer to him on what setup is preferable for that process.

@hartm
Copy link

hartm commented Aug 27, 2025

Thanks @dstebila ! If we rebase, I think we're completely fine. If we don't, I think we are probably fine, but I will ask for legal's blessing anyway.

Does this need to be urgently merged? If so, let me know here and I'll try to expedite things.

@dstebila
Copy link
Member

Does this need to be urgently merged? If so, let me know here and I'll try to expedite things.

No rush.

@andrewyounkers
Copy link
Author

Technically speaking, @andrewyounkers can count as the contributor here since he is doing the upstreaming, so he can sign the DCO for these commits. This should be the same as pulling in any (acceptably licensed) code.
Are you all OK rebasing this or do you want the full commit history?

I think we want the full commit history to make future pulls from upstream easier, but @andrewyounkers has done that process much more recently than me, so I defer to him on what setup is preferable for that process.

I believe we will want to keep the full commit history as well for future merges with the upstream. If we did a rebase to sign off on the commits and changed the history, I would expect future merges to try and pull in these commits again.

@dstebila
Copy link
Member

dstebila commented Sep 9, 2025

DCO sign-off for commits from upstream:

Signed-off-by: Douglas Stebila dstebila@uwaterloo.ca

@dstebila dstebila merged commit 823716d into open-quantum-safe:OQS-v9 Sep 9, 2025
1 of 3 checks passed
@dstebila
Copy link
Member

dstebila commented Sep 9, 2025

Shoot, I screwed up merging the commit. In 2 ways. I forgot to not squash and merge, and also I think we want to merge this into a new branch (OQS-v10?) so that we can still merge #180 into the v9 branch.

@dstebila
Copy link
Member

dstebila commented Sep 9, 2025

Shoot, I screwed up merging the commit. In 2 ways. I forgot to not squash and merge, and also I think we want to merge this into a new branch (OQS-v10?) so that we can still merge #180 into the v9 branch.

I've backed out the merge to OQS-v9 (git reset --soft HEAD^; git push origin +OQS-v9).

@andrewyounkers Can you make the pull request again from your fork? I've created a new OQS-v10 branch (just branched from the same point of OQS-v9), so if you can make it against that, great, but if you can only make it against OQS-v9, then do that and we'll see if I can still merge it into OQS-v10. Sorry for the extra work.

@andrewyounkers
Copy link
Author

Shoot, I screwed up merging the commit. In 2 says. I forgot to not squash and merge, and also I think we want to merge this into a new branch (OQS-v10?) so that we can still merge #180 into the v9 branch.

I've backed out the merge to OQS-v9 (git reset --soft HEAD^; git push origin +OQS-v9).

@andrewyounkers Can you make the pull request again from your fork? I've created a new OQS-v10 branch (just branched from the same point of OQS-v9), so if you can make it against that, great, but if you can only make it against OQS-v9, then do that and we'll see if I can still merge it into OQS-v10. Sorry for the extra work.

It's no problem at all, I was able to open a new PR against OQS-v10

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.