Compare commits

...

4 Commits

Author SHA1 Message Date
Praneeth Sarode
09155adb19 tests(string): add unit tests for ssh_string_cmp function
Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
2025-07-30 16:06:32 +02:00
Praneeth Sarode
95f8cbc7f0 feat(string): add ssh_string_cmp function for comparing ssh_strings
Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
2025-07-30 16:06:31 +02:00
Praneeth Sarode
3423399f98 fix(pki): remove redundant key type_c assignment in pki_import_pubkey_buffer
We already assign the correct key type_c using ssh_key_type_to_char before this point.

Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
2025-07-30 16:05:52 +02:00
Praneeth Sarode
ccbec9c275 fix(pki): remove redundant key type_c assignment in build pubkey and privkey functions
Whenever the pki_pubkey_build_ecdsa and pki_privkey_build_ecdsa functions are called, the key type assignment is already done. So, we don't need to assign it again. Moreover, because the pki_key_ecdsa_nid_to_name function was used, for key types like the SSH_KEYTYPE_SK_ECDSA, we assign the wrong type string to the key, based on the nid.

Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
2025-07-30 16:05:51 +02:00
8 changed files with 181 additions and 12 deletions

View File

@@ -851,6 +851,7 @@ LIBSSH_API char *ssh_string_to_char(ssh_string str);
#define SSH_STRING_FREE_CHAR(x) \
do { if ((x) != NULL) { ssh_string_free_char(x); x = NULL; } } while(0)
LIBSSH_API void ssh_string_free_char(char *s);
LIBSSH_API int ssh_string_cmp(ssh_string s1, ssh_string s2);
LIBSSH_API int ssh_getpass(const char *prompt, char *buf, size_t len, int echo,
int verify);

View File

@@ -491,5 +491,6 @@ LIBSSH_AFTER_4_10_0
ssh_get_supported_methods;
sshsig_sign;
sshsig_verify;
ssh_string_cmp;
} LIBSSH_4_10_0;

View File

@@ -1497,11 +1497,6 @@ static int pki_import_pubkey_buffer(ssh_buffer buffer,
goto fail;
}
/* Update key type */
if (type == SSH_KEYTYPE_ECDSA) {
key->type_c = ssh_pki_key_ecdsa_name(key);
}
/* Unpack SK specific parameters */
if (type == SSH_KEYTYPE_SK_ECDSA) {
ssh_string application = ssh_buffer_get_ssh_string(buffer);

View File

@@ -227,7 +227,6 @@ int pki_privkey_build_ecdsa(ssh_key key, int nid, ssh_string e, ssh_string exp)
}
key->ecdsa_nid = nid;
key->type_c = pki_key_ecdsa_nid_to_name(nid);
#if OPENSSL_VERSION_NUMBER < 0x30000000L
ecdsa = EC_KEY_new_by_curve_name(key->ecdsa_nid);
@@ -341,7 +340,6 @@ int pki_pubkey_build_ecdsa(ssh_key key, int nid, ssh_string e)
#endif /* OPENSSL_VERSION_NUMBER */
key->ecdsa_nid = nid;
key->type_c = pki_key_ecdsa_nid_to_name(nid);
#if OPENSSL_VERSION_NUMBER < 0x30000000L
ecdsa = EC_KEY_new_by_curve_name(key->ecdsa_nid);

View File

