Skip to content

Commit

Permalink
build fixes and improvements (mostly for Windows)
Browse files Browse the repository at this point in the history
- in `hostkey.c` check the result of `libssh2_sha256_init()` and
  `libssh2_sha512_init()` calls. This avoid the warning that we're
  ignoring the return values.

- fix code using `int` (or `SOCKET`) for sockets. Use libssh2's
  dedicated `libssh2_socket_t` and `LIBSSH2_INVALID_SOCKET` instead.

- fix compiler warnings due to `STATUS_*` macro redefinitions between
  `ntstatus.h` / `winnt.h`. Solve it by manually defining the single
  `STATUS` value we need from `ntstatus.h` and stop including the whole
  header.
  Fixes libssh2#733

- improve Windows UWP/WinRT builds by detecting it with code copied
  from the curl project. Then excluding problematic libssh2 parts
  according to PR by Dmitry Kostjučenko.
  Fixes libssh2#734

- always use `SecureZeroMemory()` on Windows.

  We can tweak this if not found or not inlined by a C compiler which
  we otherwise support. Same if it causes issues with UWP apps.

  Ref: https://learn.microsoft.com/en-us/previous-versions/windows/desktop/legacy/aa366877(v=vs.85)
  Ref: https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-rtlsecurezeromemory

- always enable `LIBSSH2_CLEAR_MEMORY` on Windows. CMake and
  curl-for-win builds already did that. Delete `SecureZeroMemory()`
  detection from autotools' WinCNG backend logic, that this
  setting used to depend on.

  TODO: Enable it for all platforms in a separate PR.
  TODO: For clearing buffers in WinCNG, call `_libssh2_explicit_zero()`,
        insead of a local function or explicit `SecureZeroMemory()`.

- Makefile.inc: move `os400qc3.h` to `HEADERS`. This fixes
  compilation on non-unixy platforms. Recent regression.

- `libssh2.rc`: replace copyright with plain ASCII, as in curl.

  Ref: curl/curl@1ca62bb
  Ref: curl/curl#7765
  Ref: curl/curl#7776

- CMake fixes and improvements:

  - enable warnings with llvm/clang.
  - enable more comprehensive warnings with gcc and llvm/clang.
    Logic copied from curl:
    https://github.com/curl/curl/blob/233810bb5f6c5e7bedfc10bdd36607b958c0cfe4/CMakeLists.txt#L131-L148
  - fix `Policy CMP0080` CMake warning by deleting that reference.
  - add `ENABLE_WERROR` (default: `OFF`) option. Ported from curl.
  - add `PICKY_COMPILER` (default: `ON`) option, as known from curl.

    It controls both the newly added picky warnings for llvm/clang and
    gcc, and also the pre-existing ones for MSVC.

- `win32/GNUmakefile` fixes and improvements:

  - delete `_AMD64_` and add missing `-m64` for x64 builds under test.
  - add support for `ARCH=custom`.
    It disables hardcoded Intel 64-bit and Intel 32-bit options,
    allowing ARM64 builds.
  - add support for `LIBSSH2_RCFLAG_EXTRAS`.
    To pass custom options to windres, e.g. in ARM64 builds.
  - add support for `LIBSSH2_RC`. To override `windres`.
  - delete support for Metrowerks C. Last released in 2004.

- `win32/libssh2_config.h`: delete unnecessary socket #includes

  `src/libssh2_priv.h` includes `winsock2.h` and `ws2tcpip.h` further
   down the line, triggered by `HAVE_WINSOCK2_H`.

  `mswsock.h` does not seem to be necessary anymore.

  Double-including these (before `windows.h`) caused compiler failures
  when building against BoringSSL and warnings with LibreSSL. We could
  work this around by passing `-DNOCRYPT`. Deleting the duplicates
  fixes these issues.

  Timeline:
  2013: c910cd3 deleted `mswsock.h` from `src/libssh2_priv.h`
  2008: 8c43bc5 added `winsock2.h` and `ws2tcpip.h` to `src/libssh2_priv.h`
  2005: dc4bb1a added the now deleted #includes

- delete or replace `LIBSSH2_WIN32` with `WIN32`.

