Bug 485829

Summary: Problems using 2048-bit keys with SmartCardManager
Product: [Retired] Dogtag Certificate System Reporter: Jack Magne <jmagne>
Component: ESCAssignee: Jack Magne <jmagne>
Status: CLOSED ERRATA QA Contact: Chandrasekar Kannan <ckannan>
Severity: medium Docs Contact:
Priority: low    
Version: 1.0CC: alee, benl, mharmsen, rrelyea
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: All   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 514299 (view as bug list) Environment:
Last Closed: 2009-07-22 23:32:27 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 443788, 514299    
Attachments:
Description Flags
Coolkey patch for 2048 bit support.
none
Coolkey applet patch for 2048 bit support.
none
A different implementation of "getKeySize()"
none
Changes to have new applet installed on the system. none

Description Jack Magne 2009-02-16 21:43:22 UTC
Description of problem:


ESC depends upon Coolkey and the Coolkey applet perform cryptographic operations. To this point, the Coolkey and the applet has been basically hard coded to support 1024 bit keys. The goal is to provide support such that 2048 bit keys can be managed by ESC and used for cryptographic operations on the target platforms.

Comment 1 Jack Magne 2009-02-16 21:45:25 UTC
Created attachment 332126 [details]
Coolkey patch for 2048 bit support.

Comment 2 Jack Magne 2009-02-16 21:46:06 UTC
Created attachment 332127 [details]
Coolkey applet patch for 2048 bit support.

Comment 3 Jack Magne 2009-02-16 21:46:49 UTC
BobR, can you please review these changes?

Comment 4 Bob Relyea 2009-02-18 00:54:56 UTC
Comment on attachment 332126 [details]
Coolkey patch for 2048 bit support.

r+ for the applet changes.

r- for the coolkey changes.

Minor comments:
change the keysize check from >= to !=.

Major comments:
getKeySize looks way to expensive (have you timed a signature with the new code?).

Every time you reach into the applet, you incurr several hundred milliseconds. Minimal calls are therefore desirable. for 2048 we can't avoid the Read/Write object apdu's, but for 1024 bit keys we should still keep it to a single apdu call.

GetKey size will add between 1 and 3-4 apdu calls as we cycle through the key list. It would be better to get this from some other means. Also, does this code work for CAC cards?

One simple way to get the key size is to look up the Modulus len in the corresponding private key. That value is already filled in for us in the PKCS #11 object.

bob

Comment 5 Bob Relyea 2009-02-18 00:56:20 UTC
To clear up my comments, I meant r+ for the changes to libckyapplet... the applet is a separate patch I'm reviewing now...

bob

Comment 6 Bob Relyea 2009-02-18 00:59:00 UTC
Comment on attachment 332127 [details]
Coolkey applet patch for 2048 bit support.

r+

keep on the non-egate branch for now... We will want a version that works with bigger tokens.

