[MIR] libssh2

I believe the comments in the libssh MIR are relevant here: bug 492931

Hi,

does this mean you want to rewrite kio_sftp and maintain it in future cause you believe the FUD written down at http://www.libssh2.org/libssh2-vs-libssh.html ?

I detect some agressivenes from Andreas Schneider here and also at bug #492931 he claims FUD, but could you please clarify where this FUD is?
Please do so in your blog and not here. This is a bug tracker and no place for personal vendettas.

I do want to understand the situation as to where there is two libs for the same thing, and have thus contacted Daniel from libssh2 / curl for a response. He is currently in Thailand though so will get back from him in a few weeks.

PS. i am not involved in libssh or libssh2... just trying to resolve a user case issue (#311029)

There are two libraries which implement the SSH protocol. Both are well maintained and under active development. Developers prefer one or the other, cause they have different feature sets. There is no reason to replace libssh cause of that page. curl uses libssh2 and other applications are using libssh.

You don't remove GTK from a distribution cause there is QT.

libssh2 should be included in the distribution, but you should stop forcing people to choose the libraries they want to use to develop applications.

I'd like to see a few things fixed up before this goes into main:

- build-time FORTIFY_SOURCE warnings should be appropriately fixed, for example:
  sftp.c: In function 'kbd_callback':
  sftp.c:78: warning: ignoring return value of 'fgets', declared with attribute warn_unused_result
  sftp.c: In function 'main':
  sftp.c:259: warning: ignoring return value of 'write', declared with attribute warn_unused_result

- I'd like to see the upstream tests actually run at build-time

Beyond that, it looks fine to me.

Those warnings are from the example code, not from code that is used in the actual library. Upstream will still appreciate a patch to fix them of course.

[Expired for libssh2 (Ubuntu) because there has been no activity for 60 days.]

This MIR expired without a decision being made. But since a project I'm working on needs bug #311029 to be fixed, and #311029 is in turned blocked by this MIR, I'm reopening the MIR to hopefully make some progress on it.

One of the concernes that Kees Cook raised over this MIR was the compile-time warnings about ignored return values. While those only appear in example code and not in actual library code, it's always best to heed the warnings the compiler gives. I've written a patch to fix the compile-time warnings in the example code by actually checking the return values and doing something about them.

The other concern was that the upstream tests should actually run at build time. This patch addresses that concern.

It's been a month since I reopened this MIR, and no apparent activity.

@adconrad - Have you noticed anything else besides the possible problems Kees Cook identified and I've submitted patches for? Are there any other reasons why this package could not go into main? LP #311029 depends on this getting resolved, and I'd like to see that one fixed in time for Raring. Which means this one also needs action soon.

Hey, we'd really like it if the packaged version of curl supported sftp, it is unexpected from an end user standpoint when it doesn't, especially given that the man page specifically says so:

curl is a tool to transfer data from or to a server, using one of the
supported protocols (DICT, FILE, FTP, FTPS, GOPHER, HTTP, HTTPS, IMAP,
IMAPS, LDAP, LDAPS, POP3, POP3S, RTMP, RTSP, SCP, SFTP, SMTP, SMTPS,
TELNET and TFTP). The command is designed to work without user inter‐
action.

***

The end user shouldn't have to run curl -V to find out that this is not true for Ubuntu.

Looking through the comments here, it looks like the patches Robin Munn submitted actually fix the issues identified. What's the block on implementing this?

Status changed to 'Confirmed' because the bug affects multiple users.

Ping? nmap is blocked by this, it has always been using its embedded libssh2, and now it has moved to the system one.

I don't want to use embedded libssh2 libraries

security team, please could you re-review this. There seem to be now at least two users in main: curl and nmap

FYI: qemu would be able to drop one delta as well, not too important for our users but nice to have (ssh to remote disks).

FYI: qemu switched to libssh, so we don't need it for that anymore.
Bu tunless it changed as Matthias outlined curl and nmap would still benefit.

Chris, do you recall if upstream responded sufficiently to your findings? If they did, can you report back whether or not this package should be promoted to main?

Thanks

Hi Seth,
do you mean me or "Chris Coulson" who is also subscribed?

If you meant me: As I said in comment #18, the reason to promote it for qemu is gone (that was my motivation to participate here), but curl/nmap would still benefit.
There are no further updates to this from my side.

Hi Christian, sorry for the inconvenience, I meant Chris Coulson; he reviewed libssh2 earlier and found issues for upstream to address.

Thanks

Christian, I'm currently thinking we may be better served by:

- unseeding nmap from ubuntu-server (daily)
- removing nmap libssh2 vendored code and linking against the version in the archive
- keeping our delta in curl to use libssh

What do you think? Does the server team still need nmap seeded?

Thanks

Hi Seth,
thanks for your reply on this long ongoing discussion.
Looking back it seems while the two libs are alternatives to each other things slowly tend to lean more towards libssh, so less reasons than int he past to promote it IMHO.

You brought up something interesting with nmap...

# General thoughts
I think nmap is still a common tool to use and needs to stay in main.
So lets strike unseeding it from the list of options.
For curl I have no deep insight/opinion - ack to for now keeping the delta there as is.

That leaves the question what we could do about nmap.

# Only libssh2?

The same considerations are needed for libdnet as well.
That also is used as bundled lib, should/could be split and use the in-archive version.
This can be checked via the nmap prefix:
# nmap --version | grep nmap-
Compiled with: liblua-5.3.3 openssl-1.1.1c nmap-libssh2-1.8.2 libz-1.2.11 libpcre-8.39 libpcap-1.9.1 nmap-libdnet-1.12 ipv6

We see that libssh2 and libdnet are currently used as embedded libs.
But other than libssh libdnet is RECOMMENDED to be used built in:

-with-libdnet=DIR Use an existing (compiled) dnet lib from DIR/include
and DIR/lib. This is NOT RECOMMENDED because we hav
made many important fixes to our included libdnet,
as described at
./libdnet-stripped/NMAP_MODIFICATIONS

Is that still true and valid, I have no idea.
It also doesn't seem to be the same as the libdnet built from src:dnprogs
So in this case is it no duplication, but for libssh2 it is.

# back to ssh2

I was checking what ssh2 support actually means for nmap (feature wise).
There are two major changes depending if it is configured.

The obvious one is in the makefile that adds:
  nse_libssh2.cc

The other loads potential scripts that need ssh2 support:
550 #ifdef HAVE_LIBSSH2
551 {LIBSSH2LIBNAME, luaopen_libssh2},
552 #endif

The problem even on a potential conversion is that these bindings are exposed to the scripts.
And that means potentially out-of-package scripts that would break on the change.
This makes it even more complex to consider asking for a switch to libssh.

I guess we have to consider between
a) MIR and own libssh2 after all
b) disable the ssh2 features in nmap, but keep it in main

We might want to:
build without ssh2 support and check if
a) ssh scripts won't work anymore (expected)
b) we still can e.g. probe for ssh port and how much we loose (e.g. server identification)

I'll talk to the Team and get back to you on this Seth.

After the discussion with the Team the consensus was for nmap to demote it from main.
It isn't needed on install media nor is is a core part of any solution - so there is no hard reason to be in main.
The tool can continued to be used as-is from the universe repo.

=> https://code.launchpad.net/~paelzer/ubuntu-seeds/+git/ubuntu/+merge/376744

Christian, thanks so much for the discussion. Should we now set this MIR bug to Invalid?

Thanks

The nmap seed change is only in a few hours the demotion hasn't taken place yet.
So it will be a few more hours ... but then should appear in the archive as planned.

For curl the delta to convert it to libssh seems to do fine.
And we have no other package needing it right now.

So yes, lets mark it invalid.