- replace hand-rolled `HAVE_WINDOWS_H` macro with `WIN32`. Also delete
  its detections/definitions.

- delete unused `LIBSSH2_DARWIN` macro.

- delete unused `writev()` Windows implementation

  There is no reference to `writev()` since 2007-02-02, commit
  9d55db6.

- fix a bunch of MSVC / llvm/clang / gcc compiler warnings:

  - `warning C4100: '...': unreferenced formal parameter`
  - using value of undefined PP macro `LIBSSH2DEBUG`
  - missing void from function definition
  - `if()` block missing in non-debug builds
  - unreferenced variable in non-debug builds
  - `warning: must specify at least one argument for '...' parameter of variadic macro [-Wgnu-zero-variadic-macro-arguments]`
    in `_libssh2_debug()`
  - `warning C4295: 'ciphertext' : array is too small to include a terminating null character`
  - `warning C4706: assignment within conditional expression`
  - `warning C4996: 'inet_addr': Use inet_pton() or InetPton() instead or
      define _WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings`
    By suppressning it. Would be best to use inet_pton() as suggested.
    On Windows this needs Vista though.
  - `warning C4152: nonstandard extension, function/data pointer conversion in expression`
    (silenced locally)
  - `warning C4068: unknown pragma`

  Ref: https://ci.appveyor.com/project/libssh2org/libssh2/builds/46354480/job/j7d0m34qgq8rag5w

Closes libssh2#808
  • Loading branch information
vszakats authored and agreppin committed Jul 14, 2024
1 parent ce541a1 commit e712784
Show file tree
Hide file tree
Showing 58 changed files with 397 additions and 268 deletions.
4 changes: 2 additions & 2 deletions Makefile.inc
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ CSOURCES = channel.c comp.c crypt.c hostkey.c kex.c mac.c misc.c \
packet.c publickey.c scp.c session.c sftp.c userauth.c transport.c \
userauth_kbd_packet.c \
version.c knownhost.c agent.c $(CRYPTO_CSOURCES) pem.c keepalive.c global.c \
blowfish.c bcrypt_pbkdf.c agent_win.c os400qc3.c os400qc3.h
blowfish.c bcrypt_pbkdf.c agent_win.c os400qc3.c

