FINeID: Initial FINeID v3 support

[jpakkane]

This is the closed mr #1608 rebased against current master. There are no code changes from that version.

[frankmorgner]

So the comments from #1608 still apply!?

[jpakkane]

Yes. I can work on fixing the issues if someone tells me which ones are still relevant and should be done first.

[Jakuje]

Yes. I can work on fixing the issues if someone tells me which ones are still relevant and should be done first.

I think all of them are still relevant and the prioritization from #1608 (comment) still applies.

[jpakkane]

I started by doing the simple stylistic fixes and the logout one. I'll keep all my changes in separate commits for now to easily tell which changes are mine and which ones come from the original.

I'm not a subject matter expert (this is the first time I'm dealing with smart cards in any way) so some of the review comments don't make much sense. There are also some questions about them, for example one review says to use existing definitions (fineid.c line 106) but the code snippet given does not actually compile. To make it work I'd probably need to make the definition a macro and then use it in the two places. This seems a bit excessive.

[jpakkane]

Removed the unnecessary function and rebased. One of the review comments say this:

did you test pkcs15-init with your card? If not, don't implement SC_CARDCTL_GET_DEFAULT_KEY

I don't know what a proper test would entail but merely doing pkcs15-tool --list-keys prints this:

Using reader with a card: SCM Microsystems Inc. SCR 3310 [CCID Interface] (21120620213761) 00 00
Failed to connect to card: Card is invalid or cannot be handled

[Jakuje]

The pkcs15-init is used to enroll the card, write certificates and go any kind of administrative operations. I assume that for the national ID cards, general user will not have administrative keys so the comment was meant that the SC_CARDCTL_GET_DEFAULT_KEY should not be implemented, not much more.

If you are able to list the keys with pkcs11-toll -O, you should be able to list them also with pkcs15-tool --list-keys. If not, its something to have a better look into.

Right now, I ran the CI for you and the build fails. Can you check the errors?

[jpakkane]

Removed default_key and fixed one signedness bug. Some of the test failures seem strange, like the fuzz test that does not seem to touch this file at all.

[jpakkane]

Not sure what the outcomes should be but here's the console output:

$ src/tools/pkcs11-tool -L
Available slots:
Slot 0 (0x0): SCM Microsystems Inc. SCR 3310 [CCID Interface] (211206202137...
  (token not recognized)

$ src/tools/pkcs11-tool -O
Using slot 0 with a present token (0x0)

$ src/tools/pkcs15-tool --list-keys
Using reader with a card: SCM Microsystems Inc. SCR 3310 [CCID Interface] (21120620213761) 00 00
Failed to connect to card: Card is invalid or cannot be handled

[Jakuje]

Not sure what the outcomes should be but here's the console output:

$ src/tools/pkcs11-tool -L
Available slots:
Slot 0 (0x0): SCM Microsystems Inc. SCR 3310 [CCID Interface] (211206202137...
  (token not recognized)

$ src/tools/pkcs11-tool -O
Using slot 0 with a present token (0x0)

$ src/tools/pkcs15-tool --list-keys
Using reader with a card: SCM Microsystems Inc. SCR 3310 [CCID Interface] (21120620213761) 00 00
Failed to connect to card: Card is invalid or cannot be handled

This looks like the card is not recognized here.

Your changes also break the oseid tests so it looks like there is still something wrong:

card init
---------
running pkcs15-init, SOPIN=00000000 SOPUK=00000000
Using myeid profile for OsEID card, please install /usr/share/opensc/oseid_0.22.profile
Press ENTER continue, or CTRL-C to abort
Using reader with a card: OsEIDsim 00 00
Failed to create PKCS #15 meta structure: Invalid ASN.1 object
init fail

[jpakkane]

Upon further examination it turns out that my card is not v3 but in fact v5 so this driver would not work on it in any case. There's very little point in keeping this open any more so closing.

[Jakuje]

how hard/different would it be to adjust the code for the v5 version?

[jpakkane]

Honestly, I have no idea. I did try to look a bit but just the fact that the code builds with Autotools and just getting it to behave reasonably in a debugger under an IDE was so unpleasant that I gave up.

Though if you are open to the idea of migrating to build with Meson, then I can do that with reasonable effort. It would make the dev experience a lot nicer, especially for newcomers.

[Jakuje]

I am ok with adding a meson build system if you are interested in working on that. We already did that with other project (libcacard) so I should be able to help there too. But we would probably need to maintain both of them at a time.

Or just open an issue so we can see if there would be large interest for this.

[jpakkane]

But we would probably need to maintain both of them at a time

Out of curiosity, why is that? Meson works on most of the esoteric platforms such as Solaris and HP-UX. I guess AIX is the only real holdout but there are even people working on that, FWICT.

If this is the case then a workable approach is to specify that Meson is the default one and the people who can't switch to it get to maintain the Autotools build themselves. In this way the maintenance burden lies solely on the people who benefit from it as opposed to everybody else.

Requiring contributors to update both build systems for their pull requests is probably even worse than the current setup.

[Jakuje]

Probably because some people are afraid of changes :)

Indeed it will not be forever. But there are really no significant changes going on in the build system of opensc itself for recent years and adding a file to three lists instead of two now (autotools and windows Makefile.mak -- not sure why we have it though and if it is needed, but AppVeyor builds need that now and)

[Jakuje]

I filled #2577 to support meson. @jpakkane Did you start working on something toward this or is it ok to ask @xhanulik to have a look into that?

[jpakkane]

I haven't looked into it yet, have been busy with other stuff. I was hoping to get something done this week But if someone else is itching to start then by all means go for it.