support for Slovak eID cards #2672

jurajsarinay · 2022-12-31T02:28:21Z

Checklist

New files have a LGPL 2.1 license statement
PKCS#11 module is tested
Windows minidriver is tested
macOS tokend is tested

I propose to add a new driver to handle eID cards issued by Slovakia from mid-2021 until November 2022.

Driver

The card runs CardOS 5.4, the new driver is therefore a stripped-down version of card-cardos.c The only place where I have to diverge from the original driver is set_security_env, because the card expects MSE RESTORE instead of MSE SET. I abuse key_ref to store the corresponding seIdentifier, as our pkcs#15 structures do not include the entry.

Because the card shares the ATR with other CardOS 5.4 cards, the new driver precedes cardos-driver within internal_card_drivers[].

PKCS#15 emulation

Within EF.DIR there are 5 applications. The last two carry (apparently not entirely usable) PKCS#15 structures.

pkcs15-skeid.c binds the fourth application in the list. Otherwise sc_pkcs15_bind_internal would get called and create an unusable token. In the case of the fifth application this is prevented by SC_PKCS11_FRAMEWORK_DATA_MAX_NUM = 4.

Because there is no point in calling sc_pkcs15_bind_internal for this card, I added it to sc_pkcs15_is_emulation_only. This does not prevent sc_pkcs15_bind_internal from getting called though (if synthetic binding was unsuccessful).

I consider this behaviour a bit counterintuitive. I mention it here to report on and justify what I have done, it has no noticeable effect on my driver (any more). Let me know if sc_pkcs15_is_emulation_only warrants a separate GitHub issue.

PINs

There is a global User PIN labeled BOK. The qualified certificate (key) requires user consent and a separate (local) Signature PIN labeled KEP. The "official" proprietary PKCS#11 module requires both the codes for every signature. Fortunately, the card is happy with the Signature PIN only, as there seems to be no convenient way to have multiple PIN codes per slot.

I considered emulating (parts of) the "official" behaviour wihtin a custom pin_cmd that contained the following:

    if (data->pin_reference == 0x87 && data->cmd != SC_PIN_CMD_CHANGE && data->pin_type != SC_AC_CONTEXT_SPECIFIC)
        {
               sc_log(card->ctx, "Non-specific KEP PIN encountered, handling it as BOK instead.");
               data->pin_reference = 0x03;
       }

I ultimately decided against the idea. It adds complexity (or confusion) and provides little benefit. I mention the issue because it is connected to a failure in pkcs11-tool --test --slot 1 --login: log.

Because the local Signature PIN is used for the session, test_verify() fails here.

The card enforces CKA_ALWAYS_AUTHENTICATE and therefore reports that the Signature PIN is (no longer) verified, apparently because signatures have been computed during test_signature(). The only effect of this is that the built-in test fails even though the token works (reasonably) well.

The above pin_cmd hack would result in a passed pkcs11-tool --test --slot 1 --login.

I include this information mainly to justify a PR with a failed test attached. If the behaviour of pkcs11-tool --test in the context of a local Signature PIN and user_consent warrants a separate GitHub issue, do please let me know.

For completeness, here is the output of pkcs11-tool --test --slot 0 --login.

src/libopensc/pkcs15-skeid.c

martinpaljak · 2023-01-01T15:17:10Z

@jurajsarinay don't take the tests at gold standard value, if it can be fixed in a generalized way to take into account "always authenticate" behavior, would be a lovely addition together with this PR.

… software distributed by the government

src/libopensc/pkcs15-syn.c

src/libopensc/pkcs15-skeid.c

src/libopensc/card-skeid.c

Jakuje · 2023-01-03T11:06:19Z

src/libopensc/card-skeid.c

+	}
+
+	key_id = env->key_ref[0];
+	r = sc_restore_security_env(card, key_id);