HHEADERS = libssh2_priv.h $(CRYPTO_HHEADERS) transport.h channel.h comp.h \
mac.h misc.h packet.h userauth.h session.h sftp.h crypto.h blf.h agent.h \
userauth_kbd_packet.h
userauth_kbd_packet.h os400qc3.h
2 changes: 0 additions & 2 deletions acinclude.m4
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,6 @@ m4_case([$1],
# Look for Windows Cryptography API: Next Generation
AC_CHECK_HEADERS([ntdef.h ntstatus.h], [], [], [#include <windows.h>])
AC_CHECK_DECLS([SecureZeroMemory], [], [], [#include <windows.h>])
LIBSSH2_LIB_HAVE_LINKFLAGS([crypt32], [], [
#include <windows.h>
Expand All @@ -477,7 +476,6 @@ m4_case([$1],
AC_DEFINE(LIBSSH2_WINCNG, 1, [Use $1])
found_crypto="$1"
found_crypto_str="Windows Cryptography API: Next Generation"
support_clear_memory="$ac_cv_have_decl_SecureZeroMemory"
])
],
)
Expand Down
58 changes: 48 additions & 10 deletions cmake/max_warnings.cmake
Original file line number Diff line number Diff line change
@@ -1,23 +1,61 @@
include(CheckCCompilerFlag)

option(ENABLE_WERROR "Turn compiler warnings into errors" OFF)
option(PICKY_COMPILER "Enable picky compiler options" ON)

if(ENABLE_WERROR)
if(MSVC)
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /WX")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /WX")
else() # llvm/clang and gcc style options
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Werror")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror")
endif()
endif()

if(MSVC)
# Use the highest warning level for visual studio.
if(CMAKE_CXX_FLAGS MATCHES "/W[0-4]")
string(REGEX REPLACE "/W[0-4]" "/W4" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
else()
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /W4")
endif()
if(CMAKE_C_FLAGS MATCHES "/W[0-4]")
string(REGEX REPLACE "/W[0-4]" "/W4" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
else()
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /W4")
if(PICKY_COMPILER)
if(CMAKE_CXX_FLAGS MATCHES "/W[0-4]")
string(REGEX REPLACE "/W[0-4]" "/W4" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
else()
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /W4")
endif()
if(CMAKE_C_FLAGS MATCHES "/W[0-4]")
string(REGEX REPLACE "/W[0-4]" "/W4" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
else()
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /W4")
endif()
endif()

# Disable broken warnings
add_definitions(-D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE)
elseif(CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_GNUCXX)
elseif(CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_GNUCXX OR CMAKE_C_COMPILER_ID MATCHES "Clang")
if(NOT CMAKE_CXX_FLAGS MATCHES "-Wall")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall")
endif()
if(NOT CMAKE_C_FLAGS MATCHES "-Wall")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall")
endif()

if(PICKY_COMPILER)
foreach(_CCOPT -pedantic -W -Wpointer-arith -Wwrite-strings -Wunused -Wshadow -Winline -Wnested-externs -Wmissing-declarations -Wmissing-prototypes -Wfloat-equal -Wsign-compare -Wundef -Wendif-labels -Wstrict-prototypes -Wdeclaration-after-statement -Wstrict-aliasing=3 -Wcast-align -Wtype-limits -Wold-style-declaration -Wmissing-parameter-type -Wempty-body -Wclobbered -Wignored-qualifiers -Wconversion -Wvla -Wdouble-promotion -Wenum-conversion -Warith-conversion)
# surprisingly, CHECK_C_COMPILER_FLAG needs a new variable to store each new
# test result in.
string(MAKE_C_IDENTIFIER "OPT${_CCOPT}" _optvarname)
check_c_compiler_flag(${_CCOPT} ${_optvarname})
if(${_optvarname})
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${_CCOPT}")
endif()
endforeach()
foreach(_CCOPT long-long multichar format-nonliteral sign-conversion system-headers pedantic-ms-format)
# GCC only warns about unknown -Wno- options if there are also other diagnostic messages,
# so test for the positive form instead
string(MAKE_C_IDENTIFIER "OPT${_CCOPT}" _optvarname)
check_c_compiler_flag("-W${_CCOPT}" ${_optvarname})
if(${_optvarname})
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-${_CCOPT}")
endif()
endforeach()
endif()
endif()
2 changes: 0 additions & 2 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,9 @@ AB_INIT
AC_CANONICAL_HOST
case "$host" in
*-mingw*)
CFLAGS="$CFLAGS -DLIBSSH2_WIN32"
LIBS="$LIBS -lws2_32"
;;
*darwin*)
CFLAGS="$CFLAGS -DLIBSSH2_DARWIN"
;;
*hpux*)
;;
Expand Down
35 changes: 16 additions & 19 deletions example/direct_tcpip.c
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
#ifdef WIN32
#ifndef _WINSOCK_DEPRECATED_NO_WARNINGS
#define _WINSOCK_DEPRECATED_NO_WARNINGS
#endif
#endif

#include "libssh2_config.h"
#include <libssh2.h>

Expand Down Expand Up @@ -64,11 +70,12 @@ int main(int argc, char *argv[])
struct timeval tv;
ssize_t len, wr;
char buf[16384];
libssh2_socket_t sock = LIBSSH2_INVALID_SOCKET;
libssh2_socket_t listensock = LIBSSH2_INVALID_SOCKET;
libssh2_socket_t forwardsock = LIBSSH2_INVALID_SOCKET;

#ifdef WIN32
char sockopt;
SOCKET sock = INVALID_SOCKET;
SOCKET listensock = INVALID_SOCKET, forwardsock = INVALID_SOCKET;
WSADATA wsadata;
int err;

Expand All @@ -78,8 +85,7 @@ int main(int argc, char *argv[])
return 1;
}
#else
int sockopt, sock = -1;
int listensock = -1, forwardsock = -1;
int sockopt;
#endif

if(argc > 1)
Expand All @@ -105,17 +111,14 @@ int main(int argc, char *argv[])

/* Connect to SSH server */
sock = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP);
if(sock == LIBSSH2_INVALID_SOCKET) {
#ifdef WIN32
if(sock == INVALID_SOCKET) {
fprintf(stderr, "failed to open socket!\n");
return -1;
}
#else
if(sock == -1) {
perror("socket");
#endif
return -1;
}
#endif

sin.sin_family = AF_INET;
sin.sin_addr.s_addr = inet_addr(server_ip);
Expand Down Expand Up @@ -193,17 +196,14 @@ int main(int argc, char *argv[])
}

listensock = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP);
if(listensock == LIBSSH2_INVALID_SOCKET) {
#ifdef WIN32
if(listensock == INVALID_SOCKET) {
fprintf(stderr, "failed to open listen socket!\n");
return -1;
}
#else
if(listensock == -1) {
perror("socket");
#endif
return -1;
}
#endif

sin.sin_family = AF_INET;
sin.sin_port = htons(local_listenport);
Expand All @@ -229,17 +229,14 @@ int main(int argc, char *argv[])
inet_ntoa(sin.sin_addr), ntohs(sin.sin_port));

