[Spice-commits] 3 commits - server/reds.cpp server/reds-private.h server/tests
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Wed Aug 3 18:04:17 UTC 2022
server/reds-private.h | 2
server/reds.cpp | 124 +++++++++++++++++++++++++++++++-----------
server/tests/Makefile.am | 1
server/tests/pki/dhparams.pem | 13 ++++
server/tests/test-leaks.c | 4 +
5 files changed, 112 insertions(+), 32 deletions(-)
New commits:
commit a225ebe921b7c17a781cc7259884f4076dcca2d8
Author: Frediano Ziglio <freddy77 at gmail.com>
Date: Thu Jul 21 14:38:43 2022 +0100
test-leaks: Load DH parameters to test this part of code
Just loding the file is enough to test code in reds.cpp.
Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am
index 5918cb9d..5a7bd52e 100644
--- a/server/tests/Makefile.am
+++ b/server/tests/Makefile.am
@@ -8,6 +8,7 @@ EXTRA_DIST = \
pki/ca-cert.pem \
pki/server-cert.pem \
pki/server-key.pem \
+ pki/dhparams.pem \
$(NULL)
AM_CPPFLAGS = \
diff --git a/server/tests/pki/dhparams.pem b/server/tests/pki/dhparams.pem
new file mode 100644
index 00000000..92760255
--- /dev/null
+++ b/server/tests/pki/dhparams.pem
@@ -0,0 +1,13 @@
+-----BEGIN DH PARAMETERS-----
+MIICCAKCAgEArKjVOoTXDuKTbh8ne4NPs46AD5npR5N43gQzEFgoDad6AW1nKt1P
+OtSHEhWZMnnqKGjeHsvG7aPaSi/SUj6ypfLLt2Ym6/BkEL/Xn4WIl++4RX/XS1gB
+Mxl4D7TIz7I/DuT8tIDWGdaHFJvFx7CeVCFRHSAOPAPPT2EtQx7Gq10674Gmfinp
+MKWeWOx0VkZJJqnUZryEMWo6G7GgN3+7ZkHeTF+1cQehSs2UB/0Vy/NsS8Spiq3X
+Dq7GNXSVC0zzzpwvE7NFeCU6Zb3FExGb04UIAO8iG2xj7Sv8ZQUHCEVmHh+Q+DX9
+GHHbE0KsBLvbVvWeYjDZK1wlkPVD9vsHWkKcwdPLFfaf6Oc+82cDdSNKetcas9Cs
+JXWMdErAXDq/0XyTHvrM4ZHP6imQbKjRx26Ca3UPYWWKf3q2NDfLDIcdT06uXBjO
+SjwmRKyz/3Ytl2WfhhYicxVaeda9Roks01homHwQrgn+WlrWQJOtj0G6JKgTYqGR
+LuUUi/CAjIhRWF5zTDxtIQp7Tk5ZuS117W/KyWe2cGPq09MH6DxBKei0mI9pDTe6
+CG2D5AvHKkFoJ8GdGVg957kTvjr1t+UxAt1TXkmQERtNZSPCLZie7C/21CnKGhIT
+QsRMk6WSmzvHRQUEbrAfJH1WhV6x8+nrj/OpgucywPLc004hQUKYsMMCAQI=
+-----END DH PARAMETERS-----
diff --git a/server/tests/test-leaks.c b/server/tests/test-leaks.c
index 53e15ba8..94cea57a 100644
--- a/server/tests/test-leaks.c
+++ b/server/tests/test-leaks.c
@@ -56,7 +56,9 @@ static void server_leaks(void)
PKI_DIR "ca-cert.pem",
PKI_DIR "server-cert.pem",
PKI_DIR "server-key.pem",
- NULL, NULL, NULL);
+ NULL,
+ PKI_DIR "dhparams.pem",
+ NULL);
g_assert_cmpint(result, ==, 0);
g_assert_cmpint(spice_server_init(server, core), ==, 0);
commit 2cd6766b0d425a1187e1abee6b62acff2224e300
Author: Frediano Ziglio <freddy77 at gmail.com>
Date: Wed Aug 3 17:51:04 2022 +0100
Adapt to new OpenSSL with less conditional code
Reduce conditional code using new OpenSSL interface and implement
missing APIs.
Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
diff --git a/server/reds-private.h b/server/reds-private.h
index 4e52828d..50e38486 100644
--- a/server/reds-private.h
+++ b/server/reds-private.h
@@ -38,11 +38,7 @@ struct TicketAuthentication {
};
struct TicketInfo {
-#if OPENSSL_VERSION_NUMBER >= 0x30000000L
EVP_PKEY *rsa;
-#else
- RSA *rsa;
-#endif
int rsa_size;
BIGNUM *bn;
SpiceLinkEncryptedTicket encrypted_ticket;
diff --git a/server/reds.cpp b/server/reds.cpp
index a2561c8d..5e9a129a 100644
--- a/server/reds.cpp
+++ b/server/reds.cpp
@@ -272,11 +272,7 @@ static void reds_link_free(RedLinkInfo *link)
link->tiTicketing.bn = nullptr;
if (link->tiTicketing.rsa) {
-#if OPENSSL_VERSION_NUMBER >= 0x30000000L
EVP_PKEY_free(link->tiTicketing.rsa);
-#else
- RSA_free(link->tiTicketing.rsa);
-#endif
link->tiTicketing.rsa = nullptr;
}
@@ -1507,6 +1503,40 @@ static bool red_link_info_test_capability(const RedLinkInfo *link, uint32_t cap)
return test_capability(caps, link->link_mess->num_common_caps, cap);
}
+// Check for EVP_RSA_gen. EVP_RSA_gen was always defined as a macro in <openssl/rsa.h>
+// and it's still so in OpenSSL 3.0 so checking for macro existence is a good way.
+#if OPENSSL_VERSION_NUMBER < 0x30000000L && !defined(EVP_RSA_gen)
+static EVP_PKEY *EVP_RSA_gen(unsigned int bits, BIGNUM *rsa_exponent)
+{
+ RSA *rsa = RSA_new();
+ if (!rsa) {
+ return nullptr;
+ }
+
+ if (RSA_generate_key_ex(rsa, SPICE_TICKET_KEY_PAIR_LENGTH,
+ rsa_exponent, nullptr) != 1) {
+ RSA_free(rsa);
+ return nullptr;
+ }
+
+ EVP_PKEY *pk = EVP_PKEY_new();
+ if (!pk) {
+ RSA_free(rsa);
+ return nullptr;
+ }
+
+ if (!EVP_PKEY_set1_RSA(pk, rsa)) {
+ EVP_PKEY_free(pk);
+ RSA_free(rsa);
+ return nullptr;
+ }
+
+ RSA_free(rsa);
+ return pk;
+}
+
+#define EVP_RSA_gen(bits) EVP_RSA_gen(bits, (link->tiTicketing.bn))
+#endif
static bool reds_send_link_ack(RedsState *reds, RedLinkInfo *link)
{
@@ -1558,7 +1588,6 @@ static bool reds_send_link_ack(RedsState *reds, RedLinkInfo *link)
return FALSE;
}
-#if OPENSSL_VERSION_NUMBER >= 0x30000000L
link->tiTicketing.rsa = EVP_RSA_gen(SPICE_TICKET_KEY_PAIR_LENGTH);
if (!link->tiTicketing.rsa) {
spice_warning("Failed to generate %d bits RSA key",
@@ -1572,25 +1601,6 @@ static bool reds_send_link_ack(RedsState *reds, RedLinkInfo *link)
red_dump_openssl_errors();
goto end;
}
-#else
- if (!(link->tiTicketing.rsa = RSA_new())) {
- spice_warning("RSA new failed");
- red_dump_openssl_errors();
- return FALSE;
- }
-
- if (RSA_generate_key_ex(link->tiTicketing.rsa,
- SPICE_TICKET_KEY_PAIR_LENGTH,
- link->tiTicketing.bn,
- nullptr) != 1) {
- spice_warning("Failed to generate %d bits RSA key",
- SPICE_TICKET_KEY_PAIR_LENGTH);
- red_dump_openssl_errors();
- goto end;
- }
- link->tiTicketing.rsa_size = RSA_size(link->tiTicketing.rsa);
- i2d_RSA_PUBKEY_bio(bio, link->tiTicketing.rsa);
-#endif
BIO_get_mem_ptr(bio, &bmBuf);
memcpy(msg.ack.pub_key, bmBuf->data, sizeof(msg.ack.pub_key));
} else {
@@ -2040,10 +2050,7 @@ static void reds_handle_ticket(void *opaque)
auto link = static_cast<RedLinkInfo *>(opaque);
RedsState *reds = link->reds;
char *password;
- int password_size;
-#if OPENSSL_VERSION_NUMBER >= 0x30000000L
EVP_PKEY_CTX *ctx = nullptr;
-#endif
if (link->tiTicketing.rsa_size < SPICE_MAX_PASSWORD_LENGTH) {
spice_warning("RSA modulus size is smaller than SPICE_MAX_PASSWORD_LENGTH (%d < %d), "
@@ -2053,12 +2060,11 @@ static void reds_handle_ticket(void *opaque)
password = static_cast<char *>(alloca(link->tiTicketing.rsa_size + 1));
-#if OPENSSL_VERSION_NUMBER >= 0x30000000L
size_t len = 0;
- ctx = EVP_PKEY_CTX_new_from_pkey(nullptr, link->tiTicketing.rsa, nullptr);
+ ctx = EVP_PKEY_CTX_new(link->tiTicketing.rsa, nullptr);
- if (EVP_PKEY_decrypt_init(ctx) <= 0) {
+ if (!ctx || EVP_PKEY_decrypt_init(ctx) <= 0) {
spice_warning("failed to initialize decrypt");
red_dump_openssl_errors();
goto error;
@@ -2076,22 +2082,8 @@ static void reds_handle_ticket(void *opaque)
red_dump_openssl_errors();
goto error;
}
- EVP_PKEY_CTX_free(ctx);
- password_size = len;
-#else
- password_size =
- RSA_private_decrypt(link->tiTicketing.rsa_size,
- link->tiTicketing.encrypted_ticket.encrypted_data,
- reinterpret_cast<unsigned char *>(password),
- link->tiTicketing.rsa, RSA_PKCS1_OAEP_PADDING);
-#endif
- if (password_size == -1) {
- spice_warning("failed to decrypt RSA encrypted password");
- red_dump_openssl_errors();
- goto error;
- }
- password[password_size] = '\0';
+ password[len] = '\0';
if (reds->config->ticketing_enabled && !link->skip_auth) {
time_t ltime;
@@ -2117,15 +2109,14 @@ static void reds_handle_ticket(void *opaque)
}
}
+ EVP_PKEY_CTX_free(ctx);
reds_handle_link(link);
return;
error:
-#if OPENSSL_VERSION_NUMBER >= 0x30000000L
if (ctx != nullptr) {
EVP_PKEY_CTX_free(ctx);
}
-#endif
reds_send_link_result(link, SPICE_LINK_ERR_PERMISSION_DENIED);
reds_link_free(link);
}
@@ -2689,15 +2680,25 @@ static int reds_init_net(RedsState *reds)
return 0;
}
-static int load_dh_params(SSL_CTX *ctx, char *file)
+#if OPENSSL_VERSION_NUMBER < 0x30000000L
+static inline int SSL_CTX_set0_tmp_dh_pkey(SSL_CTX *ctx, EVP_PKEY *dhpkey)
{
-#if OPENSSL_VERSION_NUMBER >= 0x30000000L
- EVP_PKEY *dh = nullptr;
-#else
- DH *dh = nullptr;
+ DH *dh = EVP_PKEY_get1_DH(dhpkey);
+ if (dh == nullptr) {
+ return 0;
+ }
+ int ret = SSL_CTX_set_tmp_dh(ctx, dh);
+ DH_free(dh);
+ if (ret > 0) {
+ EVP_PKEY_free(dhpkey);
+ }
+ return ret;
+}
#endif
+
+static int load_dh_params(SSL_CTX *ctx, char *file)
+{
BIO *bio;
- int ret;
if ((bio = BIO_new_file(file, "r")) == nullptr) {
spice_warning("Could not open DH file");
@@ -2705,11 +2706,7 @@ static int load_dh_params(SSL_CTX *ctx, char *file)
return -1;
}
-#if OPENSSL_VERSION_NUMBER >= 0x30000000L
- dh = PEM_read_bio_Parameters(bio, &dh);
-#else
- dh = PEM_read_bio_DHparams(bio, nullptr, nullptr, nullptr);
-#endif
+ auto dh = PEM_read_bio_Parameters(bio, nullptr);
BIO_free(bio);
if (dh == nullptr) {
@@ -2718,16 +2715,9 @@ static int load_dh_params(SSL_CTX *ctx, char *file)
return -1;
}
-#if OPENSSL_VERSION_NUMBER >= 0x30000000L
- ret = SSL_CTX_set0_tmp_dh_pkey(ctx, dh);
-#else
- ret = SSL_CTX_set_tmp_dh(ctx, dh);
- DH_free(dh);
-#endif
- if (ret < 0) {
-#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ auto ret = SSL_CTX_set0_tmp_dh_pkey(ctx, dh);
+ if (ret <= 0) {
EVP_PKEY_free(dh);
-#endif
spice_warning("Could not set DH params");
red_dump_openssl_errors();
return -1;
commit dc40a18092808e2ce0ce6e352a446bccf21b49ca
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date: Tue Jul 19 14:23:47 2022 +0400
Fix OpenSSL 3.0 API deprecations
Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
diff --git a/server/reds-private.h b/server/reds-private.h
index fc867fa3..4e52828d 100644
--- a/server/reds-private.h
+++ b/server/reds-private.h
@@ -38,7 +38,11 @@ struct TicketAuthentication {
};
struct TicketInfo {
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ EVP_PKEY *rsa;
+#else
RSA *rsa;
+#endif
int rsa_size;
BIGNUM *bn;
SpiceLinkEncryptedTicket encrypted_ticket;
diff --git a/server/reds.cpp b/server/reds.cpp
index 49113316..a2561c8d 100644
--- a/server/reds.cpp
+++ b/server/reds.cpp
@@ -272,7 +272,11 @@ static void reds_link_free(RedLinkInfo *link)
link->tiTicketing.bn = nullptr;
if (link->tiTicketing.rsa) {
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ EVP_PKEY_free(link->tiTicketing.rsa);
+#else
RSA_free(link->tiTicketing.rsa);
+#endif
link->tiTicketing.rsa = nullptr;
}
@@ -1548,14 +1552,29 @@ static bool reds_send_link_ack(RedsState *reds, RedLinkInfo *link)
msg.ack.caps_offset = GUINT32_TO_LE(sizeof(SpiceLinkReply));
if (!reds->config->sasl_enabled
|| !red_link_info_test_capability(link, SPICE_COMMON_CAP_AUTH_SASL)) {
- if (!(link->tiTicketing.rsa = RSA_new())) {
- spice_warning("RSA new failed");
+ if (!(bio = BIO_new(BIO_s_mem()))) {
+ spice_warning("BIO new failed");
red_dump_openssl_errors();
return FALSE;
}
- if (!(bio = BIO_new(BIO_s_mem()))) {
- spice_warning("BIO new failed");
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ link->tiTicketing.rsa = EVP_RSA_gen(SPICE_TICKET_KEY_PAIR_LENGTH);
+ if (!link->tiTicketing.rsa) {
+ spice_warning("Failed to generate %d bits RSA key",
+ SPICE_TICKET_KEY_PAIR_LENGTH);
+ red_dump_openssl_errors();
+ goto end;
+ }
+ link->tiTicketing.rsa_size = SPICE_TICKET_KEY_PAIR_LENGTH / 8;
+ if (i2d_PUBKEY_bio(bio, link->tiTicketing.rsa) <= 0) {
+ spice_warning("Failed to get public key");
+ red_dump_openssl_errors();
+ goto end;
+ }
+#else
+ if (!(link->tiTicketing.rsa = RSA_new())) {
+ spice_warning("RSA new failed");
red_dump_openssl_errors();
return FALSE;
}
@@ -1570,8 +1589,8 @@ static bool reds_send_link_ack(RedsState *reds, RedLinkInfo *link)
goto end;
}
link->tiTicketing.rsa_size = RSA_size(link->tiTicketing.rsa);
-
i2d_RSA_PUBKEY_bio(bio, link->tiTicketing.rsa);
+#endif
BIO_get_mem_ptr(bio, &bmBuf);
memcpy(msg.ack.pub_key, bmBuf->data, sizeof(msg.ack.pub_key));
} else {
@@ -2022,19 +2041,51 @@ static void reds_handle_ticket(void *opaque)
RedsState *reds = link->reds;
char *password;
int password_size;
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ EVP_PKEY_CTX *ctx = nullptr;
+#endif
- if (RSA_size(link->tiTicketing.rsa) < SPICE_MAX_PASSWORD_LENGTH) {
+ if (link->tiTicketing.rsa_size < SPICE_MAX_PASSWORD_LENGTH) {
spice_warning("RSA modulus size is smaller than SPICE_MAX_PASSWORD_LENGTH (%d < %d), "
"SPICE ticket sent from client may be truncated",
- RSA_size(link->tiTicketing.rsa), SPICE_MAX_PASSWORD_LENGTH);
+ link->tiTicketing.rsa_size, SPICE_MAX_PASSWORD_LENGTH);
}
- password = static_cast<char *>(alloca(RSA_size(link->tiTicketing.rsa) + 1));
+ password = static_cast<char *>(alloca(link->tiTicketing.rsa_size + 1));
+
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ size_t len = 0;
+
+ ctx = EVP_PKEY_CTX_new_from_pkey(nullptr, link->tiTicketing.rsa, nullptr);
+
+ if (EVP_PKEY_decrypt_init(ctx) <= 0) {
+ spice_warning("failed to initialize decrypt");
+ red_dump_openssl_errors();
+ goto error;
+ }
+ if (EVP_PKEY_CTX_set_rsa_padding(ctx, RSA_PKCS1_OAEP_PADDING) <= 0) {
+ spice_warning("failed to set OAEP padding");
+ red_dump_openssl_errors();
+ goto error;
+ }
+
+ len = link->tiTicketing.rsa_size;
+ if (EVP_PKEY_decrypt(ctx, reinterpret_cast<unsigned char *>(password), &len,
+ link->tiTicketing.encrypted_ticket.encrypted_data, link->tiTicketing.rsa_size) <= 0) {
+ spice_warning("failed to decrypt RSA encrypted password");
+ red_dump_openssl_errors();
+ goto error;
+ }
+ EVP_PKEY_CTX_free(ctx);
+
+ password_size = len;
+#else
password_size =
RSA_private_decrypt(link->tiTicketing.rsa_size,
link->tiTicketing.encrypted_ticket.encrypted_data,
reinterpret_cast<unsigned char *>(password),
link->tiTicketing.rsa, RSA_PKCS1_OAEP_PADDING);
+#endif
if (password_size == -1) {
spice_warning("failed to decrypt RSA encrypted password");
red_dump_openssl_errors();
@@ -2070,6 +2121,11 @@ static void reds_handle_ticket(void *opaque)
return;
error:
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ if (ctx != nullptr) {
+ EVP_PKEY_CTX_free(ctx);
+ }
+#endif
reds_send_link_result(link, SPICE_LINK_ERR_PERMISSION_DENIED);
reds_link_free(link);
}
@@ -2635,8 +2691,13 @@ static int reds_init_net(RedsState *reds)
static int load_dh_params(SSL_CTX *ctx, char *file)
{
- DH *ret = nullptr;
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ EVP_PKEY *dh = nullptr;
+#else
+ DH *dh = nullptr;
+#endif
BIO *bio;
+ int ret;
if ((bio = BIO_new_file(file, "r")) == nullptr) {
spice_warning("Could not open DH file");
@@ -2644,16 +2705,29 @@ static int load_dh_params(SSL_CTX *ctx, char *file)
return -1;
}
- ret = PEM_read_bio_DHparams(bio, nullptr, nullptr, nullptr);
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ dh = PEM_read_bio_Parameters(bio, &dh);
+#else
+ dh = PEM_read_bio_DHparams(bio, nullptr, nullptr, nullptr);
+#endif
+
BIO_free(bio);
- if (ret == nullptr) {
+ if (dh == nullptr) {
spice_warning("Could not read DH params");
red_dump_openssl_errors();
return -1;
}
-
- if (SSL_CTX_set_tmp_dh(ctx, ret) < 0) {
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ ret = SSL_CTX_set0_tmp_dh_pkey(ctx, dh);
+#else
+ ret = SSL_CTX_set_tmp_dh(ctx, dh);
+ DH_free(dh);
+#endif
+ if (ret < 0) {
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ EVP_PKEY_free(dh);
+#endif
spice_warning("Could not set DH params");
red_dump_openssl_errors();
return -1;
More information about the Spice-commits
mailing list