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

Minidriver: Invalid path cache in newer feitian cards #1159

Closed
VelinPavlov opened this issue Sep 28, 2017 · 32 comments
Closed

Minidriver: Invalid path cache in newer feitian cards #1159

VelinPavlov opened this issue Sep 28, 2017 · 32 comments
Labels

Comments

@VelinPavlov
Copy link

Problem Description

Hello. I have several generations of Feitian-manufactured smart cards, all supported by opensc. Types reported by opensc are 19002, 19003 (older ones, supported by opensc-pre-0.17) and 19004 (recent, supported only by opensc-0.17). I use them for a local PKI, and they all work fine with Mozilla and LibreOffice, on both Linux and Windows platforms.
My problem is that I need to be able to use them with Microsoft office, e.g. with minidriver.dll.

On Windows, certutil reports types 19002 and 19003 as ok (there are problems with trust-chain, OIDs, but this is not related to opensc). 19004 is not working with minidriver.dll - error message is "file not found".

Proposed Resolution

I believe the problems are related to minidriver.dll not being built with support for the newer feitian cards. IMHO, changes are needed in 2 places - src/minidriver/minidriver-feitian.reg and win32/customactions.cpp to add the new ATR/ATRMask values for the newer 19004 == SC_CARD_TYPE_ENTERSAFE_EJAVA_PK_01C, as defined in src/opensc/card-entersafe.c. I can do the changes, but need some advice/approval by more knowledgeable people and a platform to build (nightly builds??). I can use local platforms / cards to test the builds.

Steps to reproduce

standard installation of opensc-0.17 .MSI and testing as described in https://github.com/OpenSC/OpenSC/wiki/Windows-Quick-Start. Testing needs cards. I can do it locally.

Logs

will suppliy if needed

@VelinPavlov
Copy link
Author

Sorry, my report is against .MSI for the released version. I now see that minidriver has been through a lot of changes. I'd expect that nightly-build minidriver would not work with any of my cards, but have not tested it. Is minidriver support for feitian cards being removed?

@VelinPavlov
Copy link
Author

Just for info: the nightly build works with the old cards and not with the newer ones. IMHO this is because feitian-support-code is not removed from customactions.cpp, just the registry-adding-files are removed. As I installed the nightly build over the released-build, the registry entries are from there. I then deleted manually the registry entries - all cards stopped working. I then 'repaired' the installation, and the registry entries did not re-appear again, so none of the cards works, as expected.

@VelinPavlov
Copy link
Author

Using 0.17-released we were able to manually add registry entries and MS-word is now willing to sign using feitian cards, both older and newer. After entry of the correct PIN, MS-word says it cannot sign.

IMHO, the important part of the log is here:

opensc-debug-partial.log

The entire seesion log is here:

opensc-debug.log

I don't understand why

  1. ... ... 2017-10-01 06:44:08.309 Pin code correct. ... ...
  2. ... ... 2017-10-01 06:44:08.539 PIN code verification failed: Data object not found; tries left -1 ... ...

Maybe this is related to
2017-10-01 06:44:08.535 Referenced data not found
2017-10-01 06:44:08.536 Verify rv:-1216
2017-10-01 06:44:08.536 [cardmod] card-entersafe.c:1044:entersafe_pin_cmd: returning with: -1216 (Data object not found)

(for info, I don't know if that is important: the card was initialized like this

echo initializing card $UUID $PIN $PUK
pkcs15-init --reader $RDR --erase-card --profile pkcs15+onepin
echo creating PKCS-15 files
pkcs15-init --reader $RDR
--use-default-transport-key --create-pkcs15
--profile pkcs15+onepin
--pin ${PIN} --puk ${PUK} --label "ДП РВД / $EMP_ID"

echo "used card $sn0 with PIN ${PIN} PUK ${PUK}";
)

@frankmorgner
Copy link
Member

The problem may be related to the internal caching of the current path. Could you check if the PIN verification works with:

diff --git a/src/libopensc/card.c b/src/libopensc/card.c
index 194634bd..91cd98ac 100644
--- a/src/libopensc/card.c
+++ b/src/libopensc/card.c
@@ -423,8 +423,10 @@ int sc_lock(sc_card_t *card)
                        if (r == 0)
                                reader_lock_obtained = 1;
                }
+#if 0
                if (r == 0)
                        card->cache.valid = 1;
+#endif
        }
        if (r == 0)
                card->lock_count++;

@FeitianSmartcardReader
Copy link
Contributor

@VelinPavlov does this issue solved on your side? if not, please let me know, I will tell our engineer focus on, thanks

@VelinPavlov
Copy link
Author

VelinPavlov commented Oct 9, 2017 via email

@VelinPavlov
Copy link
Author

