Starcos 3.2 #1347

jpecholt · 2018-04-28T10:33:25Z

Added support for Starcos 3.2. cards.
Encryption, signature and decryption functionalities were implemented.
GetSerialnumber and PIN-info are supported.
Superfluous "User Pin" removed from card label.

Starcos 3.2. cards were used during testing.

Documentation is added and updated
PKCS#11 module is tested. Note: Starcos 3.2. only generates random numbers of 8-bytes length, which causes the generateRandom test case (./pkcs11-tool --test) to fail, since it excepts a different length.

frankmorgner · 2018-05-23T12:27:53Z

Well, the current (and past) approach seems very hacky. Though we could continue with it until we hit a starcos version that's used in multiple configurations, it is also possible to try and be more generic, at least with Starcos 3.X. For example, which operation is allowed with what key, is specified in EF.KEYD. If parsed, it indicates which exact operation and mechanism to use for signature and encryption...

frankmorgner

Although I'd better like to see the real card versions integrated instead of more and more card profiles (which by accident are based on different card versions), I'm aware that it's hard to perform a full restart with the given code base. Hence, I would accept your hack, but please have a look at the inline comments...

frankmorgner · 2018-07-04T14:31:08Z

src/libopensc/card-starcos.c

+ sc_debug(ctx, SC_LOG_DEBUG_NORMAL, "Found alg ref id%02x\n",env->algorithm_ref & 0xFF );
+ *p++ = 0x80;
+ *p++ = 0x01;
+ *p++ = env->algorithm_ref & 0xFF;


missing length check

frankmorgner · 2018-07-04T14:31:34Z

src/libopensc/card-starcos.c

