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

CardOS 5 driver for OpenSC #283

Closed
wants to merge 0 commits into from
Closed

CardOS 5 driver for OpenSC #283

wants to merge 0 commits into from

Conversation

martelletto
Copy link
Contributor

This change builds upon existing CardOS 4 code to implement a CardOS 5
driver for OpenSC. CardOS 4 and 5 differ substantially. The code
currently supports on-card generation and storage of RSA keys up to 2048
bits in modulus.

Support for 4096-bit RSA keys is still defective. I'm currently
investigating why.

The driver was tested using the following commands:

$ pkcs15-init -C --so-pin 12345678 --so-puk 12345678
$ pkcs15-init --store-pin --auth-id 01 --label potato --pin 1234 --puk 1234 --so-pin 12345678
$ pkcs15-init --generate-key rsa/2048 --auth-id 01 --pin 1234 --so-pin 12345678

(or)

$ openssl genrsa 2048 > /tmp/openssl-privkey
$ pkcs15-init --store-private-key /tmp/openssl-privkey --id 45 --auth-id 01 --pin 1234 --so-pin 12345678

And subsequently:

$ ssh-agent ksh
$ ssh-add -s /usr/lib/opensc-pkcs11.so
$ ssh-add -L > /tmp/x && ssh-keygen -e -m PEM -f /tmp/x > /tmp/y && ssh-add -T /tmp/y && echo ok

