libnetconf2 support on QNX platform

[apropp-molex]

Hi,

I'd like to port libnetconf2 over to QNX. Below I've summarized the issues in no particular order.
Let me know if you want more details for any of these issues. Aside from issue 2, I can submit patches for the rest. If you do want patches, how would you want them divided?

1. fd_set and friends are defined in #include <sys/select.h> on QNX. The relevant file is session_client.c.
2. asprintf, vasprintf and strndup are used in io.c, messages_server.c, session.c and session_client.c but are not available on QNX. Would it be possible to provide a compat.c similar to this commit in libyang?
3. #include <sys/syscall.h> is not available on QNX. It's used in session_client.c. Would it be possible to put it in a #ifdef __linux__ wrapper, as the only time it's called is in a section of code also delimited by #ifdef __linux__?
4. In session_server.c, nc_accept_unix relies on SO_PEERCRED. I think it would be possible to provide an alternate implementation that would cover the BSDs and as a result QNX (networking stack is NetBSD) and perhaps MAC (due to Darwin). getpeereid could be used to get the euid which in turn could be used for the rest of nc_accept_unix. See here for details and here for QNX documentation. What are your thoughts on this?
5. QNX links against -llogin instead of -lcrypt for the crypt() function. See target_link_libraries(netconf2 "-L${LIBSSH_LIBRARY_DIR}" -lssh -lcrypt) in CMakeLists.txt.
6. In session_server_ssh.c, QNX doesn't provide #include <crypt.h>. It does however provide #include <shadow.h>.
7. In session_server_ssh.c, getspnam_r is used in auth_password_get_pwd_hash. getspnam_r is defined differently for QNX compared to linux. In QNX it returns what would otherwise be obtained from the fifth parameter. Here's the doc.

[michalvasko]

Hi,
okay, my feedback:

1. Simply a missing header, was added.
2. Should now work.
3. Done.
4. You can provide a patch for this, I suppose.
5. I cleared it up a bit so it should be easier to implement this. Please provide a patch.
6. Include in a patch.
7. A patch.

You can include it all in a single pull request (instead of a patch) and please try to make as little mess as possible although I am quite worried about the result. ifdefs are always a pain.

Regards,
Michal

[apropp-molex]

Sorry for the delay. I've linked the PR against this issue.
I should note that asprintf is still implicitly defined in time.c and session_server_tls.c. I'm unsure of how to fix this issue. Thanks for your efforts so far.

[michalvasko]

As for the implicit declarations, the header was missing from those files. Should now work fine.

Regards,
Michal

[apropp-molex]

Hi Michal,

Thanks for adding the header to those files. No warnings now.

[apropp-molex]

Hi Michal,

One issue I've come to notice is this snippet in CMakeLists.txt.

if(ENABLE_SSH)
    find_library(LIBCRYPT crypt)
    if(LIBCRYPT STREQUAL LIBCRYPT-NOTFOUND)
        message(WARNING "LIBCRYPT not found! SSH, and TLS support disabled.")
        set(ENABLE_SSH OFF)
        set(ENABLE_TLS OFF)
    endif()
endif()

I've bypassed this safety check with -DLIBCRYPT and pointing to libcrypto.so. However, this is more of a hack than anything else.
As far as I understand, libcrypt exposes crypt, encrypt and setkey, of which only crypt is used in libnetconf2, which is only called in session_server_ssh.c. So at the very least, only set(ENABLE_SSH OFF) should be present. However, as noted above, QNX links crypt with liblogin instead.
Based off this do you think it makes more sense to change it to if(ENABLE_SSH and not QNX) in the above snippet or just let the linking fail during compilation and drop the safety check? The safety check might be cumbersome as libnetconf2 is ported to other patforms.
Let me know your thoughts and if you want a separate PR/issue for this.

[michalvasko]

Hi,
right, so I think this if can be removed altogether as crypt is linked later and so any checks should be performed there. So, one possibility is to just link it and let it fail later like you said, the error will be clear enough. The second one is instead of linking it directly, using find_library to find and link the correct library. I will leave the choice up to you.

Regards,
Michal

[apropp-molex]

Hi Michal,

Closing this issue off. Thanks so much for your help with this.