Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libopensc: allow setting driver via OPENSC_DRIVER environment variable #882

Merged
merged 1 commit into from
Nov 20, 2016
Merged

libopensc: allow setting driver via OPENSC_DRIVER environment variable #882

merged 1 commit into from
Nov 20, 2016

Conversation

lbschenkel
Copy link
Contributor

@lbschenkel lbschenkel commented Oct 4, 2016

When you have cards with multiple supported applets (such as the YubiKey with OpenPGP+PIV and any other GlobalPlatform card) it is very convenient to be able to temporarily override the selected card driver and use a different applet than it would be selected by default without having to create and maintain a completely separate config file.

I'm proposing that the driver name could be specified via an environment variable, so in case the driver is not forced by the client the variable would override the auto-selection mechanism. I have been trying this for a while and it proved to be very useful, especially when using the PKCS#11 module: I can easily use the OpenPGP keys in one program and the PIV certificates in another. The change is so small and localized that I feel it is worth to incorporate that into the codebase.

This PR is for mostly to start this conversation and for feedback. The proposed variable name is just a suggestion.

@frankmorgner
Copy link
Member

I think having two PKI applets on a single card is really rare. Actually using two PKI applets on a single card is even more rare. Also, the PGP applet is likely to be used by gpg rather than by OpenSC. And if your on Windows, you are likely using the built in minidriver for PIV rather than OpenSC's minidriver.

I think having two config files is no big issue, so I'd opt for not merging this change.

@lbschenkel
Copy link
Contributor Author

lbschenkel commented Oct 5, 2016

OK, but I think I'm may not be the only one who bought GlobalPlatform cards with the specific intent of having multiple applets on them and having to carry just a single card. (I understand that I'm not a typical user, however.)

Personal opinions aside, what do you see as being the drawbacks of something like this being merged? Would the feature itself be a liability or cause problems to somebody? Is the code itself problematic?

Note that I'm just trying to follow your train of thought here, not making a big case that this should absolutely go in. (I can always run my own patched version in any case.) This will help me in any eventual future contributions.

@dengert
Copy link
Member

dengert commented Oct 6, 2016

I kind of like this modification, and think it should be considered.

But trying to use multiple apps on the same card can be a problem especially if you are trying to use one of the apps from a long running process while trying to use another app from another process.

The cards I have seen only allow one app to be active at a time. (they are like MS DOS where you could only run one program at a time.) You have to select the app, then login to use the app, which invalidates the login status of other apps that may have been active. Current host software is not aware that other processes may cause interference with the app they are using and may the have to request the PIN (or cache it) to get the login state back to where they were.

The host software is not always OpenSC. There is Java and vendor provided software that can interfere as well. PCSC does not keep track the login state or the selected app. It can only grant exclusive access or shared access to a process. PKCS#11 does not help much either.

@lbschenkel
Copy link
Contributor Author

I always thought that all software that uses PC/SC in non-exclusive mode shall be coded in a way that every time it talks to the card it starts a transaction (so it temporarily gets exclusive access to the card) and then it has to put the card in the desired state (selecting the right app, logging in, etc.) since any other process could have talked to the card in the meantime and interfered. Any software that uses shared mode and does not account for changes in the card state seems to be fatally flawed in my opinion.

@dengert
Copy link
Member

dengert commented Oct 6, 2016

You may call that flawed, but that is what people want to happen. Insert card, enter PIN once (maybe by pin pad reader which means the PIN can not be cached) and have access to it from all their applications. The software today can almost support this for read only cards used in PCSC shared mode, as long as only one applet used on the card and all host applications are careful to test if the card state is unlocked, and they don't reset or lose the login state. Even the OS can cause problems they may time out operations or power down the USB reader! See the opensc.conf "reader_driver pcsc" to use exclusive mode, or shared mode.

@lbschenkel
Copy link
Contributor Author

Right, I am not disagreeing with your stance however that already happens every day if you have a YubiKey for example and you use the OpenSC PKCS#11 library in Firefox/Thunderbird (which is talking to the PIV applet) and also run the Yubico Authenticator app (which is talking to the OATH applet).

I totally agree with what you are saying in terms of user experience, however it is entirely up to the software to be coded to provide that user experience — pretending that nothing else is talking to the card when the software explicitly said others could talk to the card is not good engineering in my opinion. If managing the card state is too cumbersome or impractical the software should request exclusive access, but that's what GnuPG does and it causes a horrible user experience in many other ways as well.

But the point I'm trying to make is that I don't see how that this PR makes matters worse. In fact it does not create any new problem that $OPENSC_CONF already doesn't create — however maintaining two config files is much more cumbersome than just changing the driver name because the config file contain much more than just the choice of driver and any change in one has now to be kept in sync with the other.

The main criticism could be that it makes things too easy for the user, but since I think any software exists to serve the user and if I happen to run software that plays well in these circumstances I see little reason why it would be a bad thing to make it easier for me to use the driver I want. Like I said, this situation is not new — it already exists today, even without multiple config files. And this is the kind of power feature that few users will even know it exists, however it will be extremely convenient for some advanced users that already juggle between multiple applets and things seem to work for them. (Naturally this feature will be useless for whoever configured OpenSC to run in exclusive mode.)

From a technical perspective, naturally I'm not part of the team nor I maintain the codebase so I totally understand why somebody would prefer not to add code to the project that they don't see a need for. I tried my best to do a small localized change that would cause no side-effects, but I understand that the use case I'm trying to solve here may be too niche to justify merging this PR.

@dengert
Copy link
Member

dengert commented Oct 6, 2016

I liked your PR, because it leaves it up to the user. I dont see any security issue with it.

@frankmorgner would you reconsider adding this PR? It might also be useful if one driver interferes with another during the match.

@frankmorgner
Copy link
Member

Sure, I'm not too fanatic about this. I guess we already have some potentially harmfull environment variables, so why not introduce some more 😉

@viktorTarasov recently introduced sc_ctx_win32_get_config_value, which should also be used. Additionally, you could check if you can generalize sc_ctx_win32_get_config_value so that you don't have to have an #ifdef _WIN32 before calling it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants