Hello, On Jul 20, 2010, at 10:34 PM, Emanuele Pucciarelli wrote: > here's a patch for Italian CNS support against the current trunk, this > time without secure messaging. The plan is to reach a consensus on > this version, then add Secure Messaging, building on Viktor's work. OK.
> - in card-cardos.c, I changed the logic around line 820, because I > think that the previous handling of outlen was downright wrong – am I > missing something fundamental? Reading the code it all feels hairy, the patchwork that sets the flags in pkcs15.c and the try-and-fail approach in card-cardos.c Stricter mapping of actual card and its capabilities to something like card flags and proper failing with a debug message would be much better. Currently it seems to me that mysterious failures can happen. Exact knowledge of what is supported as input (hash, raw data, padded data etc), what gets fed to the card and what is expected as the return should be very explicit. As should be the hacks that are supported. I don't have any cardos cards to try it out. > - I kept the "Give random" function in iso7816.c. It doesn't really > belong to ISO 7816, but it does belong to ETSI TS 101 206-3, and it is > shared by many cards. What do you think about this? Should I confine > it to card-itacns.c, or is it OK to keep it there, os other (and > future) drivers could benefit from it? No real preference, I think a comment about ETSI origin in both the header file as well as source file would do just fine. > Any feedback is extremely welcome! I attached a modified patch to the original ticket (#177). Comments: * No dead code (#if 0 and #if 1) in the patch * The card should be read-only (correct?) and pkcs15-emulated, so it does not support key generation -> no flags should be set about it. In fact, I'm not sure SC_ALGORITHM_ONBOARD_KEY_GEN is useful at all. It should be checked if a pkcs15init driver exists that does currently not support key generation. * Same applies to SC_ALGORITHM_NEED_USAGE. Are you sure about it? * I don't understand "itacns_ops = *(sc_get_incrypto34_driver()->ops);" when initializing the card driver, why so? Why not use the ISO function overloading as other drivers do? * MAX_LE and ITACNS_MAX_PAYLOAD, would it make sense to resort on one single value that gets fed to card max_recv_size? > > Thanks, Thanks! -- Martin Paljak @martinpaljak.net +3725156495 _______________________________________________ opensc-devel mailing list opensc-devel@lists.opensc-project.org http://www.opensc-project.org/mailman/listinfo/opensc-devel