@@ -1100,7 +1100,6 @@ int pki_privkey_build_ecdsa(ssh_key key, int nid, ssh_string e, ssh_string exp)
gpg_error_t err;
key->ecdsa_nid = nid;
key->type_c = pki_key_ecdsa_nid_to_name(nid);
err = gcry_sexp_build(&key->ecdsa,
NULL,
@@ -1122,7 +1121,6 @@ int pki_pubkey_build_ecdsa(ssh_key key, int nid, ssh_string e)
gpg_error_t err;
key->ecdsa_nid = nid;
key->type_c = pki_key_ecdsa_nid_to_name(nid);
err = gcry_sexp_build(&key->ecdsa,
NULL,

View File

@@ -1802,7 +1802,6 @@ int pki_privkey_build_ecdsa(ssh_key key, int nid, ssh_string e, ssh_string exp)
mbedtls_ecp_point Q;
key->ecdsa_nid = nid;
key->type_c = pki_key_ecdsa_nid_to_name(nid);
key->ecdsa = malloc(sizeof(mbedtls_ecdsa_context));
if (key->ecdsa == NULL) {
@@ -1870,7 +1869,6 @@ int pki_pubkey_build_ecdsa(ssh_key key, int nid, ssh_string e)
mbedtls_ecp_point Q;
key->ecdsa_nid = nid;
key->type_c = pki_key_ecdsa_nid_to_name(nid);
key->ecdsa = malloc(sizeof(mbedtls_ecdsa_context));
if (key->ecdsa == NULL) {

View File

@@ -27,8 +27,8 @@
#include <limits.h>
#ifndef _WIN32
#include <netinet/in.h>
#include <arpa/inet.h>
#include <netinet/in.h>
#endif
#include "libssh/priv.h"
@@ -244,6 +244,56 @@ struct ssh_string_struct *ssh_string_copy(struct ssh_string_struct *s)
return new;
}
/**
* @brief Compare two SSH strings.
*
* @param[in] s1 The first SSH string to compare.
* @param[in] s2 The second SSH string to compare.
*
* @return 0 if the strings are equal,
* < 0 if s1 is less than s2,
* > 0 if s1 is greater than s2.
*/
int ssh_string_cmp(struct ssh_string_struct *s1, struct ssh_string_struct *s2)
{
size_t len1, len2, min_len;
int cmp;
/* Both are NULL */
if (s1 == NULL && s2 == NULL) {
return 0;
}
/* Only one is NULL - NULL is considered "less than" non-NULL */
if (s1 == NULL) {
return -1;
} else if (s2 == NULL) {
return 1;
}
/* Get lengths */
len1 = ssh_string_len(s1);
len2 = ssh_string_len(s2);
min_len = MIN(len1, len2);
/* Compare data up to the shorter length */
if (min_len > 0) {
cmp = memcmp(s1->data, s2->data, min_len);
if (cmp != 0) {
return cmp;
}
}
/* If common prefix is equal, compare lengths */
if (len1 < len2) {
return -1;
} else if (len1 > len2) {
return 1;
}
return 0;
}
/**
* @brief Destroy the data in a string so it couldn't appear in a core dump.
*

View File

@@ -247,6 +247,133 @@ static void torture_ssh_string_burn(void **state)
ssh_string_free(str);
}
static void torture_ssh_string_cmp(void **state)
{
struct ssh_string_struct *str1 = NULL, *str2 = NULL;
const char *test_string1 = "Hello, World!";
const char *test_string2 = "Hello, libssh";
const char *test_string3 = "Hello";
const char *test_string4 = "Apple";
const char data1[] = "Hello\x00World!";
const char data2[] = "Hello\x00libssh";
const char data3[] = "Hello";
int rc;
(void)state;
/* Test comparing two NULL strings - should be equal */
assert_int_equal(ssh_string_cmp(NULL, NULL), 0);
/* Test comparing NULL with non-NULL string - NULL should be less */
str1 = ssh_string_from_char(test_string1);
assert_non_null(str1);
assert_true(ssh_string_cmp(NULL, str1) < 0);
assert_true(ssh_string_cmp(str1, NULL) > 0);
ssh_string_free(str1);
/* Test comparing empty strings */
str1 = ssh_string_from_char("");
str2 = ssh_string_from_char("");
assert_non_null(str1);
assert_non_null(str2);
/* Both empty strings should be equal */
assert_int_equal(ssh_string_cmp(str1, str2), 0);
ssh_string_free(str1);
ssh_string_free(str2);
/* Test comparing empty string with non-empty string */
str1 = ssh_string_from_char("");
str2 = ssh_string_from_char("test");
assert_non_null(str1);
assert_non_null(str2);
/* Empty string should be less than non-empty string */
assert_true(ssh_string_cmp(str1, str2) < 0);
assert_true(ssh_string_cmp(str2, str1) > 0);
ssh_string_free(str1);
ssh_string_free(str2);
/* Test comparing strings where one is a prefix of another */
str1 = ssh_string_from_char(test_string1); /* "Hello, World!" */
str2 = ssh_string_from_char(test_string3); /* "Hello" - prefix */
assert_non_null(str1);
assert_non_null(str2);
/* "Hello" is shorter and a prefix, so it should be < "Hello, World!" */
assert_true(ssh_string_cmp(str2, str1) < 0);
assert_true(ssh_string_cmp(str1, str2) > 0);
ssh_string_free(str1);
ssh_string_free(str2);
/* Test comparing different strings with same length */
str1 = ssh_string_from_char(test_string1); /* "Hello, World!" */
str2 = ssh_string_from_char(test_string2); /* "Hello, libssh" */
assert_non_null(str1);
assert_non_null(str2);
/* "Hello, World!" vs "Hello, libssh" - 'W' < 'l' */
assert_true(ssh_string_cmp(str1, str2) < 0);
assert_true(ssh_string_cmp(str2, str1) > 0);
ssh_string_free(str1);
ssh_string_free(str2);
/* Test comparing strings with different lengths and different characters */
str1 = ssh_string_from_char(test_string1); /* "Hello, World!" */
str2 = ssh_string_from_char(test_string4); /* "Apple" */
assert_non_null(str1);
assert_non_null(str2);
/* 'A' < 'H' so "Apple" < "Hello, World!" */
assert_true(ssh_string_cmp(str2, str1) < 0);
assert_true(ssh_string_cmp(str1, str2) > 0);
ssh_string_free(str1);
ssh_string_free(str2);
/* Test comparing identical strings - should be equal */
str1 = ssh_string_from_char(test_string1);
str2 = ssh_string_from_char(test_string1);
assert_non_null(str1);
assert_non_null(str2);
assert_int_equal(ssh_string_cmp(str1, str2), 0);
assert_int_equal(ssh_string_cmp(str2, str1), 0);
ssh_string_free(str1);
ssh_string_free(str2);
/* Test comparing strings with embedded null characters */
str1 = ssh_string_new(sizeof(data1)); /* "Hello\x00World!" */
str2 = ssh_string_new(sizeof(data3)); /* "Hello" */
assert_non_null(str1);
assert_non_null(str2);
rc = ssh_string_fill(str1, data1, sizeof(data1));
assert_int_equal(rc, 0);
rc = ssh_string_fill(str2, data3, sizeof(data3));
assert_int_equal(rc, 0);
/* "Hello\x00World!" > "Hello" because its length is greater */
assert_true(ssh_string_cmp(str1, str2) > 0); /* data1 > data3 */
assert_true(ssh_string_cmp(str2, str1) < 0); /* data3 < data1 */
ssh_string_free(str1);
ssh_string_free(str2);
/* Comparing binary strings with same length, but different characters */
str1 = ssh_string_new(sizeof(data1)); /* "Hello\x00World!" */
str2 = ssh_string_new(sizeof(data2)); /* "Hello\x00libssh" */
assert_non_null(str1);
assert_non_null(str2);
rc = ssh_string_fill(str1, data1, sizeof(data1));
assert_int_equal(rc, 0);
rc = ssh_string_fill(str2, data2, sizeof(data2));
assert_int_equal(rc, 0);
/* 'W' < 'l' so str1 < str2 */
assert_true(ssh_string_cmp(str1, str2) < 0); /* data1 < data2 */
assert_true(ssh_string_cmp(str2, str1) > 0); /* data2 > data1 */
ssh_string_free(str1);
ssh_string_free(str2);
}
int torture_run_tests(void)
{
int rc;
@@ -257,6 +384,7 @@ int torture_run_tests(void)
cmocka_unit_test(torture_ssh_string_to_char),
cmocka_unit_test(torture_ssh_string_copy),
cmocka_unit_test(torture_ssh_string_burn),
cmocka_unit_test(torture_ssh_string_cmp),
};
ssh_init();