This is at least weird. I would be probably for reimplementing the iso7816_restore_security_env (its just one APDU) with some comments, instead of reusing it for setting the security environment (with the key id instead of security environment number). The restore of security environment is completely different operation and that it works is probably pure coincidence.

pure coincidence

... of design & luck.

I need to bolt this on top of OpenSC in one way or another. Because of use_key() there needs to be a meaningful sc_set_security_env() for the card.

OpenSC/src/libopensc/pkcs15-sec.c

Line 151 in 7077173

r = sc_set_security_env(p15card->card, senv, 0);

MSE RESTORE is what the proprietary module does. The operation is simply a shorthand for MSE SET, that's why I hoped this would not be viewed as utter heresy.

If others dont have issue with this. I am ok with keeping this as long as it will contain some comment in the code that this is what the official driver does. Without comment it still looks like an error (and comments on github are not guaranteed to be accessible forever).

src/libopensc/card-skeid.c

frankmorgner

Please add your card's ATR to the Windows installation so that it will be supported via Windows' Minidriver via https://github.com/OpenSC/OpenSC/blob/master/win32/customactions.cpp

src/libopensc/ctx.c

frankmorgner · 2023-01-05T08:08:14Z

I wonder if card-skeid.c is needed at if the difference from card-cardos.c is so small. card-cardos.c is written in a generic way, which the slowak card doesn't need. I think it should be better to use skeid_known_url() in cardos_init to get the card type and later to use the card type in cardos_set_security_env to implement the workaround for the slowak card.

@Jakuje , since you are familiar with the cardos specification, do you know where the use of MSE RESTORE originates and if it could be integrated in a general way into card-cardos.c?

jurajsarinay · 2023-01-05T09:17:25Z

I wonder if card-skeid.c is needed at if the difference from card-cardos.c is so small.

That was my original plan. I went for a separate driver, because I did not feel comfortable touching (old) code in the CardOS driver(s).

Apart from using a specific sc_set_security_env() I also simply use iso7816_compute_signature() and iso7816_decipher() instead of the CardOS-specific functions.

…overnment

frankmorgner · 2023-01-05T12:48:48Z

Apart from using a specific sc_set_security_env() I also simply use iso7816_compute_signature() and iso7816_decipher() instead of the CardOS-specific functions.

OK, fair enough, card-skeid.c is simple enough to be maintained seperately.

src/libopensc/ctx.c

… expression

andrewshadura · 2023-01-28T09:46:14Z

@jurajsarinay, you should probably rebase your commits on top of the current master instead of merging it in.

jurajsarinay · 2023-01-28T10:01:50Z

@jurajsarinay, you should probably rebase your commits on top of the current master instead of merging it in.

This is a result of clicking "Sync fork" within GitHub, I should probably have refrained from that :-)
I would very much prefer to avoid GitHub altogether, yet when in Rome, do as the Romans do.

andrewshadura · 2023-01-28T10:02:48Z

Oh, I never used that button, I always rebase and force-push manually 🙂

Jakuje

minor whitespace nits, but otherwise looks good

src/libopensc/card-skeid.c

Jakuje · 2023-01-30T15:06:36Z

src/libopensc/card-skeid.c

+	}
+
+	key_id = env->key_ref[0];
+	r = sc_restore_security_env(card, key_id);


If others dont have issue with this. I am ok with keeping this as long as it will contain some comment in the code that this is what the official driver does. Without comment it still looks like an error (and comments on github are not guaranteed to be accessible forever).

src/libopensc/card-skeid.c

src/libopensc/pkcs15-skeid.c

minor whitespace nits Co-authored-by: Jakub Jelen <[email protected]>

…y_env()

jurajsarinay · 2023-01-31T10:25:29Z

minor whitespace nits

Thank you, @Jakuje!

nervous-inhuman · 2023-03-22T02:43:17Z

@frankmorgner Any updates as to when this could possibly be merged?

frankmorgner · 2023-03-22T08:20:58Z

done, thank you

jurajsarinay marked this pull request as draft December 31, 2022 02:41

