-
Notifications
You must be signed in to change notification settings - Fork 62
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
Upgrade to OpenSSH v9.7 #159
Conversation
Hardenedmalloc dropped support for "legacy glibc" versions in their 64dad0a69 so use a newer Ubuntu version for the runner for that test.
`MAXHOSTNAMELEN` is not defined in POSIX, which breaks on musl: https://pubs.opengroup.org/onlinepubs/9699919799/functions/gethostname.html Bug: https://bugs.gentoo.org/834044
ok djm@ dtucker@ OpenBSD-Commit-ID: 3e6d47567b895c7c28855c7bd614e106c987a6d8
by github PR#410, ok deraadt. OpenBSD-Commit-ID: 0514cd51db3ec60239966622a0d3495b15406ddd
This function is apparently deprecated. Documentation on what is the supposed replacement is is non-existent, so this follows the approach glibc used https://sourceware.org/git/?p=glibc.git;a=patch;h=f278835f59 ok dtucker@
OpenBSD-Commit-ID: d0f12af0a5067a756aa707bc39a83fa6f58bf7e5
overflows reported by Yair Mizrahi @ JFrog; feedback/ok millert@ OpenBSD-Commit-ID: 52af085f4e7ef9f9d8423d8c1840a6a88bda90bd
These too are unreachable, but we want the code to be safe regardless of context. Reported by Yair Mizrahi @ JFrog
OpenBSD-Commit-ID: e7c31034a5434f2ead3579b13a7892960651e6b0
This defines wire formats for optional KRL extensions and implements parsing of the new submessages. No actual extensions are supported at this point. ok markus OpenBSD-Commit-ID: ae2fcde9a22a9ba7f765bd4f36b3f5901d8c3fa7
When the KRL format was originally defined, it included support for signing of KRL objects. However, the code to sign KRLs and verify KRL signatues was never completed in OpenSSH. Now, some years later, we have SSHSIG support in ssh-keygen that is more general, well tested and actually works. So this removes the semi-finished KRL signing/verification support from OpenSSH and refactors the remaining code to realise the benefit - primarily, we no longer need to perform multiple parsing passes over KRL objects. ok markus@ OpenBSD-Commit-ID: 517437bab3d8180f695c775410c052340e038804
This allows matching on the addresses of available network interfaces and may be used to vary the effective client configuration based on network location (e.g. to use a ProxyJump when not on a particular network). ok markus@ OpenBSD-Commit-ID: cffb6ff9a3803abfc52b5cad0aa190c5e424c139
This adds a ssh_config(5) "Tag" directive and corresponding "Match tag" predicate that may be used to select blocks of configuration similar to the pf.conf(5) keywords of the same name. ok markus OpenBSD-Commit-ID: dc08358e70e702b59ac3e591827e5a96141b06a3
valid magic number and not SSH_ERR_MESSAGE_INCOMPLETE; the former is needed to fall back to text revocation lists in some cases; fixes t-cert-hostkey. OpenBSD-Commit-ID: 5c670a6c0f027e99b7774ef29f18ba088549c7e1
where it caused merge conflict in -portable for each commit :( OpenBSD-Commit-ID: 756ebac963df3245258b962e88150ebab9d5fc20
too no code change OpenBSD-Commit-ID: ef5bf46b57726e4260a63b032b0b5ac3b4fe9cd4
OpenBSD-Commit-ID: 4776ced33b780f1db0b2902faec99312f26a726b
OpenBSD-Commit-ID: 535f5257c779e26c6a662a038d241b017f8cab7c
with that in ssh.1 - reformat usage() to match what "man ssh" does on 80width OpenBSD-Commit-ID: 5235dd7aa42e5bf90ae54579d519f92fc107036e
OpenBSD-Commit-ID: 9a08ed8dae27d3f38cf280f1b28d4e0ff41a737a
Fixes build breakage on platforms that lack getifaddrs()
fixes build on AIX5 at least
that isn't a PKCS#11 provider; from / ok markus@ OpenBSD-Commit-ID: 39532cf18b115881bb4cfaee32084497aadfa05c
libraries to ssh-agent by default. The old behaviour of allowing remote clients from loading providers can be restored using `ssh-agent -O allow-remote-pkcs11`. Detection of local/remote clients requires a ssh(1) that supports the `[email protected]` extension. Forwarding access to a ssh-agent socket using non-OpenSSH tools may circumvent this control. ok markus@ OpenBSD-Commit-ID: 4c2bdf79b214ae7e60cc8c39a45501344fa7bd7c
This checks via nlist(3) that candidate provider libraries contain one of the symbols that we will require prior to dlopen(), which can cause a number of side effects, including execution of constructors. Feedback deraadt; ok markus OpenBSD-Commit-ID: 1508a5fbd74e329e69a55b56c453c292029aefbe
Make ssh-pkcs11-client start an independent helper for each provider, providing better isolation between modules and reliability if a single module misbehaves. This also implements reference counting of PKCS#11-hosted keys, allowing ssh-pkcs11-helper subprocesses to be automatically reaped when no remaining keys reference them. This fixes some bugs we have that make PKCS11 keys unusable after they have been deleted, e.g. https://bugzilla.mindrot.org/show_bug.cgi?id=3125 ok markus@ OpenBSD-Commit-ID: 0ce188b14fe271ab0568f4500070d96c5657244e
to the active configuration. This fixes the config parser from erroneously rejecting cases like: AuthenticationMethods password Match User ivy AuthenticationMethods any bz3657 ok markus@ OpenBSD-Commit-ID: 7f196cba634c2a3dba115f3fac3c4635a2199491
spotted by Coverity (CID 438039) OpenBSD-Commit-ID: 208839699939721f452a4418afc028a9f9d3d8af
discussed with deraadt and dtucker a while ago
Unbreaks "make test" when compiled --without-openssl. Similar treatment to how we do DSA and ECDSA.
OpenBSD-Commit-ID: 463e4a69eef3426a43a2b922c4e7b2011885d923
found by RASU JSC, reported by Maks Mishin in GHPR#467 OpenBSD-Commit-ID: 97d96a166b1ad4b8d229864a553e3e56d3116860
Use openssl in the directory specified by --with-ssl-dir as long as it's functional. Reported by The Doctor.
$TEST_SHELL. Fixes test when run by a user whose login shell is tcsh. Found by vinschen at redhat.com. OpenBSD-Regress-ID: f68d79e7f00caa8d216ebe00ee5f0adbb944062a
allowed_signers files with blank lines; reported by Wiktor Kwapisiewicz OpenBSD-Commit-ID: b3a22a2afd753d70766f34bc7f309c03706b5298
ppoll() bz3670, reported by Ben Hamilton; ok dtucker@ OpenBSD-Commit-ID: e58f18042b86425405ca09e6e9d7dfa1df9f5f7f
Fixes test failures on Solaris 8 reported by Tom G. Christensen
Add LibreSSL 3.9.0, bump older branches to their respective current releases.
OpenBSD-Commit-ID: 618ececf58b8cdae016b149787af06240f7b0cbc
Thanks very much for your work and particularly the explanations, @geedo0 ! So do you indeed prefer to merge this first to the new branch before all CI checks turn green? That feels not quite right, but would surely create a clean git log (upstream merge in this PR, getting OQS integration to work again in the next one), so fine with me unless someone objects. |
Yes, I think it's okay since it's going into the new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've read through the description of your process, and also skimmed the diff between geedo0:merge-ossh97
and openssh:V_9_7_P1
. I'm okay with the idea of merging this into OQS-v9
and then continuing with the next phase of work on that branch.
My only comment would be about whether we should omit the .get_allowed_signers*
files from our repository; I'm not sure what the purpose of these files is, but if it's about who's authorized to sign certain things, then we may nto intend to authorize the OpenSSH team to sign things related to our fork.
Good catch, you got me curious and I looked into it. It looks like you're allowed to use SSH keys to sign commits (instead of GPG keys). This file gives you granular control over the SSH keys authorized to do that. This fork doesn't enforce anything related to commit signing, but it's certainly good hygeine to not pull-in such enforcement from upstream. |
This is a first pass at resolving all of the merge conflicts between the current tip of `OQS-v8` and the `V_9_7_P1` tag in upstream OpenSSH. The merge strategy here differs a bit from previous upstream merges (e.g. PR open-quantum-safe#106 and PR open-quantum-safe#121) where all of the changes were squashed and incorporated into a single commit and applied to the trunk. This is a more typical `git merge` in that we retain both parents and their commit histories. This will make future merges more straightforward by allowing git to notice the shared history and avoid marking these merged commits as conflicting changes. Here's the git-foo used to script the merge and handle the false positives from the "squash merges". ``` oqs_tip=OQS-v8 openssh_release=V_9_7_P1 git merge ${openssh_release} base=`git merge-base ${oqs_tip} ${openssh_release}` for f in `git diff --name-only --diff-filter=U`; do # This fetches all of the commits which touched the file since the merge base # Filter out the two commits for the 8.6 and 8.9 merges since they are technically already incorporated conflicts=$(git log --oneline ${base}..${oqs_tip} -- $f | ggrep -v -P '(1f58edd|f058d3168)') # Check if we have no OQS-OpenSSH conflicts specific if [[ -z ${conflicts} ]]; then echo "$f has no conflicts" # Resolve the conflict by taking the upstream version of the file git checkout --theirs -- $f git add $f else echo "$f has conflicts" echo ${conflicts} # Send all of the OQS diffs to a file to help resolve the merge conflicts for c in `echo ${conflicts} | cut -d ' ' -f1`; do git show $c -- $f >> ~/conflicting_diffs.t done fi done ``` For the remaining conflicts, I went through each file one-by-one with this pseudo-algorithm: 1. Incorporate all changes from both sides that have no direct conflicts. 2. Look for OQS specific changes with conflicts and apply them as-appropriate. 3. Take the upstream version for any remaining conflicts. Callouts from this process: - `sshkey.c` and `sshkey.h` experienced a major refactor upstream that impacted how OQS modified these files. I simply took the upstream versions for now and plan to address the conflict properly in a separate PR. - Kept `README.md` as-is from OQS and applied changes to `README.original.md`. - Took `.depend` from upstream, will update in a subsequent commit. - `version.h` retained the 2022-01 datestamp from OQS, will update this when we're ready to stage a release. - In `ssh-keygen.c` the `OQS_TEMPLATE_FRAGMENT_PRINT_RESOURCE_RECORDS_START` template changed to accept two additional arguments `opts` and `nopts`. I added these in manually for now. To self-check I did the following: - Test build by running `build_openssh.sh` and finding compiler errors. - Run `git diff HEAD V_9_7_P1` to highlight all the changes and assert that all changes were introduced by OQS alone. This last process flagged a handful of issues. Mostly around duplicated code blocks from taking them from previous upstream merges and this current merge and git not noticing it. With that out of the way, I'm reasonably confident that this PR is pretty close to upstream v9.7 with only the changes from OQS applied to it. So after all that, what's working so far? `build_openssh.sh` will build the project but fail to install with some error about unknown key types. What's next? - Properly handle the merge conflicts in `sshkey.(c|h)`. - Regenerate `.depend`. - Fix the impacted OQS templates and regenerate the source. - Cut a new `OQS-v9` branch and update `version.h`.
I removed the |
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.
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]>
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]>
This is a first pass at resolving all of the merge conflicts between the current tip of
OQS-v8
and theV_9_7_P1
tag in upstream OpenSSH (Issue #135).The merge strategy here differs a bit from previous upstream merges (e.g. PR #106 and PR #121) where all of the changes were squashed and incorporated into a single commit and applied to the trunk. This is a more typical
git merge
in that we retain both parents and their commit histories. This will make future merges more straightforward by allowing git to notice the shared history and avoid marking these merged commits as conflicting changes. For the maintainers, make sure to merge using the default "Merge Pull Request" button. I tested this on my own fork and seems to be the only one of Github's options that preserves the history.Here's the git-foo used to script the merge and handle the false positives from the "squash merges".
For the remaining conflicts, I went through each file one-by-one with this pseudo-algorithm:
Callouts from this process:
sshkey.c
andsshkey.h
experienced a major refactor upstream that impacted how OQS modified these files. I simply took the upstream versions for now and plan to address the conflict properly in a separate PR.README.md
as-is from OQS and applied changes toREADME.original.md
..depend
from upstream, will update in a subsequent commit.version.h
retained the 2022-01 datestamp from OQS, will update this when we're ready to stage a release.ssh-keygen.c
theOQS_TEMPLATE_FRAGMENT_PRINT_RESOURCE_RECORDS_START
template changed to accept two additional argumentsopts
andnopts
. I added these in manually for now.To self-check I did the following:
build_openssh.sh
and finding compiler errors.git diff HEAD V_9_7_P1
to highlight all the changes and assert that all changes were introduced by OQS alone.This last process flagged a handful of issues. Mostly around duplicated code blocks from taking them from previous upstream merges and this current merge and git not noticing it. With that out of the way, I'm reasonably confident that this PR is pretty close to upstream v9.7 with only the changes from OQS applied to it.
So after all that, what's working so far?
build_openssh.sh
will build the project but fail to install with some error about unknown key types. The failure I had locally is the same one reported by the CI job.What's next?
sshkey.(c|h)
..depend
.OQS-v9
branch and updateversion.h
.