Thanks for looking into this.

What I find confusing is the mix of the additional parameter to sc_pkcs15_read_* and the added parameter to struct sc_pkcs15_card_opts. If I'm not mistaken, the parameter struct sc_pkcs15_card *card contains the new configuration option, so that the additional function's parameter isn't necessary, don't you think?

opensc.conf also allows a special handling of PIN-protected certificates. Unfortunately, the naming private_certificate is not really exact and may confuse users with the private data this PR is targetting. What do you think, should we unify or rename the configuration options?

Also, instead of adding a new config option, we could think about making use_file_caching a multi-option instead of a boolean one. We could use the terms yes(all), no and public for keeping backward compatibility and for introducing the new option to only cache the public data from the card.

What I find confusing is the mix of the additional parameter to sc_pkcs15_read_* and the added parameter to struct sc_pkcs15_card_opts. If I'm not mistaken, the parameter struct sc_pkcs15_card *card contains the new configuration option so that the additional function's parameter isn't necessary

The struct sc_pkcs15_card *p15card holds the value of the configuration option as p15card->opts.cache_private_data, which says whether the private data can be cached or not.

The additional parameter private_obj to sc_pkcs15_read_file() states, whether the read object is marked as private or not. As sc_pkcs15_read_data_object() and sc_pkcs15_read_certificate() call sc_pkcs15_read_file() but do not work with struct sc_pkcs15_object which contains the needed flag, the private_obj parameter is passed there as well. However, it can be simplified, if sc_pkcs15_read_data_object() and sc_pkcs15_read_certificate() would take the whole object, or directly sc_pkcs15_read_file() may check, whether the object.

the naming private_certificate is not really exact and may confuse users with the private data this PR is targetting

I found it a bit confusing too. It would help to rename it - maybe something like pin_protected_certificated to get rid of private_?

We could use the terms yes (all), no and public for keeping backward compatibility and for introducing the new option to only cache the public data from the card.

Looks good to me, I will redo the handling of this option.

The struct sc_pkcs15_card *p15card holds the value of the configuration option as p15card->opts.cache_private_data, which says whether the private data can be cached or not.

Thanks for the clearification, I now understand the difference between the two. A different approach for avoiding caching on private data without changing sc_pkcs15_read_file() could look like this:

diff --git a/src/libopensc/pkcs15-pubkey.c b/src/libopensc/pkcs15-pubkey.c
index 9aceea64..1fcd5f4f 100644
--- a/src/libopensc/pkcs15-pubkey.c
+++ b/src/libopensc/pkcs15-pubkey.c
@@ -951,7 +951,12 @@ sc_pkcs15_read_pubkey(struct sc_pkcs15_card *p15card, const struct sc_pkcs15_obj
        }
        else if (info->path.len)   {
                sc_log(ctx, "Read from EF and decode");
+               struct sc_pkcs15_card_opts old_opts = p15card->opts;
+               if (obj->flags & SC_PKCS15_CO_FLAG_PRIVATE) {
+                       p15card->opts.use_file_cache = 0;
+               }
                r = sc_pkcs15_read_file(p15card, &info->path, &data, &len);
+               p15card->opts = old_opts;
                LOG_TEST_GOTO_ERR(ctx, r, "Failed to read public key file.");
 
                if ((algorithm == SC_ALGORITHM_EC || algorithm == SC_ALGORITHM_EDDSA || algorithm == SC_ALGORITHM_XEDDSA)

This would allow keeping the ABI stabile and it would block some of the complexity to drip through to the lower level. However, my review is purely informational - I'll leave it to you to decide which approach you find more suitable.

the naming private_certificate is not really exact and may confuse users with the private data this PR is targetting

I found it a bit confusing too. It would help to rename it - maybe something like pin_protected_certificated to get rid of private_?

Yes, this would be more suitable. Maybe just open an issue about changing the naming would be sufficient for now.

Yes, in sc_pkcs15_read_pubkey() opts can be directly checked and adjusted before calling sc_pkcs15_read_file(). At some places (such as sc_pkcs15_read_data_object() or sc_pkcs15_read_certificate()), the check can not be performed as the flags are not part of given parameters. That means it would be necessary to wrap also other functions with checking and adjusting p15card->opts.use_file_cache.

@frankmorgner do you have some other concerns around this PR or are we good to merge it?

no, fine for me