support for Slovak eID cards - GitHub export

174f9aa

jurajsarinay marked this pull request as ready for review December 31, 2022 08:40

martinpaljak reviewed Jan 1, 2023

View reviewed changes

src/libopensc/pkcs15-skeid.c Outdated Show resolved Hide resolved

martinpaljak reviewed Jan 1, 2023

View reviewed changes

src/libopensc/pkcs15-skeid.c Outdated Show resolved Hide resolved

renamed certificates and PIN codes to match the names used within the…

d378951

… software distributed by the government

Jakuje requested changes Jan 3, 2023

View reviewed changes

frankmorgner requested changes Jan 4, 2023

View reviewed changes

src/libopensc/ctx.c Show resolved Hide resolved

tabs instead of spaces

ea88559

jurajsarinay added 4 commits January 5, 2023 10:36

removed unnecessary newlines

55a4c8c

changed token manufacturer to match the software distributed by the g…

b20640b

…overnment

Pull definitions of contstants out of for loops.

5228ed5

fix initialization within minidriver

4597bbd

frankmorgner requested changes Jan 5, 2023

View reviewed changes

src/libopensc/ctx.c Show resolved Hide resolved

jurajsarinay added 3 commits January 5, 2023 14:43

comments explaining the relation to cardos

98fc011

suppress -Werror=parentheses by adding extra parentheses to a logical…

7d4397a

… expression

Merge branch 'OpenSC:master' into skeid

4b43036

jurajsarinay requested a review from Jakuje January 28, 2023 09:45

Jakuje reviewed Jan 30, 2023

View reviewed changes

jurajsarinay and others added 2 commits January 30, 2023 20:00

Apply suggestions from code review

e28d6fa

minor whitespace nits Co-authored-by: Jakub Jelen <[email protected]>

justify the use of sc_restore_security_env() within skeid_set_securit…

546370e

…y_env()

frankmorgner approved these changes Feb 7, 2023

View reviewed changes

Jakuje approved these changes Mar 22, 2023

View reviewed changes

frankmorgner merged commit d952a9f into OpenSC:master Mar 22, 2023

jurajsarinay deleted the skeid branch March 27, 2023 19:56

jurajsarinay mentioned this pull request Apr 10, 2023

support the latest generation of Slovak eID #2745

Closed

5 tasks

jurajsarinay mentioned this pull request May 28, 2023

Support D-Trust Card 4.1 (Std. RSA 2ca) #2784

Closed

xhanulik mentioned this pull request Jul 3, 2023

New release 0.24.0 #2792

Closed

hamarituc mentioned this pull request Nov 26, 2023

Add support for RSA D-Trust Card 4.1 & 4.4 #2943

Merged

4 tasks

support for Slovak eID cards #2672

support for Slovak eID cards #2672

Uh oh!

Conversation

jurajsarinay commented Dec 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Driver

PKCS#15 emulation

PINs

Uh oh!

Uh oh!

Uh oh!

martinpaljak commented Jan 1, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jakuje Jan 3, 2023

Choose a reason for hiding this comment

Uh oh!

jurajsarinay Jan 5, 2023

Choose a reason for hiding this comment

Uh oh!

Jakuje Jan 30, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

frankmorgner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

frankmorgner commented Jan 5, 2023

Uh oh!

jurajsarinay commented Jan 5, 2023

Uh oh!

frankmorgner commented Jan 5, 2023

Uh oh!

Uh oh!

andrewshadura commented Jan 28, 2023

Uh oh!

jurajsarinay commented Jan 28, 2023

Uh oh!

andrewshadura commented Jan 28, 2023

Uh oh!

Jakuje left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jakuje Jan 30, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jurajsarinay commented Jan 31, 2023

Uh oh!

nervous-inhuman commented Mar 22, 2023

Uh oh!

frankmorgner commented Mar 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jurajsarinay commented Dec 31, 2022 •

edited

Loading