On Sunday, June 10 at 03:39PM, Viktor Tarasov wrote:
> Le 10/06/2012 14:21, Frank Morgner a écrit :
> > Uh, I think I am a bit late on this discussion...
> >
> > But I wanted to add a general concern that there are some conceptual
> > problems with the SM branch (and the recently included patches in
> > staging).
> >
> > 1. Global SM configuration is mixed with a configuration at the driver
> >    level. For example, look at [3]. It includes CWA, IAS/ECC data types
> >    which should be realized at the card driver level (or maybe some SM
> >    driver).
> 
> In [3] there is no IAS/ECC types.
> There are data types related to the two more-or-less different SM
> implementations: GlobalPlatform and CWA.  It's not card specific.

Well, well... I copied "CWA, IAS/ECC data types" from
https://github.com/OpenSC/OpenSC/blob/staging/src/libopensc/sm.h:133.
If there are no IAS/ECC types, it shows how wrong the documentation is.
(By the way, there *is* some function called
`iasecc_sm_external_authentication`, line 342)

However, sm.h still requires you to know cwa structures and gp
structures. This is typically what you would want to abstract with a SM
layer. Look at opensc.h and cards.h, which offer a similar abstraction
to the smart card driver. If you add a smart card driver to opensc, you
only have to add sc_get_yourcard_driver to card.h and add it to
internal_card_drivers in ctx.c. Effectively, you have to add only two
lines to OpenSC's core files and your card driver is integrated. Now think
about what you would have to change when adding a new SM implementation with
your architecture.

> > 2. There are at least two methods to hook into SM, using a SM module and
> >    implementing SM at the driver level. This conceptually introduces
> >    duplication. It is sure to be followed with code duplication. Both
> >    should be unified: Each card driver has a SM driver, which provides
> >    wrapping and unwrapping SM. A SM driver can then also be a SM module
> >    with external key sets.
> 
> Was it a question?
> Yes, there are two methods to trigger SM wrapping: 'APDU transmit' and
> 'ACL' mode.

I don't understand ACL mode, because it isn't used anywhere. (This, by
the way, begs the question why it is defined...) The two hooks I see is
via module or via implementation by the card driver.  I am advocating
for merging sm_module_operations and sm_card_operations.

> Duplication can happen due to fact that each card driver implement SM
> as it wants, and can include into card specific part entire SM crypto
> library.

Code duplication is bad. It leads to error duplication and other
problems.

> I do not contest that duplication can happen, but:
> - it should not be obstacle to the implementation;
> - please point out exactly, where in code you see duplication.

- CWA implements SM according to ISO 7816 ([4], p67) and so does
  epass3000.
- About two years ago I have pointed at some places where bit padding is
  implemented multiple times (with different implementations) -- this still
  holds today.