+ }
+ if(env->algorithm == SC_ALGORITHM_DES){
+ return -1; //for now, not supported
+ /**p++= 0x89;


please remove dead code

frankmorgner · 2018-07-04T14:31:56Z

src/libopensc/card-starcos.c

+ *p++= 0x13;//signature
+ *p++= 0x23;//PKCS RSA (standard)
+ if (env->algorithm_flags & SC_ALGORITHM_RSA_HASH_SHA1){
+ //TODO: not tested yet


please remove untested code

frankmorgner · 2018-07-04T14:34:29Z

src/libopensc/card-starcos.c

@@ -268,7 +406,7 @@ static int process_fci(sc_context_t *ctx, sc_file_t *file,
 return SC_SUCCESS;
 }

-static int process_fci_v3_4(sc_context_t *ctx, sc_file_t *file,
+static int process_fci_v3_2_v3_4_v3_5(sc_context_t *ctx, sc_file_t *file,


maybe renaming this to process_fci_v3 would be useful

Starcos 3.1 and 3.6 are not included, that's the reason for this naming scheme. Maybe process_fci_v3_2-5 ? Or solely put this information in a comment above the function and name it process_fci_v3?

there is no 3.1 or 3.6 at the moment, hence there is only 3.x and everything else. just change the naming.

frankmorgner · 2018-07-04T14:34:41Z

src/libopensc/card-starcos.c

@@ -304,7 +442,7 @@ static int process_fci_v3_4(sc_context_t *ctx, sc_file_t *file,
 return SC_SUCCESS;
 }

-static int process_fcp_v3_4(sc_context_t *ctx, sc_file_t *file,
+static int process_fcp_v3_2_v3_4_v3_5(sc_context_t *ctx, sc_file_t *file,


see above comment on the naming

src/libopensc/card-starcos.c

frankmorgner · 2018-07-04T14:49:21Z

src/libopensc/card-starcos.c

@@ -1857,7 +2165,8 @@ static struct sc_card_driver * sc_get_driver(void)
 starcos_ops.card_ctl = starcos_card_ctl;
 starcos_ops.logout = starcos_logout;
 starcos_ops.pin_cmd = starcos_pin_cmd;
-
+ starcos_ops.list_files = NULL;


this is not needed

frankmorgner · 2018-07-04T14:50:07Z

src/pkcs11/framework-pkcs15.c

@@ -1071,7 +1071,7 @@ pkcs15_init_slot(struct sc_pkcs15_card *p15card, struct sc_pkcs11_slot *slot,
 pin_info = NULL;
 }
 else {
- if (auth->label[0] && strncmp(auth->label, "PIN", 4) != 0)
+ if (auth->label[0] && strncmp(auth->label, "PIN", 4) != 0 && strncmp(auth->label, "User Pin", 9) != 0)


what do you need this change for?

The term User PIN is normally used to differentiate from SO PIN or Signature PIN.

Without this string compare, the label is copied into the tokeninfo label, which causes the inserted card to be displayed as "User Pin()" instead of "". At the same time, the closing bracket is often cut off. With the additional clause, the string is not copied and hence not shown.

It's intended to be shown as PIN LABEL (CARD LABEL). It seems your card doesn't have set p15card->tokeninfo->label. A better approach would be to only append (CARD LABEL) if p15card->tokeninfo->label is not empty.

Both auth->label and p15card->tokeninfo->label are not empty, Github just removed the strings surrounded by angle brackets from my example. What the output shows is "User Pin(Example card" -where the closing bracket is oftentimes missing- instead of the desired "Example card".

As explained above, I don't think this should be done for all cards. Either do this specifically for your card in something like pkcs15-starcos.c or create a configuration option that allows setting the pkcs#11 slot labels.

I am not as well-versed in the code structure and order of execution of certain functions: Is either one of the pkcs15-starcos.c::starcos_init_card( ) or pkcs15-starcos.c::starcos_finalize_card( ) functions definitely going to be called After the above function pkcs15_init_slot( ) ? In that case, one could change the card label in pkcs15-starcos.c, otherwise it might be overwritten again.

please remove this change. We'll think about configuring the slot naming seperately.

frankmorgner · 2018-09-13T13:47:43Z

If you look at the inline comments, we can merge this for the next release...

jpecholt · 2018-09-15T16:27:27Z

I undid changes concerning the renaming of the card lable in framework-pkcs15.c:1089 and integrated all other requests. I hope this satisfies all requirements.

frankmorgner · 2018-09-22T22:32:41Z

There's some error handling missing with malloc

jpecholt · 2018-09-23T11:56:55Z

As far as I could see, after each malloc, a pointer check is performed (Though this is not shown here, it is visible in the file)?

frankmorgner

the result of malloc was checked, indeed. But there is still some dead code lying around, for which I don't see a good reason.

frankmorgner · 2018-09-23T22:12:30Z

src/libopensc/card-starcos.c

- sc_format_apdu(card, &apdu, SC_APDU_CASE_3_SHORT, 0x22, 0x81,
- 0xb8);
+ // adapt last byte : FIXME DIRTY HACK (somehow works?)
+ p += env->key_ref_len -1;


what are you trying to achieve? As a general rule, don't use hacks!

There is a discrepancy between provided key reference number by the card and reference number expected by it. This was tested on multiple cards and compared to other drivers which also receive the same numbers by the card. Even though e.g. a 0x84 is given as a key reference number when the card is read out, it produces an error when later specifying this exact key reference number for an operation. For an unknown reason, it always expects a number which is smaller by three, 0x81 in the example. The reason for this could not be found after a thorough in depth search, so we left it at that. I see that there needs to be an additional check concerning a zero length key reference number, though.

What is the exact key reference and the exact value you need to code in the MSE? What's the content of EF_KEYD?

I think understanding EF_KEYD is key in avoiding hacks like this. Please read the documentation and try to find some hints.

I am fairly sure I tried the following, but I will look this up to confirm it:
I was not able to access any EF_KEYD file as an additional source of information. Neither was this file accessed by any other driver, implementation or program which could be used as a reference. The documentation did not provide any further helpful information. Additionally, OpenSC chooses a key with the appropriate usage flags (see e.g. pkcs15-framework.c:pkcs15_prkey_sign, line 3773), whose reference number is written into the key-ref-pointer. This number is the exact same as the one which was initially read out from the Starcos Smart-Card. It is later not accepted as a valid ref-number anymore, the reported error code deeming the key not suitable or the number invalid. No information in the documentation suggests a change in or an adaption of the key reference number.

Did you read the starcos reference manual? Did you search for EF_KEYD?

Sorry for being picky, but this hack is a little too problematic. Unfortunately, I currently don't have more time to implement this myself...

I completely understand. I spent an extensive amount of time myself trying to find the reason for this. This also lead to the fact that I compared the complete communication process between driver and card for different implementations, none was making use of this EF_KEYD file or gaining any further relevant information. I am certain that the information found in the manual was not sufficient, but I am going to check that again tomorrow and come back to you.

frankmorgner · 2018-09-23T22:13:21Z

src/libopensc/card-starcos.c

+ * Not supported yet
+ * TODO: check one-sided symmetric
+ * double-sided symmetric */
+ return 0;


Here is more dead code

frankmorgner · 2018-09-23T22:16:27Z

src/libopensc/card-starcos.c

+ "not supported for STARCOS 3.2 cards"); \
+ return SC_ERROR_NOT_SUPPORTED; \
+ } \
+ } while (0);


