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

[Christophe Fergeau]

No taker for a review ?

Christophe

On Tue, Oct 21, 2014 at 03:54:08PM +0200, Christophe Fergeau 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20141106/e0f04f57/attachment.sig>