forwardsock = accept(listensock, (struct sockaddr *)&sin, &sinlen);
if(forwardsock == LIBSSH2_INVALID_SOCKET) {
#ifdef WIN32
if(forwardsock == INVALID_SOCKET) {
fprintf(stderr, "failed to accept forward socket!\n");
goto shutdown;
}
#else
if(forwardsock == -1) {
perror("accept");
#endif
goto shutdown;
}
#endif

shost = inet_ntoa(sin.sin_addr);
sport = ntohs(sin.sin_port);
Expand Down
9 changes: 8 additions & 1 deletion example/scp.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@
* Sample showing how to do a simple SCP transfer.
*/

#ifdef WIN32
#ifndef _WINSOCK_DEPRECATED_NO_WARNINGS
#define _WINSOCK_DEPRECATED_NO_WARNINGS
#endif
#endif

#include "libssh2_config.h"
#include <libssh2.h>

Expand Down Expand Up @@ -33,7 +39,8 @@
int main(int argc, char *argv[])
{
unsigned long hostaddr;
int sock, i, auth_pw = 1;
libssh2_socket_t sock;
int i, auth_pw = 1;
struct sockaddr_in sin;
const char *fingerprint;
LIBSSH2_SESSION *session;
Expand Down
11 changes: 9 additions & 2 deletions example/scp_nonblock.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@
* "scp_nonblock 192.168.0.1 user password /tmp/secrets"
*/

#ifdef WIN32
#ifndef _WINSOCK_DEPRECATED_NO_WARNINGS
#define _WINSOCK_DEPRECATED_NO_WARNINGS
#endif
#endif

#include "libssh2_config.h"
#include <libssh2.h>

Expand Down Expand Up @@ -47,7 +53,7 @@ static long tvdiff(struct timeval newer, struct timeval older)
}
#endif

static int waitsocket(int socket_fd, LIBSSH2_SESSION *session)
static int waitsocket(libssh2_socket_t socket_fd, LIBSSH2_SESSION *session)
{
struct timeval timeout;
int rc;
Expand Down Expand Up @@ -80,7 +86,8 @@ static int waitsocket(int socket_fd, LIBSSH2_SESSION *session)
int main(int argc, char *argv[])
{
unsigned long hostaddr;
int sock, i, auth_pw = 1;
libssh2_socket_t sock;
int i, auth_pw = 1;
struct sockaddr_in sin;
const char *fingerprint;
LIBSSH2_SESSION *session;
Expand Down
11 changes: 9 additions & 2 deletions example/scp_write.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@
* Sample showing how to do an SCP upload.
*/

#ifdef WIN32
#ifndef _WINSOCK_DEPRECATED_NO_WARNINGS
#define _WINSOCK_DEPRECATED_NO_WARNINGS
#endif
#endif

#include "libssh2_config.h"
#include <libssh2.h>

Expand Down Expand Up @@ -33,7 +39,8 @@
int main(int argc, char *argv[])
{
unsigned long hostaddr;
int sock, i, auth_pw = 1;
libssh2_socket_t sock;
int i, auth_pw = 1;
struct sockaddr_in sin;
const char *fingerprint;
LIBSSH2_SESSION *session = NULL;
Expand Down Expand Up @@ -98,7 +105,7 @@ int main(int argc, char *argv[])
* connection
*/
sock = socket(AF_INET, SOCK_STREAM, 0);
if(-1 == sock) {
if(sock == LIBSSH2_INVALID_SOCKET) {
fprintf(stderr, "failed to create socket!\n");
return -1;
}
Expand Down
12 changes: 9 additions & 3 deletions example/scp_write_nonblock.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,13 @@
* Sample showing how to do an SCP non-blocking upload transfer.
*/

#include "libssh2_config.h"
#ifdef WIN32
#ifndef _WINSOCK_DEPRECATED_NO_WARNINGS
#define _WINSOCK_DEPRECATED_NO_WARNINGS
#endif
#endif

#include "libssh2_config.h"
#include <libssh2.h>

#ifdef HAVE_WINSOCK2_H
Expand Down Expand Up @@ -35,7 +40,7 @@
#include <ctype.h>
#include <time.h>

static int waitsocket(int socket_fd, LIBSSH2_SESSION *session)
static int waitsocket(libssh2_socket_t socket_fd, LIBSSH2_SESSION *session)
{
struct timeval timeout;
int rc;
Expand Down Expand Up @@ -68,7 +73,8 @@ static int waitsocket(int socket_fd, LIBSSH2_SESSION *session)
int main(int argc, char *argv[])
{
unsigned long hostaddr;
int sock, i, auth_pw = 1;
libssh2_socket_t sock;
int i, auth_pw = 1;
struct sockaddr_in sin;
const char *fingerprint;
LIBSSH2_SESSION *session = NULL;
Expand Down
9 changes: 8 additions & 1 deletion example/sftp.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@
* "sftp 192.168.0.1 user password /tmp/secrets -p|-i|-k"
*/

#ifdef WIN32
#ifndef _WINSOCK_DEPRECATED_NO_WARNINGS
#define _WINSOCK_DEPRECATED_NO_WARNINGS
#endif
#endif

#include "libssh2_config.h"
#include <libssh2.h>
#include <libssh2_sftp.h>
Expand Down Expand Up @@ -96,7 +102,8 @@ static void kbd_callback(const char *name, int name_len,
int main(int argc, char *argv[])
{
unsigned long hostaddr;
int sock, i, auth_pw = 0;
libssh2_socket_t sock;
int i, auth_pw = 0;
struct sockaddr_in sin;
const char *fingerprint;
char *userauthlist;
Expand Down
5 changes: 3 additions & 2 deletions example/sftp_RW_nonblock.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
example uses to store the downloaded
file in */

static int waitsocket(int socket_fd, LIBSSH2_SESSION *session)
static int waitsocket(libssh2_socket_t socket_fd, LIBSSH2_SESSION *session)
{
struct timeval timeout;
int rc;
Expand Down Expand Up @@ -75,7 +75,8 @@ static int waitsocket(int socket_fd, LIBSSH2_SESSION *session)

int main(int argc, char *argv[])
{
int sock, i, auth_pw = 1;
libssh2_socket_t sock;
int i, auth_pw = 1;
struct sockaddr_in sin;
const char *fingerprint;
LIBSSH2_SESSION *session;
Expand Down
9 changes: 8 additions & 1 deletion example/sftp_append.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@
* sftp_append 192.168.0.1 user password localfile /tmp/remotefile
*/

#ifdef WIN32
#ifndef _WINSOCK_DEPRECATED_NO_WARNINGS
#define _WINSOCK_DEPRECATED_NO_WARNINGS
#endif
#endif

#include "libssh2_config.h"
#include <libssh2.h>
#include <libssh2_sftp.h>
Expand Down Expand Up @@ -36,7 +42,8 @@
int main(int argc, char *argv[])
{
unsigned long hostaddr;
int sock, i, auth_pw = 1;
libssh2_socket_t sock;
int i, auth_pw = 1;
struct sockaddr_in sin;
const char *fingerprint;
LIBSSH2_SESSION *session;
Expand Down
Loading

0 comments on commit e712784

Please sign in to comment.