> > 3. As stated earlier, I am advocating for an additional abstraction
> >    between encoding and encryption. Thus, a SM driver would only
> >    implement encoding, offering an other abstraction to the crypto.
> >    Not implementing this abstraction has led to code duplication [1].
> >    And reinventing the wheel will continue, when all ISO-7816-like
> >    cards [2, for example] (or modules) offer their own implementation
> >    of:
> >    - TLV-encoding/decoding of cryptogram (with padding content
> >      indicator), mac and le (depending on the APDU case)
> >    - ISO padding of APDU header and data
> 
> As stated ealier, every card driver (at least at the beginning) can do
> roughly what it wants inside it's card specific part.  We have only
> started to introduce the common SM framework, and for a while, we will
> not blame the card driver if it includes it's own SM related crypto
> procedures (... and thus duplicates the code. Is that the code
> duplication that you are talking about?

Yes, a good architecture can reduce code duplication. Again, look at
OpenSC's card drivers. All can do what they want -- that's fine.
However, most drivers are reusing the iso7816 driver adding some tweaks.
For SM implementations, we should offer the same possibility.

> > 4. General problems with code quality:
> >    - A lot of dead code pieces ("#if 0")
> 
> You mean the SM part or entire code base of OpenSC.

Uh, I `grep`ped for '#if 0' and saw that this seems to be a common
practice in OpenSC. However, new patches should not introduce new pieces
of dead code. But the recent integration of SM into staging has has done
just that.

> >    - Usage of global switches instead of switches in the card context
> 
> What do you mean?

g_sm in epass3000

By the way, is there a possibility to comment on code? I think, github
only lets you comment on commits. I looked through the code yesterday
and found some other problems, which would have been easy to report...

> >    - No or wrong documentation of the SM stuff
> >    - directory "sm" should be renamed to "sm-modules"
> 
> ...
> 
> > These issues, I think, disqualify the SM branch to be included in
> > OpenSC's trunk.
> >
> > The biggest problem, I think is the coexistance of two independant SM 
> > hooks. 
> 
> You simply cannot cover all SM use cases with one low level SM wrapping 
> method.
> But understand you quite well -- you know/use only this one.

I disagree. I think it is possible. Simply look at the interface to
OpenSC's card drivers.

> > In general I dislike the concept of a SM module, because all
> > OpenSC initialization magic is lost, when only the APDU buffer is passed
> > to the module. If a module is only used for external keysets, then it
> > should do only encrypting/decrypting/authenticating instead of handling
> > the whole smart card stuff.

> External module do not handle all the smart card staff, but it has to
> do part of it.  Example: when importing, the RSA key has to be
> presented by a sequence of 5-7 APDUs, which could be different for the
> different cards.  So, external module has to be able to compose the
> plain APDUs .

Partly wrong: Yes, SM has to be smart card aware (as you have pointed
out with your RSA example). But the crypto module should not need to be
smart card aware. The whole point of your sm modules (correct me if I am
wrong) are cryptographic keys, which are located somewhere else. When it
comes to your RSA example: Simply let OpenSC collect all RSA data and
then send it to you crypto module.

> > Then, what you get is OpenSC that handles
> > smart card stuff, including SM encoding and a crypto layer with loadable
> > modules.
> 
> Sorry, do not understood.

Do something like:

struct sc_card_driver {
    ...
    struct sc_sm_driver *sm;
    ...
}
struct sc_sm_driver {
    ...
    struct sm_ctx *sm_ctx;
    int (*get_sm_apdu)(struct sc_card *card, struct sc_apdu *apdu, struct 
sc_apdu **sm_apdu);
    int (*free_sm_apdu)(struct sc_card *card, struct sc_apdu *apdu, struct 
sc_apdu **sm_apdu);
    ...
}
/** Secure messaging context */
struct sm_ctx {
    /** 1 if secure messaging is activated, 0 otherwise */
    unsigned char active;

    /** data of the specific crypto implementation */
    void *priv_data;

    /** Padding-content indicator byte (ISO 7816-4 Table 30) */
    u8 padding_indicator;
    /** Pad to this block length */
    size_t block_length;

    /** Call back function for authentication of data */
    int (*authenticate)(sc_card_t *card, const struct sm_ctx *ctx,
            const u8 *data, size_t datalen, u8 **outdata);
    /** Call back function for verifying authentication data */
    int (*verify_authentication)(sc_card_t *card, const struct sm_ctx *ctx,
            const u8 *mac, size_t maclen,
            const u8 *macdata, size_t macdatalen);

    /** Call back function for encryption of data */
    int (*encrypt)(sc_card_t *card, const struct sm_ctx *ctx,
            const u8 *data, size_t datalen, u8 **enc);
    /** Call back function for decryption of data */
    int (*decrypt)(sc_card_t *card, const struct sm_ctx *ctx,
            const u8 *enc, size_t enclen, u8 **data);

    /** Call back function for actions before encoding and encryption of \a 
apdu */
    int (*pre_transmit)(sc_card_t *card, const struct sm_ctx *ctx,
            sc_apdu_t *apdu);
    /** Call back function for actions before decryption and decoding of \a 
sm_apdu */
    int (*post_transmit)(sc_card_t *card, const struct sm_ctx *ctx,
            sc_apdu_t *sm_apdu);
    /** Call back function for actions after decrypting SM protected APDU */
    int (*finish)(sc_card_t *card, const struct sm_ctx *ctx,
            sc_apdu_t *apdu);

    /** Clears and frees private data */
    void (*clear_free)(const struct sm_ctx *ctx);
};

(struct sm_ctx is actually copied from my approach to SM [5])

Now, you have two points to customize SM: First you can specify get_apdu and
free_apdu (which should really be renamed in something like sm_wrap_apdu and
sm_unwrap_apdu). Second, you can change the call back functions that actually
calculate a cryptogram or a mac using external keys. The second hook does not
need to be aware to smart card stuff.

> > [1] 
> > http://www.opensc-project.org/pipermail/opensc-devel/2010-October/015121.html
> > [2] 
> > https://github.com/OpenSC/OpenSC/blob/staging/src/libopensc/card-epass2003.c
> > [3] https://github.com/OpenSC/OpenSC/blob/staging/src/libopensc/sm.h

[4] ftp://ftp.cenorm.be/Public/Cwas/e-europe/esign/cwa14890-01-2004-Mar.pdf
[5] http://vsmartcard.sourceforge.net

-- 
Frank Morgner

Virtual Smart Card Architecture http://vsmartcard.sourceforge.net
OpenPACE                        http://openpace.sourceforge.net
IFD Handler for libnfc Devices  http://sourceforge.net/projects/ifdnfc

Attachment: pgpXoY3EdN5f5.pgp
Description: PGP signature

_______________________________________________
opensc-devel mailing list
opensc-devel@lists.opensc-project.org
http://www.opensc-project.org/mailman/listinfo/opensc-devel

Reply via email to