[Pcsclite-muscle] Support for multiple devices with ifdnfc, serial hotplug, and other improvements.

[Ben Mehlman]

Hello all..  Ludovic, Frank Morgner.. any users of ifdnfc...

So I am using ifdnfc for a project where I need to communicate with several
pn532_serial readers, and I discovered that it was not able to work with
multiple readers, and it also seemed to have some memory handling problems,
outputting garbage characters when it shouldn't, etc.

So I worked on it and was able to add support for multiple readers.  And in
the process of doing that, I found some coding issues I was able to
resolve, and I added some other features that make things work more
reliably.  All of which I would like to give to the project.

It ended up being a lot of changes, so I thought that it would be better
for me to explain on this list what I did, so that people will understand
them, and if some of my changes aren't ok it could be discussed and I could
change them if necessary.

What I found/ What I did:

In IFDHCreateChannelByName, the ifd_devices entry was defaulting to index=0
for any device other than those configured with a "usb:" connstring.  This
resulted in a segfault if ifdnfc was loaded more than once for one of these
devices.  This was fixed so that each device gets its own ifd_devices
entry, allowing multiple non-usb devices.

For non-usb devices, the DEVICENAME in the pcscd configuration used to be
ignored by ifdnfc.  In order to do the nfc_open, ifdnfc-activate would
query libnfc to get the connstring from the libnfc configuration.  If there
was more than one device in libnfc, a human interaction would have to occur
to select the correct nfc device.  None of this would work very well for
multiple devices, especially the human interaction part.

I changed it so that it now works with two different configuration styles,
one compatible with the code as it exists today (so that I wouldn't break
the environment of anyone currently using ifdnfc), and a new one that
allows multiple devices and some other things:

Old config (as it is today):

FRIENDLYNAME "IFD-NFC"
DEVICENAME /dev/null
LIBPATH
/usr/lib/pcsc/drivers/ifdnfc.bundle/Contents/Linux/libifdnfc.so.0.1.4

There can only be one ifdnfc device since there can be only one named
"IFD-NFC".  DEVICENAME is not supplied in this file for serial devices
using the current code.  In this case we use the old behavior of running
ifdnfc-activate which checks with libnfc to get the nfc_connstring, then
use it to open the device.  If there's more than one device configured for
libnfc, ifdnfc-activate will ask the user to choose a device.  This is all
the same as before.

New config (with the changes I've made):

FRIENDLYNAME "IFD-NFC-Reader1"
DEVICENAME pn532_uart:/dev/ttyS0
LIBPATH
/usr/lib/pcsc/drivers/ifdnfc.bundle/Contents/Linux/libifdnfc.so.0.1.4

FRIENDLYNAME "IFD-NFC-Reader2"
DEVICENAME pn532_uart:/dev/ttyS1
LIBPATH
/usr/lib/pcsc/drivers/ifdnfc.bundle/Contents/Linux/libifdnfc.so.0.1.4

..and so on up to 10 readers (default)

The devicename is now the nfc_connstring, so there's no more ambiguity as
to which nfc device belongs to which pcsc reader, there's no need to query
libnfc for the connstring, and the user is never asked to choose the nfc
device.

In the IFDHCreateChannelByName I look to see if the devicename passed looks
like something that nfc_open would accept.. simply it checks to see if the
name contains a colon.  If so, we know this is the new configuration
style.  In this case, we try to nfc_open() the device.  Whether we can
successfully open the device or not, the connstring is copied for later
use.  It is not necessary to use ifdnfc-activate.

If nfc_open() doesn't work when the channel is created.. for example if the
reader isn't powered or connected, it is ok.  To allow "hot plug" of serial
devices, I added retry logic driven by the polling behavior of pcscd when
used with the pn532_serial.. every 15 seconds, ifdnfc will attempt
nfc_open() on unopened devices.  This is done in IFDHICCPresence() which,
at least for the pn532_uart.. since it is a polled device we get called
every half second and have the opportunity to try again to nfc_open the
device.

ifdnfc-activate was significantly rearranged to allow control of multiple
ifdnfc devices.  As I said, using this configuration style, the driver will
try to open all devices on startup.  If this is all you need then there's
no need to run ifdnfc-activate.  But if you want to control them manually,
to shut them down, switch to se mode, or if you want to check the status,
you can do that the same as before, except that the commands will be
applied to all readers named starting with "IFD-NFC".

If you have multiple devices and want to control them individually rather
than all at once, you can pass the name to ifdnfc-activate, eg:

ifdnfc-activate off IFD-NFC-Reader1

This is actually a substring match so if you have your readers grouped by
name you can control the group with a left hand match.

There were some problems before where if ifdnfc-activate was used multiple
times to stop and start the device it would leak devices and as well there
were null termination issues in places with the string handling in the
control messages that resulted in garbage output and core dumps at times.
This was cleaned up and simplified to have a simple C structure passed back
and forth replacing all the pointer manipulation in the existing message
format.  With the low volume of these transactions it was simply not worth
it to fix the variable length message code.  Fixed length messages are much
safer and it was easier to add additional data to the response message for
better status messages.    There were also similar problems in the driver
itself where for example the control structure for the Lun was tagged for
reuse when a close command was sent by ifdnfc-activate.. but pcscd had not
actually closed the channel.  This led to a segfault if you tried to turn
that reader back on again.  I made it so that once the channel is allocated
it is not deallocated until pcscd closes it.

So it's now ok to run ifdnfc-activate any time you want to open and close
devices, or to enable SE mode, and the output of ifdnfc-activate will tell
you more about the status of each device than it used to:  What mode it is
in.. mode being what we ASKED it to do with ifdnfc-activate (inactive,
active, or active se), and then what the actual status of the device is at
this time: whether it is actually connected (nfc_open was successful), and
whether it is actually in se mode.  A lot of the other error outputs in
ifdnfc-activate were improved and made more consistent.

So that's about it.

You can look at the code @
https://github.com/benmehlman/ifdnfc/tree/serial-nfc-open-on-startup

I am sorry that it is a bit hard to follow the individual changes.. I
started out with a branch for each change I was going to make.. but what
happened was, when I first started out, I thought I'd be making only a few
small changes, and it wasn't until I got into it that I really understood
what needed to be done, and by that time everything had been done in the
wrong order and couldn't be easily separated anymore without my starting
over from the beginning.

However if anything I have done does not make sense please ask and I'll
explain.

Thanks!
Ben Mehlman
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.alioth.debian.org/pipermail/pcsclite-muscle/attachments/20160822/cf64b851/attachment-0001.html>