From: Anthony Liguori <anthony@codemonkey.ws>
To: Alon Levy <alevy@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 2/7] ccid: add passthru card device
Date: Tue, 25 Jan 2011 08:17:32 -0600 [thread overview]
Message-ID: <4D3EDB7C.3050604@codemonkey.ws> (raw)
In-Reply-To: <1294735359-4009-3-git-send-email-alevy@redhat.com>
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<stdint.h>
> +
> +#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
>
next prev parent reply other threads:[~2011-01-25 14:17 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-01-11 8:42 [Qemu-devel] [PATCH 0/7] usb-ccid (v15) Alon Levy
2011-01-11 8:42 ` [Qemu-devel] [PATCH 1/7] usb-ccid: add CCID bus Alon Levy
2011-01-11 8:42 ` [Qemu-devel] [PATCH 2/7] ccid: add passthru card device Alon Levy
2011-01-25 14:17 ` Anthony Liguori [this message]
2011-01-25 16:21 ` Alon Levy
2011-01-25 16:24 ` Anthony Liguori
2011-01-25 16:50 ` Alon Levy
2011-01-27 21:13 ` Alon Levy
2011-01-27 21:42 ` Anthony Liguori
2011-01-30 17:35 ` Alon Levy
2011-01-11 8:42 ` [Qemu-devel] [PATCH 3/7] libcacard: initial commit after coding style fixes Alon Levy
2011-01-25 14:19 ` Anthony Liguori
2011-01-11 8:42 ` [Qemu-devel] [PATCH 4/7] ccid: add ccid-card-emulated device (v2) Alon Levy
2011-01-25 14:21 ` Anthony Liguori
2011-01-25 16:24 ` Alon Levy
2011-01-25 16:27 ` Anthony Liguori
2011-01-31 19:28 ` Alon Levy
2011-01-11 8:42 ` [Qemu-devel] [PATCH 5/7] ccid: add docs Alon Levy
2011-01-11 8:42 ` [Qemu-devel] [PATCH 6/7] ccid: configure: add --enable/disable and nss only disable Alon Levy
2011-01-11 8:42 ` [Qemu-devel] [PATCH 6/6] ccid: configure: add --enable-smartcard and --disable-smartcard Alon Levy
2011-01-11 9:03 ` Alon Levy
2011-01-11 8:42 ` [Qemu-devel] [PATCH 7/7] ccid: add qdev description strings Alon Levy
2011-01-17 15:56 ` [Qemu-devel] [PATCH 0/7] usb-ccid (v15) Alon Levy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4D3EDB7C.3050604@codemonkey.ws \
--to=anthony@codemonkey.ws \
--cc=alevy@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.