should be merged with CHECK_NOT_SUPPORTED_V3_4

There is one additional operation which was not tested on Starcos 3.4. Apart from this, one could merge both checks and implement a manual error check at that point.

frankmorgner · 2018-09-23T22:18:07Z

src/libopensc/card-starcos.c

+ } else if(card->type == SC_CARD_TYPE_STARCOS_V3_2) {
+ //TODO: for now: not supported (Internal authenticate used instead)
+ return SC_ERROR_NOT_SUPPORTED;
+ #if 0


remove dead code

frankmorgner · 2018-09-24T08:17:53Z

src/libopensc/card-starcos.c

+ if(env->flags & SC_SEC_ENV_KEY_REF_SYMMETRIC){
+ /* For now, signature sometimes runs into this case -> return 0, not -1 to continue signature procedures
+ * Not supported yet. */
+ return 0;


should be an error

I'll update the comment, thanks.

frankmorgner · 2018-09-24T08:18:20Z

src/libopensc/card-starcos.c

+ } while (0);
+
+/*Finds appropriate alg based on sec environment and writes these into the ptr
+* returns number of bytes which were added, 0 if no alg specified. -1 if not supported


-1 or an error code?

Error code, thank you.

src/libopensc/card-starcos.c

frankmorgner · 2020-11-25T07:39:52Z

Closing this issue due to inactivity. Please re-open the ticket if more input is available.

Starcos 3.2 support

ed05e9e

frankmorgner requested changes Jul 4, 2018

View reviewed changes

OpenSC deleted a comment from jpecholt Jul 4, 2018

Incorporating requested changes

6edb4ec

jpecholt force-pushed the Starcos-3.2 branch from e2001b3 to 6edb4ec Compare September 15, 2018 15:48

frankmorgner requested changes Sep 23, 2018

View reviewed changes

Removal of dead code and superfluous lines

a242926

frankmorgner requested changes Sep 24, 2018

View reviewed changes

rumpelsepp mentioned this pull request Apr 16, 2020

Starcos 3.2 support #2006

Closed

frankmorgner closed this Nov 25, 2020

Starcos 3.2 #1347

Starcos 3.2 #1347

Conversation

jpecholt commented Apr 28, 2018

frankmorgner commented May 23, 2018

frankmorgner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frankmorgner commented Sep 13, 2018

jpecholt commented Sep 15, 2018

frankmorgner commented Sep 22, 2018

jpecholt commented Sep 23, 2018

frankmorgner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpecholt Sep 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frankmorgner commented Nov 25, 2020

jpecholt Sep 24, 2018 •

edited

Loading