From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=57458 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PhjiG-00045M-T6 for qemu-devel@nongnu.org; Tue, 25 Jan 2011 09:17:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PhjiA-0004Vh-K1 for qemu-devel@nongnu.org; Tue, 25 Jan 2011 09:17:35 -0500 Received: from mail-yw0-f45.google.com ([209.85.213.45]:63773) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PhjiA-0004Va-GT for qemu-devel@nongnu.org; Tue, 25 Jan 2011 09:17:34 -0500 Received: by ywa8 with SMTP id 8so1542600ywa.4 for ; Tue, 25 Jan 2011 06:17:34 -0800 (PST) Message-ID: <4D3EDB7C.3050604@codemonkey.ws> Date: Tue, 25 Jan 2011 08:17:32 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 2/7] ccid: add passthru card device References: <1294735359-4009-1-git-send-email-alevy@redhat.com> <1294735359-4009-3-git-send-email-alevy@redhat.com> In-Reply-To: <1294735359-4009-3-git-send-email-alevy@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alon Levy Cc: qemu-devel@nongnu.org On 01/11/2011 02:42 AM, Alon Levy wrote: > diff --git a/libcacard/vscard_common.h b/libcacard/vscard_common.h > new file mode 100644 > index 0000000..9ff1295 > --- /dev/null > +++ b/libcacard/vscard_common.h > This file (and the .c file) need a coding style pass to fixup comments and the use of _ as a prefix but I want to focus on the protocol itself. First, let's get a written spec into the wiki. I think it's important that all of our compatibility protocols are documented in a more formal way such that can be reviewed by a wider audience. > @@ -0,0 +1,130 @@ > +/* Virtual Smart Card protocol definition > + * > + * This protocol is between a host implementing a group of virtual smart card > + * reader, and a client implementing a virtual smart card, or passthrough to > + * a real card. > + * > + * The current implementation passes the raw APDU's from 7816 and additionally > + * contains messages to setup and teardown readers, handle insertion and > + * removal of cards, negotiate the protocol and provide for error responses. > + * > + * Copyright (c) 2010 Red Hat. > + * > + * This code is licensed under the LGPL. > + */ > + > +#ifndef _VSCARD_COMMON_H > +#define _VSCARD_COMMON_H > + > +#include > + > +#define VERSION_MAJOR_BITS 11 > +#define VERSION_MIDDLE_BITS 11 > +#define VERSION_MINOR_BITS 10 > Distros make versioning not enough. Inevitably, someone wants to back port a bug fix or a feature for some RHEL7.2 release or something like that. Feature negotiation has worked pretty well for us and I'd suggest using it within the protocol. > +#define MAKE_VERSION(major, middle, minor) \ > + ( (major<< (VERSION_MINOR_BITS + VERSION_MIDDLE_BITS)) \ > + | (middle<< VERSION_MINOR_BITS) \ > + | (minor) ) > + > +/** IMPORTANT NOTE on VERSION > + * > + * The version below MUST be changed whenever a change in this file is made. > + * > + * The last digit, the minor, is for bug fix changes only. > + * > + * The middle digit is for backward / forward compatible changes, updates > + * to the existing messages, addition of fields. > + * > + * The major digit is for a breaking change of protocol, presumably > + * something that cannot be accomodated with the existing protocol. > + */ > + > +#define VSCARD_VERSION MAKE_VERSION(0,0,1) > + > +typedef enum { > + VSC_Init, > + VSC_Error, > + VSC_ReaderAdd, > + VSC_ReaderAddResponse, > + VSC_ReaderRemove, > + VSC_ATR, > + VSC_CardRemove, > + VSC_APDU, > + VSC_Reconnect > +} VSCMsgType; > Should number the enum to be specific at least. > + > +typedef enum { > + VSC_GENERAL_ERROR=1, > + VSC_CANNOT_ADD_MORE_READERS, > +} VSCErrorCode; > + > +typedef uint32_t reader_id_t; > This namespace is reserved by C. > +#define VSCARD_UNDEFINED_READER_ID 0xffffffff > +#define VSCARD_MINIMAL_READER_ID 0 > + > +typedef struct VSCMsgHeader { > + VSCMsgType type; > + reader_id_t reader_id; > + uint32_t length; > Is length just the data length or the whole message length? > + uint8_t data[0]; > +} VSCMsgHeader; > + > +/* VSCMsgInit Client<-> Host > + * Host replies with allocated reader id in ReaderAddResponse > + * */ > +typedef struct VSCMsgInit { > + uint32_t version; > +} VSCMsgInit; > + > +/* VSCMsgError Client<-> Host > + * */ > +typedef struct VSCMsgError { > + uint32_t code; > +} VSCMsgError; > + > +/* VSCMsgReaderAdd Client -> Host > + * Host replies with allocated reader id in ReaderAddResponse > + * name - name of the reader on client side. > + * */ > +typedef struct VSCMsgReaderAdd { > + uint8_t name[0]; > Is this a string? > +} VSCMsgReaderAdd; > + > +/* VSCMsgReaderAddResponse Host -> Client > + * Reply to ReaderAdd > + * */ > +typedef struct VSCMsgReaderAddResponse { > +} VSCMsgReaderAddResponse; > + > +/* VSCMsgReaderRemove Client -> Host > + * */ > +typedef struct VSCMsgReaderRemove { > +} VSCMsgReaderRemove; > + > +/* VSCMsgATR Client -> Host > + * Answer to reset. Sent for card insertion or card reset. > + * */ > +typedef struct VSCMsgATR { > + uint8_t atr[0]; > +} VSCMsgATR; > + > +/* VSCMsgCardRemove Client -> Host > + * */ > +typedef struct VSCMsgCardRemove { > +} VSCMsgCardRemove; > + > +/* VSCMsgAPDU Client<-> Host > + * */ > +typedef struct VSCMsgAPDU { > + uint8_t data[0]; > +} VSCMsgAPDU; > + > +/* VSCMsgReconnect Host -> Client > + * */ > +typedef struct VSCMsgReconnect { > + uint32_t ip; > This is not ipv6 friendly. Two strings would be a better choice. Regards, Anthony Liguori > + uint16_t port; > +} VSCMsgReconnect; > + > +#endif >