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

Merge several enhancements (ECC, SHA-2, naming the token by CN) #28

Open
wants to merge 90 commits into
base: master
Choose a base branch
from

Conversation

mouse07410
Copy link

This PR adds multiple enhancements and bug fixes, such as:

  • Ability to use SHA-2 hash family
  • Ability to use ECC
    • fully functional for ECDSA
    • completely untested and most likely not ready for ECDH - but there is no application we know of that uses ECDH, so there's nothing to test against
  • Token name is displayed as Common Name from the first valid certificate on the token, rather than "PIV-II"
  • Several bug fixes (details escape me at the moment)

This PR tracks the current OpenSC master with the changes made in June and July 2016 (0.16.0 release, and drivers enhancements)

Frank Morgner and others added 30 commits December 2, 2015 02:27
Original patch by Uri Blumenthal (Mouse) <[email protected]>
Original patch by Uri Blumenthal (Mouse) <[email protected]>
Making sure ECDSA works. (ECDH still seems to have problems, possibly because of applications.)
# The first commit's message is:
Making sure RSA S/MIME fully works.
Making sure ECDSA works. (ECDH still seems to have problems, possibly because of applications.)

# This is the 2nd commit message:

Merging OpenSC#19 from frankmorgner/OpenSC.tokend

# This is the 3rd commit message:

Use dedicated Xcode variable for deploy target

Signed-off-by: Raul Metsma <[email protected]>
:
Making sure RSA S/MIME fully works.
Making sure ECDSA works. (ECDH still seems to have problems, possibly because of applications.)

Merging OpenSC#19 from frankmorgner/OpenSC.tokend

Use dedicated Xcode variable for deploy target

Signed-off-by: Raul Metsma <[email protected]>

Detect the situation where the user has not entered a PIN
at all -and- we have a reader with its own PIN pad connected
(e.g. a SPR532, ParityMedical, eH880, etc).

In that case; defer entry to the reader.

We do not, however, prevent the user from entering the
PIN on the normal keyboard. As we're 'too late' already. If
the user has already done that; we simply pass on the
entered value.
Added ECDH support by adding call to sc_pkcs15_derive(). Testing it.
# The first commit's message is:
# This is a combination of 3 commits.
# The first commit's message is:
Making sure RSA S/MIME fully works.
Making sure ECDSA works. (ECDH still seems to have problems, possibly because of applications.)

# This is the 2nd commit message:

Merging OpenSC#19 from frankmorgner/OpenSC.tokend

# This is the 3rd commit message:

Use dedicated Xcode variable for deploy target

Signed-off-by: Raul Metsma <[email protected]>

# This is the 2nd commit message:

Merging OpenSC#19 from frankmorgner/OpenSC.tokend

# This is the 3rd commit message:

Use dedicated Xcode variable for deploy target

Signed-off-by: Raul Metsma <[email protected]>
mit message:

Merging OpenSC#19 from frankmorgner/OpenSC.tokend

Use dedicated Xcode variable for deploy target

Signed-off-by: Raul Metsma <[email protected]>

Merging OpenSC#19 from frankmorgner/OpenSC.tokend

Use dedicated Xcode variable for deploy target

Signed-off-by: Raul Metsma <[email protected]>

Detect the situation where the user has not entered a PIN
at all -and- we have a reader with its own PIN pad connected
(e.g. a SPR532, ParityMedical, eH880, etc).

In that case; defer entry to the reader.

We do not, however, prevent the user from entering the
PIN on the normal keyboard. As we're 'too late' already. If
the user has already done that; we simply pass on the
entered value.
clang: warning: optimization flag '-finline-functions' is not supported
clang: warning: argument unused during compilation: '-finline-functions'
Install to /Library/Security/tokend instead /System/Library/Security/tokend
http://forums.macrumors.com/threads/os-x-10-11-all-the-little-things.1890519/
/System folder is readonly