(where ssh-add -T is an extension to OpenSSH's ssh-add command that generates a random buffer, submits it to the agent to sign and then verifies the signature).

edit: Fixed typo. Added test commands.

@martelletto martelletto mentioned this pull request Sep 4, 2014
@szikora
Copy link
Member

szikora commented Sep 9, 2014

Thanks for this nice work. I applied your diff on the top of the last commit.I found that the share/opensc/cardos5.profile is missing when I installed opensc. I just created one by copying cardos.profile.

On a freshly reformatted CardOS5 (erase, create 1024byte MF and apply the ServicePack), I tried to do a pkcs15-init -C --so-pin 12345678 --so-puk 12345678, I have this output:
Create PKCS15 structure...
Using reader with a card: Dell Dell Smart Card Reader Keyboard 00 00
User PIN required.
Please enter User PIN: (I give the SO-PIN)
Failed to create PKCS #15 meta structure: Security status not satisfied
I can send you the complete debug file of this command.
Thanks

@martelletto
Copy link
Contributor Author

Hi,

Thank you for testing the driver. Would it be possible for you to retrieve the output of OPENSC_DEBUG=10 when this problem happens?

My guess is that the difference in behaviour is due to the fact that we are using different tools to format the card. I'm using a proprietary tool which unfortunately cannot be shared (since it contains the payload of an encrypted APDU legally protected by the card manufacturer). Our tools probably create the MF with different permissions, and so parse_df_acl() needs adjustment.

With the output of OPENSC_DEBUG, I should be able to patch parse_df_acl() accordingly.

@dengert
Copy link
Member

dengert commented Sep 20, 2014

The code modifies iso7816.c. This file should not be modified for individual cards. Can you move the cardos5 code to card-cardos5.c

The ctx.c defines the internal card drivers and the order they are detected. Should cardos5 be placed before cardos?
i.e. is there anyway that the cardos diver could detect a cardos5 card by accident?

@martelletto
Copy link
Contributor Author

Hi,

I've moved the CardOS 5 specific bits back to card-cardos5.c; iso7816.c should now be untouched. Thank you for your suggestion.

As to the order in which the CardOS 4 and CardOS 5 drivers are attached: as long as the cards have different ATRs, we don't run the risk of an accidental mismatch.

@dengert
Copy link
Member

dengert commented Sep 21, 2014

Looks OK to me, but I have no way to test. The code changes should not affect other cards. Others with cardos5 cards should try this if possible.

@martelletto
Copy link
Contributor Author

Generating, uploading and signing with RSA keys up to 2048-bit in modulus seem to work fine. Keys larger than that are still problematic; I have no idea why. :( I've tested the code in this branch with CardOS 5.0 as well as CardOS 4.4 (for possible side effects). Unless memory fails me, @szikora was also able to test the driver with a CardOS 5.0 card.

@martelletto
Copy link
Contributor Author

Good news: with the four commits above, 4096-bit keys work fine.

@dengert
Copy link
Member

dengert commented Sep 23, 2014

I would like to see @szikora build and test as well.

Dies this rely on the #260?

@martelletto
Copy link
Contributor Author

No, this pull request does not rely on #260.

static int
asn1_put_tag(unsigned char tag, const unsigned char *tag_content,
size_t tag_content_len, buf_t *buf)
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Header files are for definitions and global parameters. Don't put the implementation into a header file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will result in code duplication in pkcs15-cardos5.c and card-cardos5.c. Is that preferable to having self-contained code in this header file, which shouldn't be included by anything else? If so, I will happily duplicate the code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment you are not duplicating code, but libopensc.so will contain two implementations of the functions.

You could declare the definition to be non-static and put the implementation into card-cardos5.c.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a very good idea. I will be away from keyboard for about a week; after which I will implement your suggested change. Thank you!

@szikora
Copy link
Member

szikora commented Oct 3, 2014

Hi Pedro,

I've recompiled your last version of the cardos5 branch on a debian7.

  • onepin and normal profile are working
  • pkcs11-tool -tl test was completed successfully for both profile
  • I put a real key (2048bit) signed by a known authority, with the complete certificate chain and I was able to authenticate to a website with the card using Iceweasel! That's a real good news! Just missing the decryption function in case of a S/MIME encrypted mail...

Cheers,

@martelletto
Copy link
Contributor Author

Hi,

Here's a short list of things I plan to work on in the next few weeks:

  • Implement Elliptic Curve Cryptography (ECC) support;
  • Look at internal refactoring suggestions by @frankmorgner;
  • Adopt a dual LGPL/BSD license;

Is there anything that I am missing? Would it be possible for this pull request to be merged as it is, so the items listed above could be implemented separately?

@frankmorgner
Copy link
Member

The licensing stuff feels mandatory for me. Also refactoring of the header file should be done in my opinion.

@martelletto
Copy link
Contributor Author

Hi!

I have committed initial support for elliptic curves. There is even a chance that ECDSA already works with the code as it is; I'm yet to give it a try. The card supports arbitrary curves, which means that you need to tell it which curve you would like to use before a key pair can be generated. Right now only one curve can be configured. The device, however, offers support for multiple coexisting curves.

For instance:

$ openssl ecparam -list_curves
(...)
prime256v1 <- (desired curve)
(...)
$ openssl ecparam -name prime256v1 -param_enc explicit -conv_form uncompressed -outform DER -out /tmp/prime256v1.der
$ openssl ecparam -name prime256v1 -param_enc named_curve -outform DER -out /tmp/prime256v1.oid
$ cardos5-tool /tmp/prime256v1.oid /tmp/prime256v1.der <- (configures the curve on the smartcard)
$ pkcs15-init --generate-key ec/prime256v1 --auth-id 01 --pin 1234 --so-pin 12345678

@martelletto
Copy link
Contributor Author

ECDSA seems to work! I will add some more bits and pieces to cardos5-tool, rebase and squash.

@martelletto
Copy link
Contributor Author

Hi!

As can be seen above, the automated merge build of 663b151 failed due to openssl/cmac.h missing on the build machine. cardos5-tool relies on OpenSSL to do AES CMAC, and therefore includes this file. I have removed cardos5-tool from the build for now; I am fairly confident that those in need of this tool will be able to figure out where to find it and how to build it.

@frankmorgner
Copy link
Member

In configure.ac you may want to use something like
AC_CHECK_HEADERS(openssl/cmac.h, [], [AC_MSG_WARN([openssl/cmac.h not found]) ]) which produces #define HAVE_OPENSSL_CMAC_H 1 in config.h if available. You may use this to handle OpenSSL without CMAC.

@martelletto
Copy link
Contributor Author

Thank you, @frankmorgner! I have implemented your suggestion and rebased the branch.

@Ecordonnier
Copy link
Contributor

I'm trying to test the code (commit aa3e2f) but I'm having the following result:

$ ./cardos5-tool -E -k /home/ecordonnier/startkey_hex -s /home/ecordonnier/seed_hex
Using reader with a card: OMNIKEY CardMan 5321 (OKCM0071703110901202429472470562) 00 00

$ ./cardos5-tool -F -k /home/ecordonnier/startkey_hex -s /home/ecordonnier/seed_hex
Using reader with a card: OMNIKEY CardMan 5321 (OKCM0071703110901202429472470562) 00 00

$ ./pkcs15-init -C --so-pin 12345678 --so-puk 12345678
Using reader with a card: OMNIKEY CardMan 5321 (OKCM0071703110901202429472470562) 00 00
User PIN required.
Please enter User PIN: (I typed 12345678)
Failed to create PKCS #15 meta structure: Security status not satisfied

Do you know what the problem could be?

@martelletto
Copy link
Contributor Author

HI!

Is this a CardOS 5.0? If so, you need to install the service pack (which, unfortunately, is proprietary).

-p.

On 12 Nov 2014, at 16:30, Ecordonnier-theobroma notifications@github.com wrote:

I'm trying to test the code (commit aa3e2f) but I'm having the following result:

$ ./cardos5-tool -E -k /home/ecordonnier/startkey_hex -s /home/ecordonnier/seed_hex
Using reader with a card: OMNIKEY CardMan 5321 (OKCM0071703110901202429472470562) 00 00

$ ./cardos5-tool -F -k /home/ecordonnier/startkey_hex -s /home/ecordonnier/seed_hex
Using reader with a card: OMNIKEY CardMan 5321 (OKCM0071703110901202429472470562) 00 00

$ ./pkcs15-init -C --so-pin 12345678 --so-puk 12345678
Using reader with a card: OMNIKEY CardMan 5321 (OKCM0071703110901202429472470562) 00 00
User PIN required.
Please enter User PIN: (I typed 12345678)
Failed to create PKCS #15 #15 meta structure: Security status not satisfied

Do you know what the problem could be?


Reply to this email directly or view it on GitHub #283 (comment).

@Ecordonnier
Copy link
Contributor

Yes this is a cardOS 5.0. I have the service pack but did not find any documentation stating what it does and if it is required, so I did not know I needed it, thank you very much!

@martelletto
Copy link
Contributor Author

No problem, and sorry for the lack of documentation. That's number one on my TODO list.

@Ecordonnier
Copy link
Contributor

You were right, the generation of the RSA key with pkcs15-init is working after I installed the service pack.

@martelletto
Copy link
Contributor Author

Hi! What's the preferred way to document the CardOS 5 driver? mandoc, troff, entry on a html wiki? Text file? :-)

@frankmorgner
Copy link
Member

@martelletto
Copy link
Contributor Author

Fixed in ef74177; thanks!

@martelletto could you please fix the following compiler warnings (I am just pasting from the CI build):

card-cardos5.c(36) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
card-cardos5.c(42) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
card-cardos5.c(43) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
card-cardos5.c(48) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
card-cardos5.c(53) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
card-cardos5.c(56) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
card-cardos5.c(291) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
card-cardos5.c(434) : warning C4267: '+=' : conversion from 'size_t' to 'unsigned int', possible loss of data
card-cardos5.c(438) : warning C4267: 'return' : conversion from 'size_t' to 'int', possible loss of data
card-cardos5.c(568) : warning C4267: 'return' : conversion from 'size_t' to 'int', possible loss of data
card-cardos5.c(587) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
card-cardos5.c(662) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
card-cardos5.c(732) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
card-cardos5.c(924) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
card-cardos5.c(926) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
card-cardos5.c(1024) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
card-cardos5.c(1026) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
card-cardos5.c(1152) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
card-cardos5.c(1409) : warning C4146: unary minus operator applied to unsigned type, result still unsigned

Reply to this email directly or view it on GitHub:
#283 (comment)

@martelletto
Copy link
Contributor Author

OK. Once the branch is deemed worthy of integration and there are no more objections, please let me know and I will rebase and squash it.

int
aes_cbc(const uint8_t *data, size_t data_len, const uint8_t *syskey,
size_t syskey_len, uint8_t **out, size_t *out_len)
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AES-CBC is already implemented mulitple times in card-epass2003.c, card-piv.c and sc-hsm-tool.c. This should definitely be tied together...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Some day, I mean. I don't think you should fix all the legacy of OpenSC ;-) )

@frankmorgner
Copy link
Member

I see two possible solutions for this problem:

a) put the code (about 200 lines) in a header file;
b) put the code in a cardos5_subr.c file, and have the resulting .o
linked from these three different locations;