Comment 7 Bob Relyea 2009-02-18 00:59:36 UTC
(It's time's like that I wish we had ifdefs in java:(....

bob

Comment 8 Jack Magne 2009-02-18 06:26:25 UTC
Created attachment 332339 [details]
A different implementation of "getKeySize()"

This seems to work ok. Will there be issues with CAC? It looks like the code creates the modulus attribute when it creates a CAC priv key.

Comment 9 Bob Relyea 2009-02-18 19:27:26 UTC
Comment on attachment 332339 [details]
A different implementation of "getKeySize()"

r+ rrelyea.

This is good enough to check in.

I would like you to make the following changes, however....

You can avoid the lookup by storing the keysize when you look up the keynumber (in objectHandleTokenKeyNum).

We can change this to return a keyNum and a keySize and pass both to the initialize function (which will have to take both).

Second, Only decrement modSize if CKY_Buffer_GetByte(modulus, 0) == 0.

The latter you can make right away. The former, write a bug to remind you, but it doesn't have to happen immediately.

bob

Comment 10 Jack Magne 2009-02-18 21:49:17 UTC
Bug #486218 created for the requested optimization.

Comment 11 Jack Magne 2009-02-19 02:04:52 UTC
Checking in cky_applet.c;
/cvs/dirsec/coolkey/src/libckyapplet/cky_applet.c,v  <--  cky_applet.c
new revision: 1.2; previous revision: 1.1
done
Checking in cky_applet.h;
/cvs/dirsec/coolkey/src/libckyapplet/cky_applet.h,v  <--  cky_applet.h
new revision: 1.2; previous revision: 1.1
done
Checking in cky_factory.c;
/cvs/dirsec/coolkey/src/libckyapplet/cky_factory.c,v  <--  cky_factory.c
new revision: 1.2; previous revision: 1.1
done
Checking in cky_factory.h;
/cvs/dirsec/coolkey/src/libckyapplet/cky_factory.h,v  <--  cky_factory.h
new revision: 1.2; previous revision: 1.1
done
Running syncmail...
Mailing relnotes...
...syncmail done.
Running syncmail...
Mailing cvsdirsec...
...syncmail done.

Checking in slot.cpp;
/cvs/dirsec/coolkey/src/coolkey/slot.cpp,v  <--  slot.cpp
new revision: 1.11; previous revision: 1.10
done
Checking in slot.h;
/cvs/dirsec/coolkey/src/coolkey/slot.h,v  <--  slot.h
new revision: 1.3; previous revision: 1.2
done
Running syncmail...
Mailing relnotes...
...syncmail done.
Running syncmail...
Mailing cvsdirsec...
...syncmail done.

Comment 12 Jack Magne 2009-02-19 02:07:10 UTC
Checking in CardEdge.java;
/cvs/dirsec/coolkey/applet/src/com/redhat/ckey/applet/CardEdge.java,v  <--  CardEdge.java
new revision: 1.4.2.2; previous revision: 1.4.2.1
done
Running syncmail...
Mailing relnotes...
...syncmail done.
Running syncmail...
Mailing cvsdirsec...
...syncmail done.

Comment 13 Jack Magne 2009-02-27 20:01:12 UTC
The new applet has to be checked into tps, to be available for use.

Comment 14 Jack Magne 2009-02-27 20:02:40 UTC
Created attachment 333524 [details]
Changes to have new applet installed on the system.

Comment 15 Jack Magne 2009-02-27 20:12:46 UTC
Spec File change:

===================================================================
--- pki-tps.spec        (revision 262)
+++ pki-tps.spec        (working copy)
@@ -34,7 +34,7 @@
 ## Package Header Definitions
 %define base_name         %{base_prefix}-%{base_component}
 %define base_version      1.0.0
-%define base_release      28
+%define base_release      29
 %define base_group        System Environment/Daemons
 %define base_vendor       Red Hat, Inc.
 %define base_license      LGPLv2 with exceptions
@@ -308,6 +308,8 @@
 ###############################################################################
 
 %changelog
+* Fri Feb 27 2009 Jack Magne <jmagne> 1.0.0-29
+- Bugzilla #485829 - Support for 2048 bit safenet keys.
 * Fri Feb 27 2009 Ade Lee <alee> 1.0.0-28
 - Bugzilla 224835 and 367171: Allow cert nicknames to be edited and sizepanel fixes
 * Thu Feb 26 2009 Ade Lee <alee> 1.0.0-27

Comment 16 Matthew Harmsen 2009-02-27 20:14:16 UTC
attachment (id=333524) +mharmsen

Comment #15 +mharmsen

Comment 17 Jack Magne 2009-02-27 20:18:57 UTC
svn commit -m "Add latest applet to tps, #485829."
Sending        tps/Makefile.am
Sending        tps/Makefile.in
Adding  (bin)  tps/applets/1.4.499dc06c.ijc
Sending        doc/CS.cfg
Transmitting file data .
Committed revision 263.