[Spice-devel] [spice-protocol PATCH] Add SpiceLinkReply::ticket_encryption

[Marc-André Lureau]

----- Original Message -----
> Currently, SPICE tickets sent to the server are encrypted using a 1024 bit
> public RSA key provided by the server. This key type/size is unfortunately
> set in stone in the SPICE protocol as part of the SpiceLinkReply message,
> and the key is sent by the server early in the link process (before the
> server and the client agree on a SpiceLinkAuthMechanism).
> 
> This can be an issue if the server can't create a 1024 bit RSA key (for
> example, if it was disabled because it's deemed not secure enough by the
> server administrator). This happens for example in fips mode (
> http://csrc.nist.gov/publications/fips/fips140-2/fips1402.pdf )
> 
> Luckily, the server gets the client caps before sending it this RSA
> key, and it sends its caps in message containing this RSA key. By
> advertising a new capability on the client and the server, it's thus
> possible to indicate that other ways of encrypting the SPICE ticket are
> supported by both client and server, and use that when available. When the
> capability is present, an additional 'ticket_encryption' field is added to
> the SpiceLinkReply structure to indicate that the SPICE ticket is not
> encrypted using the legacy RSA 1024 bit key.
> 
> As the situation described above would happen in hardened setups, I've
> added support for an unencrypted SPICE ticket which is only used for TLS
> channels. For non-TLS channels, the old method is still used in order to
> not send the ticket in plain text on unencrypted connections.
> ---
>  spice/protocol.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/spice/protocol.h b/spice/protocol.h
> index 961a303..f35c72e 100644
> --- a/spice/protocol.h
> +++ b/spice/protocol.h
> @@ -56,8 +56,15 @@ enum {
>      SPICE_COMMON_CAP_AUTH_SPICE,
>      SPICE_COMMON_CAP_AUTH_SASL,
>      SPICE_COMMON_CAP_MINI_HEADER,
> +    SPICE_COMMON_CAP_PROTOCOL_PLAIN_TEXT_TICKET,
>  };
>  
> +typedef enum {
> +    SPICE_TICKET_ENCRYPTION_INVALID,
> +    SPICE_TICKET_ENCRYPTION_RSA,
> +    SPICE_TICKET_ENCRYPTION_NONE,
> +} SpiceTicketEncryption;
> +
>  typedef struct SPICE_ATTR_PACKED SpiceLinkMess {
>      uint32_t connection_id;
>      uint8_t channel_type;
> @@ -73,6 +80,7 @@ typedef struct SPICE_ATTR_PACKED SpiceLinkReply {
>      uint32_t num_common_caps;
>      uint32_t num_channel_caps;
>      uint32_t caps_offset;
> +    uint8_t ticket_encryption;
>  } SpiceLinkReply;
>  

Is this going to break an old server compiled with a new version of the protocol, since you are changing the fields of an existing message structure? For example, isn't there any sizeof() somewhere that would break some size assumptions?

I quickly found this check in current server:
    if (num_caps && (num_caps * sizeof(uint32_t) + link_mess->caps_offset >
                     link->link_header.size ||
                     link_mess->caps_offset < sizeof(*link_mess))) {
        reds_send_link_error(link, SPICE_LINK_ERR_INVALID_DATA);
        reds_link_free(link);
        return;
    }

will break once compiled with a new version of the header, I am afraid.

You probably need a new structure, or just keep this additional field as  seperate/optionnal/commented?

>  typedef struct SPICE_ATTR_PACKED SpiceLinkEncryptedTicket {
> --
> 1.8.5.3
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>