I suppose b) is preferable? Once we reach a consensus I will happily
adapt the code accordingly.

Yes, b) looks good.

Frank Morgner

Virtual Smart Card Architecture http://vsmartcard.sourceforge.net
OpenPACE http://openpace.sourceforge.net
IFD Handler for libnfc Devices http://sourceforge.net/projects/ifdnfc

@frankmorgner
Copy link
Member

Thanks for extending sc_asn1_put_tag!

Could someone try it with a different card?

@philipWendland
Copy link
Contributor

Hasn't this been done already in 7e7a44a?
Hence the merge conflict?

@martelletto You might be interested in testing the driver rebased upon a recent version of OpenSC.

@martelletto
Copy link
Contributor Author

You are right. I have rebased the branch.

Hasn't this been done already in 7e7a44a?
Hence the merge conflict?

@martelletto You might be interested in testing the driver rebased upon a recent version of OpenSC.


Reply to this email directly or view it on GitHub:
#283 (comment)

@martelletto
Copy link
Contributor Author

Implemented in d6e257e; thanks!

-p.

Yes, b) looks good.

@frankmorgner
Copy link
Member

I think that the essential remarks have been fixed now, thanks.

Have you thought about using OpenSC's SM infrastructure instead of doing SM "by hand in cardod5-tool?

