Re: [PATCH RESEND] Don't set SpiceLinkReply::pub_key if client advertises SASL cap

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



ack

On Tue, Oct 21, 2014 at 3:54 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
If the client advertises the SASL cap, it means it guarantees it will be
able to use SASL if the server supports, and that it does not need a valid
SpiceLinkReply::pub_key field when using SASL.

When the client cap is set, we thus don't need to create a RSA public key
if SASL is enabled server side.

The reason for needing client guarantees about not looking at the pub_key
field is that its presence and size is hardcoded in the protocol, but in
some hardened setups (using fips mode), generating a RSA 1024 bit key as
expected is forbidden and fails. With this new capability, the server
knows the client will be able to handle SASL if needed, and can skip
the generation of the key altogether. This means that on the setups
described above, SASL authentication has to be used.
---
Hey,

This is a resend of http://lists.freedesktop.org/archives/spice-devel/2014-March/016419.html
No changes, rebased on top of git master. The previous 2 patches in the series got ACK'ed.

Christophe


 server/reds.c | 61 +++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 38 insertions(+), 23 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index 5a95ba0..7ecea13 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1352,7 +1352,7 @@ static int reds_send_link_ack(RedLinkInfo *link)
     RedChannel *channel;
     RedChannelCapabilities *channel_caps;
     BUF_MEM *bmBuf;
-    BIO *bio;
+    BIO *bio = NULL;
     int ret = FALSE;

     header.magic = SPICE_MAGIC;
@@ -1377,31 +1377,45 @@ static int reds_send_link_ack(RedLinkInfo *link)
     ack.num_channel_caps = channel_caps->num_caps;
     header.size += (ack.num_common_caps + ack.num_channel_caps) * sizeof(uint32_t);
     ack.caps_offset = sizeof(SpiceLinkReply);
+    if (!sasl_enabled
+        || !red_link_info_test_capability(link, SPICE_COMMON_CAP_AUTH_SASL)) {
+        if (!(link->tiTicketing.rsa = RSA_new())) {
+            spice_warning("RSA new failed");
+            return FALSE;
+        }

-    if (!(link->tiTicketing.rsa = RSA_new())) {
-        spice_warning("RSA new failed");
-        return FALSE;
-    }
+        if (!(bio = BIO_new(BIO_s_mem()))) {
+            spice_warning("BIO new failed");
+            return FALSE;
+        }

-    if (!(bio = BIO_new(BIO_s_mem()))) {
-        spice_warning("BIO new failed");
-        return FALSE;
-    }
+        if (RSA_generate_key_ex(link->tiTicketing.rsa,
+                                SPICE_TICKET_KEY_PAIR_LENGTH,
+                                link->tiTicketing.bn,
+                                NULL) != 1) {
+            spice_warning("Failed to generate %d bits RSA key: %s",
+                          SPICE_TICKET_KEY_PAIR_LENGTH,
+                          ERR_error_string(ERR_get_error(), NULL));
+            goto end;
+        }
+        link->tiTicketing.rsa_size = RSA_size(link->tiTicketing.rsa);

-    if (RSA_generate_key_ex(link->tiTicketing.rsa,
-                            SPICE_TICKET_KEY_PAIR_LENGTH,
-                            link->tiTicketing.bn,
-                            NULL) != 1) {
-        spice_warning("Failed to generate %d bits RSA key: %s",
-                      SPICE_TICKET_KEY_PAIR_LENGTH,
-                      ERR_error_string(ERR_get_error(), NULL));
-        goto end;
+        i2d_RSA_PUBKEY_bio(bio, link->tiTicketing.rsa);
+        BIO_get_mem_ptr(bio, &bmBuf);
+        memcpy(ack.pub_key, bmBuf->data, sizeof(ack.pub_key));
+    } else {
+        /* if the client sets the AUTH_SASL cap, it indicates that it
+         * supports SASL, and will use it if the server supports SASL as
+         * well. Moreover, a client setting the AUTH_SASL cap also
+         * indicates that it will not try using the RSA-related content
+         * in the SpiceLinkReply message, so we don't need to initialize
+         * it. Reason to avoid this is to fix auth in fips mode where
+         * the generation of a 1024 bit RSA key as we are trying to do
+         * will fail.
+         */
+        spice_warning("not initialising RSA key");
+        memset(ack.pub_key, '\0', sizeof(ack.pub_key));
     }
-    link->tiTicketing.rsa_size = RSA_size(link->tiTicketing.rsa);
-
-    i2d_RSA_PUBKEY_bio(bio, link->tiTicketing.rsa);
-    BIO_get_mem_ptr(bio, &bmBuf);
-    memcpy(ack.pub_key, bmBuf->data, sizeof(ack.pub_key));

     if (!reds_stream_write_all(link->stream, &header, sizeof(header)))
         goto end;
@@ -1415,7 +1429,8 @@ static int reds_send_link_ack(RedLinkInfo *link)
     ret = TRUE;

 end:
-    BIO_free(bio);
+    if (bio != NULL)
+        BIO_free(bio);
     return ret;
 }

--
2.1.0

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel



--
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]