A test build (1437) with the patch suggested is ready.

Testing it will take some time. ETA Fri, 13 oct.

@frankmorgner
Copy link
Member

@FeitianSmartcardReader you can tell your engineer right away. my suggestion is a hack to identify the problem, not as a solution.

@VelinPavlov
Copy link
Author

Hi. The test build works - M$-word is willing to sign, and can sign.

The signed docx is here (signature has been verified on a different system, too)
Test.docx

IMHO this proves @frankmorgner was right - problem is with internal hashing, which the patch just disabled. If this is the case, then @FeitianSmartcardReader has to look closer, and I think this will take some time (I am willing to help as much as I can, of course).

As far as I am concerned, I'd prefer to apply the patch to 0.17-released and do another test build, to isolate what has accumulated in master between release-time and now. I could use this build locally as 'production', because I'd expect it would be as stable as released (and I have no problems with it, except for the windows part) and also be able to sign in M$-office (without cache-ing, but I can live with that). As a matter of fact, I did try to disable cache-ing using opensc.cfg, but obviously my attempts did not have the expected effect.

@FeitianSmartcardReader
Copy link
Contributor

will forward to our engineer to follow up, thanks for the feedback.

@VelinPavlov
Copy link
Author

I now have a repository (https://github.com/VelinPavlov/OpenSC) that is actually stepped-back to released 0.17. I've added the patch suggested by @frankmorgner (to actually block use of cache-ing). I've also added two cosmetic changes with the idea to fix windows-registry-issues for the newer feitian cards (based on 0.17-released; for master, a subset only would be enough).

I did this with the idea to request a pull, use the build system to create the .MSI files, which would be my 'production suite'.

My problem is that I'm not sure such a pull would be totally isolated from the work done by many people for several months. If there is any chance of interfering with that work, I will not do it, but think about some other way.

@frankmorgner, could you please advise?

@VelinPavlov
Copy link
Author

I did a patch for the windows-registry part and will revert back my one-line change in case it doesn't do what I meant; if ok, it might deserve inclusion into master.

The second patch, avoiding path-caching, is only meant for local use of the .MSI's and is not expected to go into master. It's only a temporary solution to a practical problem, until @FeitianSmartcardReader finds a better one.

Finally, my thanks to everybody who helped (or will help:)) me with this issue.

@VelinPavlov
Copy link
Author

