[Spice-commits] 3 commits - server/red_channel.c server/red_channel.h server/reds.c

Christophe Fergau teuf at kemper.freedesktop.org
Mon Nov 24 08:43:32 PST 2014


 server/red_channel.c |    2 -
 server/red_channel.h |    2 -
 server/reds.c        |   82 ++++++++++++++++++++++++++++++++++-----------------
 3 files changed, 58 insertions(+), 28 deletions(-)

New commits:
commit 1ac4dfb3170bd970c90ff970e9f93775867b83e5
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Mar 13 16:16:00 2014 +0100

    Don't set SpiceLinkReply::pub_key if client advertises SASL cap
    
    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.

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;
 }
 
commit 24bebaa8558f41224cea7f730933bf25899a12a8
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Wed Mar 5 11:59:37 2014 +0100

    Introduce red_link_info_test_capability()
    
    This just hides a bit of pointer arithmetic away from reds_send_link_ack.
    This helper will be used in the next commits.

diff --git a/server/reds.c b/server/reds.c
index ef7ff62..5a95ba0 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1329,6 +1329,22 @@ static void reds_channel_init_auth_caps(RedLinkInfo *link, RedChannel *channel)
     red_channel_set_common_cap(channel, SPICE_COMMON_CAP_PROTOCOL_AUTH_SELECTION);
 }
 
+
+static const uint32_t *red_link_info_get_caps(const RedLinkInfo *link)
+{
+    const uint8_t *caps_start = (const uint8_t *)link->link_mess;
+
+    return (const uint32_t *)(caps_start + link->link_mess->caps_offset);
+}
+
+static bool red_link_info_test_capability(const RedLinkInfo *link, uint32_t cap)
+{
+    const uint32_t *caps = red_link_info_get_caps(link);
+
+    return test_capability(caps, link->link_mess->num_common_caps, cap);
+}
+
+
 static int reds_send_link_ack(RedLinkInfo *link)
 {
     SpiceLinkHeader header;
@@ -2050,7 +2066,6 @@ static void reds_handle_read_link_done(void *opaque)
     RedLinkInfo *link = (RedLinkInfo *)opaque;
     SpiceLinkMess *link_mess = link->link_mess;
     uint32_t num_caps = link_mess->num_common_caps + link_mess->num_channel_caps;
-    uint32_t *caps = (uint32_t *)((uint8_t *)link_mess + link_mess->caps_offset);
     int auth_selection;
 
     if (num_caps && (num_caps * sizeof(uint32_t) + link_mess->caps_offset >
@@ -2061,8 +2076,8 @@ static void reds_handle_read_link_done(void *opaque)
         return;
     }
 
-    auth_selection = test_capability(caps, link_mess->num_common_caps,
-                                     SPICE_COMMON_CAP_PROTOCOL_AUTH_SELECTION);
+    auth_selection = red_link_info_test_capability(link,
+                                                   SPICE_COMMON_CAP_PROTOCOL_AUTH_SELECTION);
 
     if (!reds_security_check(link)) {
         if (reds_stream_is_ssl(link->stream)) {
commit ef44803a3a8ef06635fd8efd40cd30c95972839f
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Wed Mar 19 18:17:32 2014 +0100

    Add const to test_capability first argument
    
    We don't modify the capabilities content, so it can be marked as const.

diff --git a/server/red_channel.c b/server/red_channel.c
index 5904381..b06efbf 100644
--- a/server/red_channel.c
+++ b/server/red_channel.c
@@ -1172,7 +1172,7 @@ void red_channel_register_client_cbs(RedChannel *channel, ClientCbs *client_cbs)
     }
 }
 
-int test_capability(uint32_t *caps, int num_caps, uint32_t cap)
+int test_capability(const uint32_t *caps, int num_caps, uint32_t cap)
 {
     uint32_t index = cap / 32;
     if (num_caps < index + 1) {
diff --git a/server/red_channel.h b/server/red_channel.h
index f9cfb95..24d29fe 100644
--- a/server/red_channel.h
+++ b/server/red_channel.h
@@ -223,7 +223,7 @@ typedef struct RedChannelCapabilities {
     uint32_t *caps;
 } RedChannelCapabilities;
 
-int test_capability(uint32_t *caps, int num_caps, uint32_t cap);
+int test_capability(const uint32_t *caps, int num_caps, uint32_t cap);
 
 typedef struct RedChannelClientLatencyMonitor {
     int state;


More information about the Spice-commits mailing list