@frankmorgner
Copy link
Member

Just one more thing ;-) Windows reports the following warning:

card-cardos5.c(321) : warning C4267: 'return' : conversion from 'size_t' to 'int', possible loss of data

@martelletto
Copy link
Contributor Author

Hi Frank,

It seems I forgot to get back to you on that point. Sorry about that.

I saw your earlier suggestion to use OpenSC's SM framework and took a
look at the code. If I understood it correctly, we could move some of
the SM functionality from cardos5-tool to the libopensc driver, allowing
other tools/applications to make use of SM. Is that correct?

I really like the idea. Unfortunately, the seed used to derive SM keys
in CardOS 5 is proprietary. :( I worked around this limitation in
cardos5-tool by receiving the seed as a command line argument.

-p.

I think that the essential remarks have been fixed now, thanks.

Have you thought about using OpenSC's SM infrastructure instead of doing SM "by hand in cardod5-tool?


Reply to this email directly or view it on GitHub:
#283 (comment)

@martelletto
Copy link
Contributor Author

Fixed in 4639a99; thanks!

-p.

Just one more thing ;-) Windows reports the following warning:

card-cardos5.c(321) : warning C4267: 'return' : conversion from 'size_t' to 'int', possible loss of data

Reply to this email directly or view it on GitHub:
#283 (comment)

@frankmorgner
Copy link
Member

Form SM, I think you have two options:

  1. Use cardos5-tool to only do the SM initialization i.e. setting up the keys. And then do the rest in the card driver's SM implementation (I have done the same here https://github.com/frankmorgner/vsmartcard/blob/master/npa/src/npa.c#L325-L371 for an external card driver).
  2. Use the configuration file to supply a key and parse the configuration entry in the card driver. This is done here https://github.com/frankmorgner/OpenSC/blob/master/etc/opensc.conf.in#L353-L370 for example.

Given your current setup I'd go with the first option.

@martelletto
Copy link
Contributor Author

Oh, I see what you mean. I think. :) Unfortunately I don't have my card
with me this weekend, but I will give it a shot next week.

Form SM, I think you have two options:

  1. Use cardos5-tool to only do the SM initialization i.e. setting up the keys. And then do the rest in the card driver's SM implementation (I have done the same here https://github.com/frankmorgner/vsmartcard/blob/master/npa/src/npa.c#L325-L371 for an external card driver).
  2. Use the configuration file to supply a key and parse the configuration entry in the card driver. This is done here https://github.com/frankmorgner/OpenSC/blob/master/etc/opensc.conf.in#L353-L370 for example.

Given your current setup I'd go with the first option.


Reply to this email directly or view it on GitHub:
#283 (comment)

@martelletto martelletto reopened this Apr 27, 2015
@martelletto
Copy link
Contributor Author

Unfortunately, I need to put this pull request on hold while NDA issues are cleared up with Atos. I will do so by pushing a dummy branch to replace the one with the CardOS 5 driver. Hopefully this situation will be resolved soon.

@EBichel
Copy link

EBichel commented Jul 10, 2015

Are there any news about the NDA issues with Atos?

@martelletto
Copy link
Contributor Author

Last I heard there were no news. I no longer work for the company that developed the driver. If you are interested, contact me in private (I suppose that's possible through GitHub?) and I will be happy to get you in touch with people able to give a more accurate answer.

@EBichel
Copy link

EBichel commented Jul 15, 2015

Thanks for your offer! Unfortunately GitHub doesn't offer private messaging anymore. If you could write me an email at eduard.bichel AT gmail DOT com, it is maybe the easiest way to get in touch.

@bigon
Copy link
Contributor

bigon commented Apr 13, 2016

Hi,

Has anything moved on this side?

@martelletto
Copy link
Contributor Author

Hi,

I will ask the company which sponsored the work, but I don't suppose anything has changed, unfortunately.

-p.

@martelletto
Copy link
Contributor Author

Indeed, nothing has changed. I was told by the company in question that there is virtually no interest on their part in resolving this issue. :(

@bigon
Copy link
Contributor

bigon commented Apr 15, 2016

:( Funny thing is that I'm currently a subco for AtoS and that I'm trying to use that cardos card to access their time-sheet system.

I guess I'll have to continue to use my Windows VM...

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.

None yet

10 participants