[Spice-devel] [PATCH spice-server v2 1/7] Split RedCharDeviceSmartcard and SmartCardChannelClient
Frediano Ziglio
fziglio at redhat.com
Thu Oct 6 09:07:59 UTC 2016
>
> On Fri, 2016-09-30 at 14:21 +0100, Frediano Ziglio wrote:
> > ---
> > server/Makefile.am | 2 +
> > server/smartcard-channel-client.c | 300
> > +++++++++++++++++++++++++++++++
> > server/smartcard-channel-client.h | 96 ++++++++++
> > server/smartcard.c | 360 ++++++--------------------
> > ------------
> > server/smartcard.h | 21 +++
> > 5 files changed, 471 insertions(+), 308 deletions(-)
> > create mode 100644 server/smartcard-channel-client.c
> > create mode 100644 server/smartcard-channel-client.h
> >
> > diff --git a/server/Makefile.am b/server/Makefile.am
> > index 9c5b3b6..abbec16 100644
> > --- a/server/Makefile.am
> > +++ b/server/Makefile.am
> > @@ -172,6 +172,8 @@ if HAVE_SMARTCARD
> > libserver_la_SOURCES += \
> > smartcard.c \
> > smartcard.h \
> > + smartcard-channel-client.c \
> > + smartcard-channel-client.h \
> > $(NULL)
> > endif
> >
> > diff --git a/server/smartcard-channel-client.c b/server/smartcard-
> > channel-client.c
> > new file mode 100644
> > index 0000000..0622be5
> > --- /dev/null
> > +++ b/server/smartcard-channel-client.c
> > @@ -0,0 +1,300 @@
> > +/*
> > + Copyright (C) 2009-2015 Red Hat, Inc.
> > +
> > + This library is free software; you can redistribute it and/or
> > + modify it under the terms of the GNU Lesser General Public
> > + License as published by the Free Software Foundation; either
> > + version 2.1 of the License, or (at your option) any later
> > version.
> > +
> > + This library is distributed in the hope that it will be useful,
> > + but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + Lesser General Public License for more details.
> > +
> > + You should have received a copy of the GNU Lesser General Public
> > + License along with this library; if not, see <http://www.gnu.org/
> > licenses/>.
> > +*/
> > +#ifdef HAVE_CONFIG_H
> > +#include <config.h>
> > +#endif
> > +
> > +#include "smartcard-channel-client.h"
> > +
> > +typedef struct RedErrorItem {
> > + RedPipeItem base;
> > + VSCMsgHeader vheader;
> > + VSCMsgError error;
> > +} RedErrorItem;
> > +
> > +uint8_t *smartcard_channel_client_alloc_msg_rcv_buf(RedChannelClient
> > *rcc,
> > + uint16_t type,
> > + uint32_t size)
> > +{
> > + SmartCardChannelClient *scc = SMARTCARD_CHANNEL_CLIENT(rcc);
> > + RedClient *client = red_channel_client_get_client(rcc);
> > +
> > + /* todo: only one reader is actually supported. When we fix the
> > code to support
> > + * multiple readers, we will porbably associate different
> > devices to
> > + * differenc channels */
>
>
> Hmm, just noticed these typos. Should we fix them in this commit?
> "porbably"
> "differenc"
>
Can go in this or a following patch.
>
> > + if (!scc->priv->smartcard) {
> > + scc->priv->msg_in_write_buf = FALSE;
> > + return spice_malloc(size);
> > + } else {
> > + RedCharDeviceSmartcard *smartcard;
> > +
> > + spice_assert(smartcard_get_n_readers() == 1);
> > + smartcard = scc->priv->smartcard;
> > + spice_assert(smartcard_char_device_get_client(smartcard) ||
> > scc->priv->smartcard);
> > + spice_assert(!scc->priv->write_buf);
> > + scc->priv->write_buf =
> > + red_char_device_write_buffer_get(RED_CHAR_DEVICE(smartca
> > rd), client,
> > + size);
> > +
> > + if (!scc->priv->write_buf) {
> > + spice_error("failed to allocate write buffer");
> > + return NULL;
> > + }
> > + scc->priv->msg_in_write_buf = TRUE;
> > + return scc->priv->write_buf->buf;
> > + }
> > +}
> > +
> > +void smartcard_channel_client_release_msg_rcv_buf(RedChannelClient
> > *rcc,
> > + uint16_t type,
> > + uint32_t size,
> > + uint8_t *msg)
> > +{
> > + SmartCardChannelClient *scc = SMARTCARD_CHANNEL_CLIENT(rcc);
> > +
> > + /* todo: only one reader is actually supported. When we fix the
> > code to support
> > + * multiple readers, we will porbably associate different
> > devices to
> > + * differenc channels */
>
> Same typos here.
>
>
> > +
> > + if (!scc->priv->msg_in_write_buf) {
> > + spice_assert(!scc->priv->write_buf);
> > + free(msg);
> > + } else {
> > + if (scc->priv->write_buf) { /* msg hasn't been pushed to the
> > guest */
> > + spice_assert(scc->priv->write_buf->buf == msg);
> > + red_char_device_write_buffer_release(RED_CHAR_DEVICE(scc
> > ->priv->smartcard),
> > + &scc->priv-
> > >write_buf);
> > + }
> > + }
> > +}
> > +
> > +void smartcard_channel_client_on_disconnect(RedChannelClient *rcc)
> > +{
> > + SmartCardChannelClient *scc = SMARTCARD_CHANNEL_CLIENT(rcc);
> > + RedCharDeviceSmartcard *device = scc->priv->smartcard;
> > +
> > + if (device) {
> > + smartcard_char_device_detach_client(device, scc);
> > + smartcard_char_device_notify_reader_remove(device);
> > + }
> > +}
> > +
> > +void smartcard_channel_client_send_data(RedChannelClient *rcc,
> > + SpiceMarshaller *m,
> > + RedPipeItem *item,
> > + VSCMsgHeader *vheader)
> > +{
> > + spice_assert(rcc);
> > + spice_assert(vheader);
> > + red_channel_client_init_send_data(rcc, SPICE_MSG_SMARTCARD_DATA,
> > item);
> > + spice_marshaller_add_ref(m, (uint8_t*)vheader,
> > sizeof(VSCMsgHeader));
> > + if (vheader->length > 0) {
> > + spice_marshaller_add_ref(m, (uint8_t*)(vheader+1), vheader-
> > >length);
> > + }
> > +}
> > +
> > +void smartcard_channel_client_send_error(RedChannelClient *rcc,
> > SpiceMarshaller *m, RedPipeItem *item)
> > +{
> > + RedErrorItem* error_item = SPICE_UPCAST(RedErrorItem, item);
> > +
> > + smartcard_channel_client_send_data(rcc, m, item, &error_item-
> > >vheader);
> > +}
> > +
> > +static void smartcard_channel_client_push_error(RedChannelClient
> > *rcc,
> > + uint32_t reader_id,
> > + VSCErrorCode error)
> > +{
> > + RedErrorItem *error_item = spice_new0(RedErrorItem, 1);
> > +
> > + red_pipe_item_init(&error_item->base, RED_PIPE_ITEM_TYPE_ERROR);
> > +
> > + error_item->vheader.reader_id = reader_id;
> > + error_item->vheader.type = VSC_Error;
> > + error_item->vheader.length = sizeof(error_item->error);
> > + error_item->error.code = error;
> > + red_channel_client_pipe_add_push(rcc, &error_item->base);
> > +}
> > +
> > +static void
> > smartcard_channel_client_add_reader(SmartCardChannelClient *scc,
> > + uint8_t *name)
> > +{
> > + if (!scc->priv->smartcard) { /* we already tried to attach a
> > reader to the client
> > + when it connected */
>
> This comment has odd indentation.
>
> > + SpiceCharDeviceInstance *char_device =
> > smartcard_readers_get_unattached();
> > +
> > + if (!char_device) {
> > + smartcard_channel_client_push_error(RED_CHANNEL_CLIENT(s
> > cc),
> > + VSCARD_UNDEFINED_REA
> > DER_ID,
> > + VSC_CANNOT_ADD_MORE_
> > READERS);
> > + return;
> > + }
> > + smartcard_char_device_attach_client(char_device, scc);
> > + }
> > + smartcard_char_device_notify_reader_add(scc->priv->smartcard);
> > + // The device sends a VSC_Error message, we will let it through,
> > no
> > + // need to send our own. We already set the correct reader_id,
> > from
> > + // our RedCharDeviceSmartcard.
> > +}
> > +
> > +static void
> > smartcard_channel_client_remove_reader(SmartCardChannelClient *scc,
> > + uint32_t
> > reader_id)
> > +{
> > + SpiceCharDeviceInstance *char_device =
> > smartcard_readers_get(reader_id);
> > + RedCharDeviceSmartcard *dev;
> > +
> > + if (char_device == NULL) {
> > + smartcard_channel_client_push_error(RED_CHANNEL_CLIENT(scc),
> > + reader_id,
> > VSC_GENERAL_ERROR);
> > + return;
> > + }
> > +
> > + dev = red_char_device_opaque_get(char_device->st);
> > + spice_assert(scc->priv->smartcard == dev);
> > + if (!smartcard_char_device_notify_reader_remove(dev)) {
> > + smartcard_channel_client_push_error(RED_CHANNEL_CLIENT(scc),
> > + reader_id,
> > VSC_GENERAL_ERROR);
> > + return;
> > + }
> > +}
> > +
> > +RedCharDeviceSmartcard*
> > smartcard_channel_client_get_device(SmartCardChannelClient *scc)
> > +{
> > + return scc->priv->smartcard;
> > +}
> > +
> > +static void
> > smartcard_channel_client_write_to_reader(SmartCardChannelClient *scc)
> > +{
> > + g_return_if_fail(scc);
> > +
> > + smartcard_channel_write_to_reader(scc->priv->write_buf);
> > + scc->priv->write_buf = NULL;
> > +}
> > +
> > +
> > +int smartcard_channel_client_handle_message(RedChannelClient *rcc,
> > + uint16_t type,
> > + uint32_t size,
> > + uint8_t *msg)
> > +{
> > + VSCMsgHeader* vheader = (VSCMsgHeader*)msg;
> > + SmartCardChannelClient *scc = SMARTCARD_CHANNEL_CLIENT(rcc);
> > +
> > + if (type != SPICE_MSGC_SMARTCARD_DATA) {
> > + /* Handles seamless migration protocol. Also handles ack's,
> > + * spicy sends them while spicec does not */
> > + return red_channel_client_handle_message(rcc, size, type,
> > msg);
> > + }
> > +
> > + spice_assert(size == vheader->length + sizeof(VSCMsgHeader));
> > + switch (vheader->type) {
> > + case VSC_ReaderAdd:
> > + smartcard_channel_client_add_reader(scc, msg +
> > sizeof(VSCMsgHeader));
> > + return TRUE;
> > + break;
> > + case VSC_ReaderRemove:
> > + smartcard_channel_client_remove_reader(scc, vheader-
> > >reader_id);
> > + return TRUE;
> > + break;
> > + case VSC_Init:
> > + // ignore - we should never get this anyway
> > + return TRUE;
> > + break;
> > + case VSC_Error:
> > + case VSC_ATR:
> > + case VSC_CardRemove:
> > + case VSC_APDU:
> > + break; // passed on to device
> > + default:
> > + printf("ERROR: unexpected message on smartcard
> > channel\n");
> > + return TRUE;
> > + }
> > +
> > + /* todo: fix */
> > + if (vheader->reader_id >= smartcard_get_n_readers()) {
> > + spice_printerr("ERROR: received message for non existing
> > reader: %d, %d, %d", vheader->reader_id,
> > + vheader->type, vheader->length);
> > + return FALSE;
> > + }
> > + spice_assert(scc->priv->write_buf->buf == msg);
> > + smartcard_channel_client_write_to_reader(scc);
> > +
> > + return TRUE;
> > +}
> > +
> > +int smartcard_channel_client_handle_migrate_data(RedChannelClient
> > *rcc,
> > + uint32_t size,
> > + void *message)
> > +{
> > + SmartCardChannelClient *scc;
> > + SpiceMigrateDataHeader *header;
> > + SpiceMigrateDataSmartcard *mig_data;
> > +
> > + scc = SMARTCARD_CHANNEL_CLIENT(rcc);
> > + header = (SpiceMigrateDataHeader *)message;
> > + mig_data = (SpiceMigrateDataSmartcard *)(header + 1);
> > + if (size < sizeof(SpiceMigrateDataHeader) +
> > sizeof(SpiceMigrateDataSmartcard)) {
> > + spice_error("bad message size");
> > + return FALSE;
> > + }
> > + if (!migration_protocol_validate_header(header,
> > + SPICE_MIGRATE_DATA_SMART
> > CARD_MAGIC,
> > + SPICE_MIGRATE_DATA_SMART
> > CARD_VERSION)) {
> > + spice_error("bad header");
> > + return FALSE;
> > + }
> > +
> > + if (!mig_data->base.connected) { /* client wasn't attached to a
> > smartcard */
> > + return TRUE;
> > + }
> > +
> > + if (!scc->priv->smartcard) {
> > + SpiceCharDeviceInstance *char_device =
> > smartcard_readers_get_unattached();
> > +
> > + if (!char_device) {
> > + spice_warning("no unattached device available");
> > + return TRUE;
> > + } else {
> > + smartcard_char_device_attach_client(char_device, scc);
> > + }
> > + }
> > + spice_debug("reader added %d partial read_size %u", mig_data-
> > >reader_added, mig_data->read_size);
> > +
> > + return smartcard_char_device_handle_migrate_data(scc->priv-
> > >smartcard,
> > + mig_data);
> > +}
> > +
> > +int
> > smartcard_channel_client_handle_migrate_flush_mark(RedChannelClient
> > *rcc)
> > +{
> > + red_channel_client_pipe_add_type(rcc,
> > RED_PIPE_ITEM_TYPE_SMARTCARD_MIGRATE_DATA);
> > + return TRUE;
> > +}
> > +
> > +void smartcard_channel_client_set_char_device(SmartCardChannelClient
> > *scc,
> > + RedCharDeviceSmartcard
> > *device)
> > +{
> > + if (device == scc->priv->smartcard) {
> > + return;
> > + }
> > +
> > + scc->priv->smartcard = device;
> > +}
> > +
> > +RedCharDeviceSmartcard*
> > smartcard_channel_client_get_char_device(SmartCardChannelClient *scc)
> > +{
> > + return scc->priv->smartcard;
> > +}
> > +
> > diff --git a/server/smartcard-channel-client.h b/server/smartcard-
> > channel-client.h
> > new file mode 100644
> > index 0000000..44a966a
> > --- /dev/null
> > +++ b/server/smartcard-channel-client.h
> > @@ -0,0 +1,96 @@
> > +/*
> > + Copyright (C) 2009-2015 Red Hat, Inc.
> > +
> > + This library is free software; you can redistribute it and/or
> > + modify it under the terms of the GNU Lesser General Public
> > + License as published by the Free Software Foundation; either
> > + version 2.1 of the License, or (at your option) any later
> > version.
> > +
> > + This library is distributed in the hope that it will be useful,
> > + but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + Lesser General Public License for more details.
> > +
> > + You should have received a copy of the GNU Lesser General Public
> > + License along with this library; if not, see <http://www.gnu.org/
> > licenses/>.
> > +*/
> > +
> > +#ifndef SMARTCARD_CHANNEL_CLIENT_H__
> > +#define SMARTCARD_CHANNEL_CLIENT_H__
> > +
> > +#include "smartcard.h"
> > +#include "red-channel-client.h"
> > +
> > +typedef struct SmartCardChannelClientPrivate
> > SmartCardChannelClientPrivate;
> > +struct SmartCardChannelClientPrivate {
> > + RedCharDeviceSmartcard *smartcard;
> > +
> > + /* read_from_client/write_to_device buffer.
> > + * The beginning of the buffer should always be VSCMsgHeader*/
> > + RedCharDeviceWriteBuffer *write_buf;
> > + int msg_in_write_buf; /* was the client msg received into a
> > RedCharDeviceWriteBuffer
> > + * or was it explicitly malloced */
> > +};
> > +
> > +typedef struct SmartCardChannelClient {
> > + RedChannelClient base;
> > +
> > + SmartCardChannelClientPrivate priv[1];
> > +} SmartCardChannelClient;
> > +
> > +#define SMARTCARD_CHANNEL_CLIENT(rcc) ((SmartCardChannelClient*)rcc)
> > +
> > +SmartCardChannelClient* smartcard_channel_client_create(RedChannel
> > *channel,
> > + RedClient
> > *client, RedsStream *stream,
> > + int
> > monitor_latency,
> > + int
> > num_common_caps, uint32_t *common_caps,
> > + int
> > num_caps, uint32_t *caps);
> > +
> > +uint8_t* smartcard_channel_client_alloc_msg_rcv_buf(RedChannelClient
> > *rcc,
> > + uint16_t type,
> > + uint32_t size);
> > +
> > +void smartcard_channel_client_release_msg_rcv_buf(RedChannelClient
> > *rcc,
> > + uint16_t type,
> > + uint32_t size,
> > + uint8_t *msg);
> > +
> > +int
> > smartcard_channel_client_handle_migrate_flush_mark(RedChannelClient
> > *rcc);
> > +
> > +void smartcard_channel_client_on_disconnect(RedChannelClient *rcc);
> > +
> > +void smartcard_channel_client_send_data(RedChannelClient *rcc,
> > + SpiceMarshaller *m,
> > + RedPipeItem *item,
> > + VSCMsgHeader *vheader);
> > +
> > +void smartcard_channel_client_send_error(RedChannelClient *rcc,
> > + SpiceMarshaller *m,
> > + RedPipeItem *item);
> > +
> > +RedCharDeviceSmartcard*
> > smartcard_channel_client_get_device(SmartCardChannelClient *scc);
> > +
> > +int smartcard_channel_client_handle_message(RedChannelClient *rcc,
> > + uint16_t type,
> > + uint32_t size,
> > + uint8_t *msg);
> > +
> > +int smartcard_channel_client_handle_migrate_data(RedChannelClient
> > *rcc,
> > + uint32_t size,
> > + void *message);
> > +
> > +void smartcard_channel_client_set_char_device(SmartCardChannelClient
> > *scc,
> > + RedCharDeviceSmartcard
> > *device);
> > +
> > +RedCharDeviceSmartcard*
> > smartcard_channel_client_get_char_device(SmartCardChannelClient
> > *scc);
> > +
> > +void smartcard_channel_client_release_msg_rcv_buf(RedChannelClient
> > *rcc,
> > + uint16_t type,
> > + uint32_t size,
> > + uint8_t *msg);
> > +
> > +uint8_t *smartcard_channel_client_alloc_msg_rcv_buf(RedChannelClient
> > *rcc,
> > + uint16_t type,
> > + uint32_t size);
> > +
> > +#endif /* SMARTCARD_CHANNEL_CLIENT_H__ */
> > diff --git a/server/smartcard.c b/server/smartcard.c
> > index 41dc106..ab95260 100644
> > --- a/server/smartcard.c
> > +++ b/server/smartcard.c
> > @@ -30,8 +30,8 @@
> >
> > #include "reds.h"
> > #include "char-device.h"
> > -#include "red-channel-client.h"
> > #include "smartcard.h"
> > +#include "smartcard-channel-client.h"
> > #include "migration-protocol.h"
> >
> > /*
> > @@ -49,25 +49,6 @@
> > // Maximal length of APDU
> > #define APDUBufSize 270
> >
> > -typedef struct SmartCardChannelClientPrivate
> > SmartCardChannelClientPrivate;
> > -struct SmartCardChannelClientPrivate {
> > - RedCharDeviceSmartcard *smartcard;
> > -
> > - /* read_from_client/write_to_device buffer.
> > - * The beginning of the buffer should always be VSCMsgHeader*/
> > - RedCharDeviceWriteBuffer *write_buf;
> > - int msg_in_write_buf; /* was the client msg received into a
> > RedCharDeviceWriteBuffer
> > - * or was it explicitly malloced */
> > -};
> > -
> > -typedef struct SmartCardChannelClient {
> > - RedChannelClient base;
> > -
> > - SmartCardChannelClientPrivate priv[1];
> > -} SmartCardChannelClient;
> > -
> > -#define SMARTCARD_CHANNEL_CLIENT(rcc) ((SmartCardChannelClient*)rcc)
> > -
> > G_DEFINE_TYPE(RedCharDeviceSmartcard, red_char_device_smartcard,
> > RED_TYPE_CHAR_DEVICE)
> >
> > #define RED_CHAR_DEVICE_SMARTCARD_PRIVATE(o)
> > (G_TYPE_INSTANCE_GET_PRIVATE ((o), RED_TYPE_CHAR_DEVICE_SMARTCARD,
> > RedCharDeviceSmartcardPrivate))
> > @@ -84,18 +65,6 @@ struct RedCharDeviceSmartcardPrivate {
> > int reader_added; // has reader_add been sent
> > to the device
> > };
> >
> > -enum {
> > - RED_PIPE_ITEM_TYPE_ERROR = RED_PIPE_ITEM_TYPE_CHANNEL_BASE,
> > - RED_PIPE_ITEM_TYPE_SMARTCARD_DATA,
> > - RED_PIPE_ITEM_TYPE_SMARTCARD_MIGRATE_DATA,
> > -};
> > -
> > -typedef struct RedErrorItem {
> > - RedPipeItem base;
> > - VSCMsgHeader vheader;
> > - VSCMsgError error;
> > -} RedErrorItem;
> > -
> > typedef struct RedMsgItem {
> > RedPipeItem base;
> >
> > @@ -114,12 +83,7 @@ static struct Readers {
> > SpiceCharDeviceInstance* sin[SMARTCARD_MAX_READERS];
> > } g_smartcard_readers = {0, {NULL}};
> >
> > -static SpiceCharDeviceInstance*
> > smartcard_readers_get_unattached(void);
> > -static SpiceCharDeviceInstance* smartcard_readers_get(uint32_t
> > reader_id);
> > static int smartcard_char_device_add_to_readers(RedsState *reds,
> > SpiceCharDeviceInstance *sin);
> > -static void smartcard_char_device_attach_client(
> > - SpiceCharDeviceInstance *char_device, SmartCardChannelClient
> > *scc);
> > -static void
> > smartcard_channel_write_to_reader(RedCharDeviceWriteBuffer
> > *write_buf);
> >
> > static RedMsgItem *smartcard_char_device_on_message_from_device(
> > RedCharDeviceSmartcard *dev, VSCMsgHeader *header);
> > @@ -179,11 +143,12 @@ static void
> > smartcard_send_msg_to_client(RedPipeItem *msg,
> > void *opaque)
> > {
> > RedCharDeviceSmartcard *dev = opaque;
> > + RedChannelClient *rcc = RED_CHANNEL_CLIENT(dev->priv->scc);
> >
> > spice_assert(dev->priv->scc &&
> > - red_channel_client_get_client(&dev->priv->scc-
> > >base) == client);
> > + red_channel_client_get_client(rcc) == client);
> > red_pipe_item_ref(msg);
> > - smartcard_channel_client_pipe_add_push(&dev->priv->scc->base,
> > msg);
> > + smartcard_channel_client_pipe_add_push(rcc, msg);
>
>
> Perhaps we should just change this to
> red_channel_client_pipe_add_push() and remove the smartcard_ version.
>
Yes, make not much sense.
Not a regression but the red_pipe_item_ref before is weird.
>
...
> > + RED_PIPE_ITEM_TYPE_SMARTCARD_MIGRATE_DATA,
> > +};
> >
> > #endif // __SMART_CARD_H__
>
>
> Otherwise looks fine.
> Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
>
Acked too.
Frediano
More information about the Spice-devel
mailing list