artyom-poptsov / guile-ssh

Guile-SSH is a library that provides access to the SSH protocol for GNU Guile programs.
https://memory-heap.org/~avp/projects/guile-ssh
GNU General Public License v3.0
60 stars 12 forks source link

compatibility issues with libssh 0.10.x #34

Closed vagrantc closed 1 year ago

vagrantc commented 1 year ago

Builds of guile-ssh 0.13.x and 0.15.x in debian started failing roughly when libssh in Debian was updated to 0.10.4, roughly mid-september 2022:

https://tests.reproducible-builds.org/debian/history/amd64/guile-ssh.html

I've also tested locally with guile-ssh 0.16.0, and it seems to have the same issues.

One of the test failures is in tests/server.scm:

Test begin: test-name: "server-set!, valid values" source-file: "tests/server.scm" source-line: 50 source-form: (test-assert "server-set!, valid values" (begin (setup-test-logging! "server-set!, valid values") (set-log-userdata! "server-set!, valid values") (let* ((server (%make-server)) (topdir (getenv "abs_top_srcdir")) (options (quasiquote ((bindaddr "127.0.0.1") (bindport 22) (unquote (if (>= %libssh-minor-version 7) (list (quote hostkey) %rsakey %dsakey) (quote (hostkey "ssh-rsa" "ssh-dss")))) (rsakey (unquote %rsakey)) (dsakey (unquote %dsakey)) (banner "string") (log-verbosity nolog rare protocol packet functions) (blocking-mode #f #t)))) (log (test-runner-aux-value (test-runner-current))) (res #t)) (for-each (lambda (opt) (for-each (lambda (val) (catch #t (lambda () (server-set! server (car opt) val)) (lambda (key . args) (set! res #f) (format log " opt: ~a, val: ~a, error: ~a~%" (car opt) val args)))) (cdr opt))) options) res))) opt: hostkey, val: /build/1st/guile-ssh-0.15.1/debian/build/guile-3.0/../../../tests/keys/dsakey, error: (server-set! Unable to set the option (#<server 127.0.0.1:22 7f35b4d6cf60> hostkey /build/1st/guile-ssh-0.15.1/debian/build/guile-3.0/../../../tests/keys/dsakey) #f) Test end: result-kind: fail actual-value: #f

The other is in tests/keys.scm, though it is not obvious to me which actually failed from the build log:

https://tests.reproducible-builds.org/debian/rbuild/experimental/amd64/guile-ssh_0.15.1-1.rbuild.log.gz

vagrantc commented 1 year ago

The bug in debian is https://bugs.debian.org/1020087 also has the reported log failures from guile-ssh 0.13.x, if that is helpful.

vagrantc commented 1 year ago

I can also confirm that updating libssh to 0.10.4 and guile-ssh 0.16.0 on Guix also triggers the same issue, so it is not specific to Debian.

foo-dogsquared commented 1 year ago

I can also confirm it on nixpkgs with libssh updated to 0.10.0 and guile-ssh to 0.16.0 with the same tests failing as written from the original post.

Here's the build log for compiling it with libssh 0.10.0 from nixpkgs (i.e., nix build github:foo-dogsquared/nix-overlay-guix/ab1f301df1bb9ad80ac3ef9805c1e41cc3b5595f --override-input nixpkgs github:NixOS/nixpkgs/b84e8342dfe0f8aaea6dabf7531bd62a9c7e216d).

Here's another build log compiling the module with NixOS 22.05 which has libssh 0.9.6 (i.e., nix build github:foo-dogsquared/nix-overlay-guix/ab1f301df1bb9ad80ac3ef9805c1e41cc3b5595f#guile-ssh --override-input nixpkgs github:NixOS/nixpkgs/22.05).

vagrantc commented 1 year ago

Maybe this change with RSA key sizes:

https://salsa.debian.org/debian/libssh/-/blob/debian/CHANGELOG#L26

And DSA key support was disabled by default:

https://salsa.debian.org/debian/libssh/-/blob/debian/CHANGELOG#L37

Could be part of the issue.

vagrantc commented 1 year ago

Looks like tests/key.scm "string->public-key, ECDSA" is failing, and I found this error log which might be relevant:

==> tests/key/private-key-to-file-libssh.log <==                                                                                        
[2022-11-09T06:20:33+0000, "private-key-to-file", 2]: ssh_pki_import_privkey_base64: Trying to decode privkey passphrase=false          
[2022-11-09T06:20:33+0000, "private-key-to-file", 0]: [GSSH ERROR] Unable to convert the key to a string: #<key ecdsa (public) ffffa043eda0>

I was able to get the errors in tests/server.scm to pass by removing all references to dsa (a.k.a. dss) keys... though i guess a better check would be to check if libssh supports dsa, and skip the test if it does not? Crude workaround patch attached...

disable-dsa-in-server.diff.txt

vagrantc commented 1 year ago

Another proof-of-concept patch that allows tests/keys.scm to build successfully with libssh 0.10.x

tests-key-libssh-version-check.diff.txt

Obviously it should be adapted to also support libssh 0.9.x, or maybe > 0.9.x ...

vagrantc commented 1 year ago

I can confirm that building with a libssh 0.10.x built with -DWITH_DSA=on allows the tests/server.scm to pass without patches.

vagrantc commented 1 year ago

I asked the maintainers of libssh in Debian to re-enable the deprecated DSA support, and they strongly preferred not to:

https://bugs.debian.org/1025455

Given that it will be removed in the next major libssh release, that makes sense.

So I've tested my patch to disable DSA in the libssh test suite and that appears to be working, although if anyone tries to use DSA support with guile-ssh, there might be surprising bugs awaiting to be discovered.

If libssh will not support DSA anymore, perhaps it would be possible to add an option to disable DSA support when building guile-ssh?

Thanks!

artyom-poptsov commented 1 year ago

Hello,

I've just released Guile-SSH 0.16.1 where all the tests that require DSA public key algorithm are disabled by default: https://github.com/artyom-poptsov/guile-ssh/releases/tag/v0.16.1

With this change Guile-SSH tests are passing with libssh 0.10.

If you want to enable DSA-related tests, just pass --enable-dsa to the configure script.

Please check if it works for you.

Thanks, avp

vagrantc commented 1 year ago

Thanks for fixing the DSA tests, that part works great!

I still need one patch for libssh 0.10.x compatibility in keys.scm when openssl is enabled:

https://salsa.debian.org/debian/guile-ssh/-/blob/debian/0.16.0-2/debian/patches/tests-key-libssh-version-check.patch

The test "string->public-key, ECDSA" still checks explicitly for libssh minor version "9" but it should maybe be >= "9" ?

My patch for debian just changes it to "10" to keep it simple and good enough for Debian, but a better upstream approach is welcome! I think Guix builds of libssh might use gnutls instead of openssl, so this test would be skipped?

artyom-poptsov commented 1 year ago

Hello Vagrant,

I pushed a bugfix to the master that should solve the libssh version check issue: https://github.com/artyom-poptsov/guile-ssh/commit/ea028ccba9551a8dd032d586c6a68bbf5796dca7

Does it work for you?

Thanks, avp

vagrantc commented 1 year ago

Applying these two patches worked (the top patch dependend on the previous one, though could be backported if needed i suspect):

ea028ccba9551a8dd032d586c6a68bbf5796dca7 tests/key.scm ("string->public-key, ECDSA"): Bugfix 6dcbaf8102110658db4fdcdc0a80b9bbbf69e3f1 tests/key.scm ("string->public-key, ECDSA"): Use 'test-skip'

It also worked with all three patches from master, including:

7f7de7037ff834e4c671a8a823f409d369da09ac tests/key.scm ("make-keypair"): Split the test into several tests

Thanks!

artyom-poptsov commented 1 year ago

Great, glad to hear that.

As the problem is solved I'm closing the issue.