Add supported algorithms for OpenPGP Card (Fixes #1432)

[alex-nitrokey]

Tested with Nitrokey Pro (OpenPGP Card v2), Nitrokey Pro 2 (OpenPGP Card v3) and Nitrokey Start (Gnuk based).

Currently only used algorithms are list in card capabilities of OpenPGP Card based devices, instead of listing all supported algorithms. Now all possible RSA algorithm types are stored.

[hongquan]

Look good to me.

[frankmorgner]

Sorry, but from the specification I don't understand your notion of current. Specifically, https://gnupg.org/ftp/specs/OpenPGP-smart-card-application-3.3.pdf it defines the following:

 4.4.3.8 Supported algorithms
An OpenPGP smart card V3.0 or higher shall support the following algorithms with their
specific details.

Mandatory:
RSA 2048 or higher for PSO:CDS, PSO:DEC and INTERNAL AUTHENTICATE

(and nothing more), i.e. a minimal v3 card needs to only support rsa 2048. I'd like to keep the detection mechanism as is.

[frankmorgner]

would it be possible to modify src/pkcs15init/pkcs15-openpgp.c to add the missing algorithms?

[alex-nitrokey]

Sorry, but from the specification I don't understand your notion of current.

Well, it just does not detect the algorithms which can be used on the card, but only which are configured right now. Therefore it is not really useful to look for these only. In fact, using opensc-tool --list-algorithms will output rsa2048 three times on a factory-reset device, just because rsa2048 is configured for all three slots of the card. Every other possible algorithm will not be listed.

Please see 4.4.3.7 of the specification which states

The content of the DO is optionally changeable (announced in Extended Capabilities) with PUT DATA. This is useful if the card supports several algorithms or different key length. The attributes can be changed independent for each key, so it is possible for example to use different key length for signing and decrypting. A card should reject unsupported values in the DO. The supported values are manufacturer specific, please ask your card vendor for the correct parameters if the card supports changing of the attributes.

You are right that it is up to the manufacturer what algorithms are actually implemented. But as a) we don't have a proper way to find out the supported algorithm without just testing them, b) the card is supposed to refuse a algorithm it does not support anyway and c) these still seem to be the regularly supported algorithms, I would prefer to have these implemented than leave the user alone with a workaround to do the exact thing manually.

In fact, openpgp-tool already does this: it just assumes that an algorithm is working, sends the information to the card and if it refuses to use this one, the user gets a proper error message. pkcs15-init on the other checks the card->algorithms first (which only includes these already configured, as stated above) and therefore fails.

would it be possible to modify src/pkcs15init/pkcs15-openpgp.c to add the missing algorithms?

I don't see how this would be different to this approach? This would be an assumption about the supported algorithms, too. And it would not apply to other tools which may access the same information (card->algorithms). Or did I misunderstood?

Please let me know, if anything is unclear or you have another idea. This is the best I could come up with. As far as I can see there just is not extensive list on card...

[marschap]

@frankmorgner alex-nitrokey is right: according to my reading of the specs there is no on-card information about the algorithms supported by the card.
The PR looks good to me too.
Please be so kind to pull it into master.

[frankmorgner]

The problem I see is that in PKCS#11 all of the registered (but possibly unsupported) algorithms will be populated to the application:

OpenSC/src/pkcs11/framework-pkcs15.c

Lines 4955 to 4982 in 336b282

	/* For now, we just OR all the algorithm specific
	* flags, based on the assumption that cards don't
	* support different modes for different key *sizes*. */
	num = card->algorithm_count;
	alg_info = card->algorithms;
	while (num--) {
	switch (alg_info->algorithm) {
	case SC_ALGORITHM_RSA:
	if (alg_info->key_length < mech_info.ulMinKeySize)
	mech_info.ulMinKeySize = alg_info->key_length;
	if (alg_info->key_length > mech_info.ulMaxKeySize)
	mech_info.ulMaxKeySize = alg_info->key_length;
	rsa_flags |= alg_info->flags;
	break;
	case SC_ALGORITHM_EC:
	if (alg_info->key_length < ec_min_key_size)
	ec_min_key_size = alg_info->key_length;
	if (alg_info->key_length > ec_max_key_size)
	ec_max_key_size = alg_info->key_length;
	ec_flags |= alg_info->flags;
	ec_ext_flags |= alg_info->u._ec.ext_flags;
	break;
	case SC_ALGORITHM_GOSTR3410:
	gostr_flags |= alg_info->flags;
	break;
	}
	alg_info++;
	}

Please just try to make a simple comparison for your hack whether you're actually running inside pkcs15-init. It should look similar to this:

OpenSC/src/pkcs11/misc.c

Lines 450 to 454 in 4de0d06

	if (strcmp(ctx->app_name, "onepin-opensc-pkcs11") == 0) {
	conf->slots_per_card = 1;
	} else {
	conf->slots_per_card = 4;
	}

[alex-nitrokey]

The problem I see is that in PKCS#11 all of the registered (but possibly unsupported) algorithms will be populated to the application:

I don't want to be too insisting here (thus, I added the requested limitation), but can you please elaborate why this could be a problem?

Having the restriction to pkcs15-init implemented, the user can not use this code for e.g. listing all supported algorithms with opensc-tool --list-algorithms and may will get problems with other use cases as well. Honestly, I think this way of assuming algorithms is in most cases more accurate than just listing currently used ones.

I don't like the situation either. I just feel like this makes it a bit better. Of course I will totally accept your decision ;-)

[frankmorgner]

Some PKCS#11 applications are confused when seeing RSA3096 bit support without a key/certificate

[alex-nitrokey]

Okay, thanks.

[alex-nitrokey]

I just realized, that I used the wrong indentation, shall I fix this minor problem before this get merged?

[frankmorgner]

Yes, please fix this and we're good to merge.