Signed-off-by: Raul Metsma <[email protected]>
fixes hard coded the SDK. Xcode now simply uses the newest sdk available
Fixes OpenSC/OpenSC#570
an at least 7 year old bug...
Original patch by Uri Blumenthal (Mouse) <[email protected]>
Original patch by Uri Blumenthal (Mouse) <[email protected]>
Added ECDH support by adding call to sc_pkcs15_derive(). Testing it.
# The first commit's message is:

Got RSA signature and encryption/decryption working correctly.
Finally!!

# This is the 2nd commit message:

# This is a combination of 3 commits.
# The first commit's message is:
# This is a combination of 3 commits.
# The first commit's message is:
Making sure RSA S/MIME fully works.
Making sure ECDSA works. (ECDH still seems to have problems, possibly because of applications.)

# This is the 2nd commit message:

Merging OpenSC#19 from frankmorgner/OpenSC.tokend

# This is the 3rd commit message:

Use dedicated Xcode variable for deploy target

Signed-off-by: Raul Metsma <[email protected]>

# This is the 2nd commit message:

Merging OpenSC#19 from frankmorgner/OpenSC.tokend

# This is the 3rd commit message:

Use dedicated Xcode variable for deploy target

Signed-off-by: Raul Metsma <[email protected]>
Finally!!

Making sure RSA S/MIME fully works.
Making sure ECDSA works. (ECDH still seems to have problems, possibly because of applications.)

Merging OpenSC#19 from frankmorgner/OpenSC.tokend

Use dedicated Xcode variable for deploy target

Signed-off-by: Raul Metsma <[email protected]>

Making sure RSA S/MIME fully works.
Making sure ECDSA works. (ECDH still seems to have problems, possibly because of applications.)

Merging OpenSC#19 from frankmorgner/OpenSC.tokend

Use dedicated Xcode variable for deploy target

Signed-off-by: Raul Metsma <[email protected]>

Detect the situation where the user has not entered a PIN
at all -and- we have a reader with its own PIN pad connected
(e.g. a SPR532, ParityMedical, eH880, etc).

In that case; defer entry to the reader.

We do not, however, prevent the user from entering the
PIN on the normal keyboard. As we're 'too late' already. If
the user has already done that; we simply pass on the
entered value.
@mouse07410
Copy link
Author

I don't know the full result, but I'd be willing to go though the tokend related bits and pieces to get to a "meaningful" PR.

Excellent, thank you!

But I can say for starters that fixing the token name should be shifted to the PIV parts in OpenSC. I won't be touching the PIV part.

If you will add the token name capability to the PIV part of OpenSC, it is fine with me. Once it is there, I might remove it from my fork. But until that (token name in PIV/OpenSC) is fully functional and tested, I am not going to touch it in tokend.

I offered you working code (that does what the main trunk does not). Feel free to do with it what you see fit.

@frankmorgner
Copy link
Member

@martinpaljak did you have any time for compiling the PR?

value = pubkey->modulus_length; /* RSA modulus length in bits */
// FIXME - need to address DSA keys too
}
else if(keyObj->type & SC_PKCS15_TYPE_PUBKEY) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Duplicate else if, see line 81 keyObj->type & SC_PKCS15_TYPE_PUBKEY)

}

// from PUBKEY I can learn whether it is ECC or RSA.
r = sc_pkcs15_get_objects(mScP15Card, SC_PKCS15_TYPE_PUBKEY, objs, 32);
Copy link
Contributor

Choose a reason for hiding this comment

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

Use SC_PKCS15_TYPE_PRKEY, some tokens do not have PKCS11 public key object
and then compare
if (objs[0]->type == SC_PKCS15_TYPE_PRKEY_EC)

sc_debug(mToken.mScCtx, SC_LOG_DEBUG_NORMAL, " Using SHA1, length is 20\n");
sc_debug(mToken.mScCtx, SC_LOG_DEBUG_NORMAL, " Using SHA1, length is 20 bytes\n");
}
else if (signOnly == CSSM_ALGID_SHA256) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Duplicate CSSM_ALGID_SHA256 line 122
Maybe switch statement is more readable?

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