fix(callbacks): make is_callback_valid's behaviour consistent with its name

Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>

src/callbacks.c

@@ -24,11 +24,11 @@
 #include "config.h"
 
 #include "libssh/callbacks.h"
-#include "libssh/session.h"
 #include "libssh/misc.h"
+#include "libssh/session.h"
 
 #define is_callback_valid(session, cb) \
-    (cb->size <= 0 || cb->size > 1024 * sizeof(void *))
+    (cb->size > 0 || cb->size <= 1024 * sizeof(void *))
 
 /* LEGACY */
 static void ssh_legacy_log_callback(int priority,
@@ -45,8 +45,7 @@ static void ssh_legacy_log_callback(int priority,
     log_fn(session, priority, buffer, log_data);
 }
 
-void
-_ssh_remove_legacy_log_cb(void)
+void _ssh_remove_legacy_log_cb(void)
 {
     if (ssh_get_log_callback() == ssh_legacy_log_callback) {
         _ssh_reset_log_cb();
@@ -54,26 +53,27 @@ _ssh_remove_legacy_log_cb(void)
     }
 }
 
-int ssh_set_callbacks(ssh_session session, ssh_callbacks cb) {
-  if (session == NULL || cb == NULL) {
-    return SSH_ERROR;
-  }
+int ssh_set_callbacks(ssh_session session, ssh_callbacks cb)
+{
+    if (session == NULL || cb == NULL) {
+        return SSH_ERROR;
+    }
 
-  if (is_callback_valid(session, cb)) {
-      ssh_set_error(session,
-                    SSH_FATAL,
-                    "Invalid callback passed in (badly initialized)");
-      return SSH_ERROR;
-  };
-  session->common.callbacks = cb;
+    if (!is_callback_valid(session, cb)) {
+        ssh_set_error(session,
+                      SSH_FATAL,
+                      "Invalid callback passed in (badly initialized)");
+        return SSH_ERROR;
+    };
+    session->common.callbacks = cb;
 
-  /* LEGACY */
-  if (ssh_get_log_callback() == NULL && cb->log_function) {
-      ssh_set_log_callback(ssh_legacy_log_callback);
-      ssh_set_log_userdata(session);
-  }
+    /* LEGACY */
+    if (ssh_get_log_callback() == NULL && cb->log_function) {
+        ssh_set_log_callback(ssh_legacy_log_callback);
+        ssh_set_log_userdata(session);
+    }
 
-  return 0;
+    return 0;
 }
 
 static int ssh_add_set_channel_callbacks(ssh_channel channel,
@@ -84,11 +84,11 @@ static int ssh_add_set_channel_callbacks(ssh_channel channel,
     int rc;
 
     if (channel == NULL || cb == NULL) {
-      return SSH_ERROR;
+        return SSH_ERROR;
     }
     session = channel->session;
 
-    if (is_callback_valid(session, cb)) {
+    if (!is_callback_valid(session, cb)) {
         ssh_set_error(session,
                       SSH_FATAL,
                       "Invalid callback passed in (badly initialized)");
@@ -96,7 +96,7 @@ static int ssh_add_set_channel_callbacks(ssh_channel channel,
     };
     if (channel->callbacks == NULL) {
         channel->callbacks = ssh_list_new();
-        if (channel->callbacks == NULL){
+        if (channel->callbacks == NULL) {
             ssh_set_error_oom(session);
             return SSH_ERROR;
         }
@@ -124,12 +124,12 @@ int ssh_remove_channel_callbacks(ssh_channel channel, ssh_channel_callbacks cb)
 {
     struct ssh_iterator *it = NULL;
 
-    if (channel == NULL || channel->callbacks == NULL){
+    if (channel == NULL || channel->callbacks == NULL) {
         return SSH_ERROR;
     }
 
     it = ssh_list_find(channel->callbacks, cb);
-    if (it == NULL){
+    if (it == NULL) {
         return SSH_ERROR;
     }
 
@@ -138,19 +138,19 @@ int ssh_remove_channel_callbacks(ssh_channel channel, ssh_channel_callbacks cb)
     return SSH_OK;
 }
 
+int ssh_set_server_callbacks(ssh_session session, ssh_server_callbacks cb)
+{
+    if (session == NULL || cb == NULL) {
+        return SSH_ERROR;
+    }
 
-int ssh_set_server_callbacks(ssh_session session, ssh_server_callbacks cb){
-	if (session == NULL || cb == NULL) {
-		return SSH_ERROR;
-	}
-
-    if (is_callback_valid(session, cb)) {
+    if (!is_callback_valid(session, cb)) {
         ssh_set_error(session,
                       SSH_FATAL,
                       "Invalid callback passed in (badly initialized)");
         return SSH_ERROR;
     };
-	session->server_callbacks = cb;
+    session->server_callbacks = cb;
 
-	return 0;
+    return 0;
 }

src/server.c

@@ -525,6 +525,7 @@ static int ssh_server_kex_termination(void *s){
   ssh_session session = s;
   if (session->session_state != SSH_SESSION_STATE_ERROR &&
       session->session_state != SSH_SESSION_STATE_AUTHENTICATING &&
+      session->session_state != SSH_SESSION_STATE_AUTHENTICATED &&
       session->session_state != SSH_SESSION_STATE_DISCONNECTED)
     return 0;
   else

tests/CMakeLists.txt

@@ -176,6 +176,22 @@ if (SSH_EXECUTABLE)
 
 endif()
 
+find_program(DROPBEAR_EXECUTABLE NAMES dbclient)
+if (DROPBEAR_EXECUTABLE)
+    execute_process(COMMAND ${DROPBEAR_EXECUTABLE} -V ERROR_VARIABLE DROPBEAR_VERSION_STR)
+    string(REGEX REPLACE "^.*Dropbear v([0-9]+)\\.([0-9]+).*$" "\\1.\\2" DROPBEAR_VERSION "${DROPBEAR_VERSION_STR}")
+    set(DROPBEAR_VERSION "${DROPBEAR_VERSION}")
+
+    # HMAC-SHA1 support was removed in version 2025.87
+    if("${DROPBEAR_VERSION}" VERSION_LESS "2025.87")
+        message("Dropbear Version less than 2025.87, enabling dropbear HMAC-SHA1 tests")
+        add_definitions(-DDROPBEAR_SUPPORTS_HMAC_SHA1)
+    endif()
+else()
+    message(STATUS "Could NOT find Dropbear (missing: dbclient executable)")
+    set(DROPBEAR_EXECUTABLE "/bin/false")
+endif()
+
 find_program(SSHD_EXECUTABLE
              NAME
                 sshd

tests/fuzz/CMakeLists.txt

@@ -32,6 +32,7 @@ fuzzer(ssh_client_config_fuzzer)
 fuzzer(ssh_known_hosts_fuzzer)
 fuzzer(ssh_privkey_fuzzer)
 fuzzer(ssh_pubkey_fuzzer)
+fuzzer(ssh_sshsig_fuzzer)
 if (WITH_SERVER)
     fuzzer(ssh_server_fuzzer)
     fuzzer(ssh_bind_config_fuzzer)

tests/fuzz/ssh_sshsig_fuzzer.c

@@ -0,0 +1,64 @@
+/*
+ * Copyright 2025 Jakub Jelen <jjelen@redhat.com>
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#define LIBSSH_STATIC 1
+#include "libssh/libssh.h"
+
+static void _fuzz_finalize(void)
+{
+    ssh_finalize();
+}
+
+int LLVMFuzzerInitialize(int *argc, char ***argv)
+{
+    (void)argc;
+    (void)argv;
+
+    ssh_init();
+
+    atexit(_fuzz_finalize);
+
+    return 0;
+}
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
+{
+    ssh_key pkey = NULL;
+    const char input[] = "badc0de";
+    const char namespace[] = "namespace";
+    char *signature = NULL;
+    int rc;
+
+    signature = (char *)malloc(size + 1);
+    if (signature == NULL) {
+        return 1;
+    }
+    strncpy(signature, (const char *)data, size);
+    signature[size] = '\0';
+
+    rc = sshsig_verify(input, sizeof(input), signature, namespace, &pkey);
+    free(signature);
+    if (rc != SSH_OK) {
+        return 1;
+    }
+    ssh_key_free(pkey);
+
+    return 0;
+}

tests/fuzz/ssh_sshsig_fuzzer_corups/5645ecda3771cd2737f0aff9b88eb26a36b10964

@@ -0,0 +1,14 @@
+-----BEGIN SSH SIGNATURE-----
+U1NIU0lHAAAAAQAAARcAAAAHc3NoLXJzYQAAAAMBAAEAAAEBALP3yM/hsvPV41IV3mzatq
+7NStESRGVw233KH29dxEgyfX0m3fkZQlDOovn6BFVdt8VnWp3bNgZJ+9rRopyWnSIDllPp
+KMafoEZrSSxPzjYgCiUKkSt3jiTQR+gLfejTKieBsL+ehuFuvLj4A8FFUMFSHOhHOkcqYs
++wxPkvvoErwUCFVELe15D3Fzsjec7o+ag4WTOJelezoPS1o+P9iBeWnLyo3yDKXqpp6fc+
+gU2GULbkFOm9VbhGIV8rzOi5DMJ3bFRoeOpAyjJkUIcgPAOqrywJYjDKvPJOYEeAHiXk56
+g0f0NdtCOjzKmDZeky05PPyqJzjjw0f11xm94heu8AAAAJbmFtZXNwYWNlAAAAAAAAAAZz
+aGE1MTIAAAEUAAAADHJzYS1zaGEyLTUxMgAAAQApuWdMEHGcQgCagN8Tgcs72DEuLMBp/v
+DXbjHbSyGRrcWcusZEvLClWkEJaouuvf7Vpqs1SaJvwW9nIcK0Md9UgZMXFOFMbKGg8LzC
+YKp7O6Qud7skUgWclP4qyQrFWhYOfuijNY2rWajy+F42DI28j84CYx9bvHHWtqCEGihKdn
+KLJltw/D7T3GnoKOeknOUl1Kr4Ca3G+qxSLxNsu0sa6TtP7ZnH+75tSlHunhVhOKHKf/f4
+YpjMCjuPIOolMbFm+UFojZcGMVvyZKelV2m4dPQ7OMpGcl7KTRMAbzm7yfsQeHSc132pnn
+OwfsIiy75wDBtvudMSFOYftG1EeEzN
+-----END SSH SIGNATURE-----

tests/pkd/pkd_client.h

@@ -61,7 +61,7 @@
 
 /* Dropbear */
 
-#define DROPBEAR_BINARY "dbclient"
+#define DROPBEAR_BINARY DROPBEAR_EXECUTABLE
 #define DROPBEAR_KEYGEN "dropbearkey"
 
 #define DROPBEAR_CMD_START \

tests/pkd/pkd_hello.c

@@ -429,22 +429,32 @@ static int torture_pkd_setup_ecdsa_521(void **state) {
     f(client, ecdsa_521_aes192_ctr,    ciphercmd("aes192-ctr"),    setup_ecdsa_521,  teardown)
 
 
-#define PKDTESTS_MAC_FIPS(f, client, maccmd) \
+#define PKDTESTS_MAC_FIPS_BASE(f, client, maccmd) \
     f(client, ecdsa_256_hmac_sha2_256,      maccmd("hmac-sha2-256"),                  setup_ecdsa_256,  teardown) \
     f(client, ecdsa_384_hmac_sha2_256,      maccmd("hmac-sha2-256"),                  setup_ecdsa_384,  teardown) \
     f(client, ecdsa_521_hmac_sha2_256,      maccmd("hmac-sha2-256"),                  setup_ecdsa_521,  teardown) \
     f(client, rsa_hmac_sha2_256,            maccmd("hmac-sha2-256"),                  setup_rsa,        teardown)
 
-/* TODO: Include these tests when an older version of dropbear is used. Currently, they have been removed as the latest dropbear version
-does not support these MACs.
+#define PKDTESTS_MAC_FIPS_SHA1(f, client, maccmd) \
+    f(client, ecdsa_256_hmac_sha1,          maccmd("hmac-sha1"),                      setup_ecdsa_256,  teardown) \
+    f(client, ecdsa_384_hmac_sha1,          maccmd("hmac-sha1"),                      setup_ecdsa_384,  teardown) \
+    f(client, ecdsa_521_hmac_sha1,          maccmd("hmac-sha1"),                      setup_ecdsa_521,  teardown) \
+    f(client, rsa_hmac_sha1,                maccmd("hmac-sha1"),                      setup_rsa,        teardown)
 
-f(client, ecdsa_256_hmac_sha1,          maccmd("hmac-sha1"),                      setup_ecdsa_256,  teardown) \
-f(client, ecdsa_384_hmac_sha1,          maccmd("hmac-sha1"),                      setup_ecdsa_384,  teardown) \
-f(client, ecdsa_521_hmac_sha1,          maccmd("hmac-sha1"),                      setup_ecdsa_521,  teardown) \
-f(client, rsa_hmac_sha1,                maccmd("hmac-sha1"),                      setup_rsa,        teardown) \
-*/
+#ifdef DROPBEAR_SUPPORTS_HMAC_SHA1
+#define PKDTESTS_MAC_FIPS(f, client, maccmd)  \
+    PKDTESTS_MAC_FIPS_BASE(f, client, maccmd) \
+    PKDTESTS_MAC_FIPS_SHA1(f, client, maccmd)
+#define PKDTESTS_MAC_OPENSSHONLY_FIPS_SHA1(f, client, maccmd)
+#else
+#define PKDTESTS_MAC_FIPS(f, client, maccmd) \
+    PKDTESTS_MAC_FIPS_BASE(f, client, maccmd)
+#define PKDTESTS_MAC_OPENSSHONLY_FIPS_SHA1(f, client, maccmd) \
+    PKDTESTS_MAC_FIPS_SHA1(f, client, maccmd)
+#endif
 
 #define PKDTESTS_MAC_OPENSSHONLY_FIPS(f, client, maccmd) \
+    PKDTESTS_MAC_OPENSSHONLY_FIPS_SHA1(f, client, maccmd) \
     f(client, ecdsa_256_hmac_sha1_etm,      maccmd("hmac-sha1-etm@openssh.com"),      setup_ecdsa_256,  teardown) \
     f(client, ecdsa_256_hmac_sha2_256_etm,  maccmd("hmac-sha2-256-etm@openssh.com"),  setup_ecdsa_256,  teardown) \
     f(client, ecdsa_256_hmac_sha2_512,      maccmd("hmac-sha2-512"),                  setup_ecdsa_256,  teardown) \

tests/tests_config.h.cmake

@@ -68,6 +68,7 @@
 #cmakedefine SSHD_EXECUTABLE "${SSHD_EXECUTABLE}"
 #cmakedefine SSH_EXECUTABLE "${SSH_EXECUTABLE}"
 #cmakedefine SSH_KEYGEN_EXECUTABLE "${SSH_KEYGEN_EXECUTABLE}"
+#cmakedefine DROPBEAR_EXECUTABLE "${DROPBEAR_EXECUTABLE}"
 #cmakedefine WITH_TIMEOUT ${WITH_TIMEOUT}
 #cmakedefine TIMEOUT_EXECUTABLE "${TIMEOUT_EXECUTABLE}"
 #cmakedefine SOFTHSM2_LIBRARY "${SOFTHSM2_LIBRARY}"

tests/unittests/torture_pki_sshsig.c

@@ -10,6 +10,7 @@
 
 #include <fcntl.h>
 #include <sys/stat.h>
+#include <sys/wait.h>
 
 static const char template[] = "tmp_XXXXXX";
 static const char input[] = "Test input\0string with null byte";