Hi. I'm re-opening #1159 as a channel to @FeitianSmartcardReader to share additional info about my tests. My practical issues with the card are resolved, so it can be closed at any time.

  1. 0.17-release works with Mozilla for both FTCOS/PK-01C and EJAVA/PK-01C. MS-word does not.
  2. 0.17.0.1443 works with Mozilla for EJAVA/PK-01C, but not with FTCOS/PK-01C. MS-word can sign. This build contains two patches -
    a. the one that actually prevents using of path-caching, because there seem to be bugs in feitian's implementation.
    b. another one, to fix the windows-registry-on-install. I used the source of card-entersafe.c to get the atr/atr-mask values. These are registered OK, but seem to be wrong, because MS-word can sign, but only after manual modification of the registry entries (in fact changing several 0x00 bytes in the beginning of the mask into 0xff - I don't have the exact values now, but can provide them later).

@VelinPavlov
Copy link
Author

@FeitianSmartcardReader, looking at card-entersafe.c, I don't understand the definition of entersafe_atrs []. It seems that you're splitting atrs into different types and later join them together as actions on the card. If the implementation-on-card is equivalent for these types, then why do we need different types?

@frankmorgner
Copy link
Member

@FeitianSmartcardReader , @VelinPavlov what's the status of this issue, did you resolve it?

@frankmorgner frankmorgner changed the title problems with minidriver and newer feitian cards Minidriver: Invalid path cache in newer feitian cards Nov 6, 2017
@VelinPavlov
Copy link
Author

No response from @FeitianSmartcardReader so far. I did not succeed to resolve it myself.

The build that works best for me is 1456 (basically no-path-caching, but for newer cards only) - it works with mozilla on linux and windows with older and newer cards and MS-word can sign on windows with newer cards only. My local build, on debian, based on the same source, refuses to import PKCS#12 into newer cards (I noticed this by accident, during some testing). This might be a clue, but I don't know enough to use it.

I backported released-0.17 to ubuntu/trusty and it works with mozilla with both older and newer cards.

As I don't really understand the difference in path-caching used by pure PKCS#11 applications and by minidriver, I gave up trying.

@frankmorgner
Copy link
Member

minidriver uses multiple threads to access the card, which may confuse the internal caching. Maybe simply adding a switch for disabling the cache would be an option...

@VelinPavlov
Copy link
Author

Hi, @frankmorgner. Just some thoughts (sorry if this is just noise)

  1. If minidriver is supposed to access the card in multiple threads, then is it minidriver that handles eventual context-switching, or the driver is supposed to do it itself?

  2. For older cards, disabling path-caching has strange effects because build 1443 (preventing any use of caching in card.c (PIN, path, ... et all)) resulted in older-cards not working even with mozilla - e.g. PKCS#11. This might really be a bug, because IMHO driver is provider-of-path-caching, so it should not rely on path-caching at all).

@VelinPavlov
Copy link
Author

and once more (again, sorry if this is more noise than signal)
2. card-acos5.c, card-entersafe.c and card-cardos.c are very similar. I'd expect that problems in one of them might also be present in the others.

@frankmorgner
Copy link
Member

@FeitianSmartcardReader will you fix this issue?

@frankmorgner
Copy link
Member

@VelinPavlov Could you try to compile without any changes, just by passing the additional compiler flag INVALIDATE_CARD_CACHE_IN_UNLOCK (i.e. something like ./configure CFLAGS="-DINVALIDATE_CARD_CACHE_IN_UNLOCK")? I think this should make the problem go away as well.

@VelinPavlov
Copy link
Author

@frankmorgner, you want me to compile current trunk with this flag and test ?
Will do it next days, test.and report back.

@frankmorgner
Copy link
Member

the code has been there for a long time, use master or 0.17.0

@VelinPavlov
Copy link
Author

yes, ok.

frankmorgner added a commit to frankmorgner/OpenSC that referenced this issue Apr 5, 2018
@frankmorgner
Copy link
Member

@VelinPavlov please try #1314, while setting keep_alive for your card in opensc.conf. This should then invalidate the cache, just as the compiler flag did before.

frankmorgner added a commit to frankmorgner/OpenSC that referenced this issue Apr 5, 2018
@VelinPavlov
Copy link
Author

@frankmorgner, I have downloaded the .MSI's from appveyor build 0.17.0.1760 (imo this is pull request #1314). I can try adding keep_alive to opensc.conf.

Testing in my linux environment will need compilation in .debs. I can do that, but it needs time - e.g. @home.

Testing this in windows environment will take some time and has some limitations:
0. for older cards, limited to PKCS#11, mozilla only.

  1. for newer cards, minidriver tests (with MS-office) are possible, but will need time, as this can only be done on a replica of my operational environment, e.g. @office.

With pre-0.18, I believe windows testing has higher priority. What do you think?

@VelinPavlov
Copy link
Author

reporting progress so far:
0. I cloned master into https://github.com/VelinPavlov/OpenSC;

  1. added there a directory debian/ (using stock-debian-opensc-0.17-testing); (painfully) added dh-buildinfo there to make the build-toolset-versions visible at build-time;
  2. merged-in pull request d4f7f42 and verified that @frankmorgner patch (to centralize invalidating the cache into a function call (for some, maybe not all) cards) is applied to the source;
  3. built .debs (idea is that I'd use practically the same sources to test both at-home and at-office.
  4. -- in progress -- testing the .debs at home (e.g. on linux);
  5. -- planned -- install .msi's and test (with help from colleagues) in windows env.

@VelinPavlov
Copy link
Author

rep-01: ad-hoc tests in linux-env work ok. i can erase-card, generate-kypair, store-cert, use-cert (mozilla-https), chang-pin.

-- this is only limited to pkcs#11 use, has no relation to minidriver;
-- will try minidriver in office-env (and do more testing with mozilla there, too)

@frankmorgner
Copy link
Member

great, thanks, that's well enough!

@VelinPavlov
Copy link
Author

@frankmorgner

just for info: with the new .debs, I get this message (related to pkcs15-init and similar)

(process:1754): GLib-GIO-CRITICAL **: 13:27:34.368: g_application_send_notification: assertion '!g_application_get_is_remote (application)' failed

IMO this is related to PIN-dialogue. It's not a problem at all - I only report it for info.

@VelinPavlov
Copy link
Author

@frankmorgner, @FeitianSmartcardReader

rep-02 (from office, under windows)
we did:
0. uninstall opensc-build #1456

  1. install MSIs of build Remove duplicate code #1760, both 32 and 64 bit, vs14; reinitialized registry entries from file
  2. certutil-64 works up to certificate verification;
  3. certutil-32 says 'application has stopped working'
  4. MS-word wants to sign (it sees certificates, trust-chain,...), but cannot sign; it might be that it's 32-bit
  5. stepped back to build#1456 and verified up to and including MS-word signatures - all works (idea was to restore environment AND exclude hardware problems)

@frankmorgner
Copy link
Member

great! thanks for the feedback

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

No branches or pull requests

3 participants