patches for review - ECC key archival / recovery feature implementation - Bug 745278 - [RFE] ECC encryption keys cannot be archived
4:49 p.m.
This is the ECC phase 2 implementation (ECC key archival / recovery
feature) in the JSS and DRM (KRA)
Bug: *Bug 745278* <https://bugzilla.redhat.com/show_bug.cgi?id=745278>
-[RFE] ECC encryption keys cannot be archived
Please review the following patches (see "BEFORE you review" at later
part of this email):
*
https://bugzilla.redhat.com/attachment.cgi?id=570109&action=diff&...
*
https://bugzilla.redhat.com/attachment.cgi?id=570110&action=diff&...
*
https://bugzilla.redhat.com/attachment.cgi?id=570112&action=diff&...
thanks,
Christina
==============
BEFORE you review:
For the ECC plan and design for the different phases, please refer to
http://pki.fedoraproject.org/wiki/ECC_in_Dogtag
Note: the designs beyond phase 2 were more like a brain dump. Although
I said "Do Not Review," you are free to take a peak at what's intended
down the road. I will go back and take a closer look and refine/adjust
the designs when I begin implementation for each new phase.
This patch contains code for the following packages:
JSS, pki-kra, pki-common, pki-util, and pki-kra-ui
What you need to know:
* Problem 1 - nethsm issue:
On the server side, if you turn on FIPS mode, in addition to nethsm, you
need to attach certicom as well to have ECC SSL working on the server
side. This problem has already been reported to Thales last year and
they said they'd look into putting the item on their next release.
Recently through a different contact, we learned there might be a way to
"turn it on" (still waiting for their further instruction)
* Problem 2- Certicom issue:
This is a show-stopper. Initially, on the client side, I used Kai's
special version of Xulrunner/Firefox, attached to Certicom token, so
that the CRMF requests can be generated with key archival option.
However, I encountered (or, re-encountered) an issue with certicom
token. Certicom generates ECC keys with the wrong format (not PKCS7
conforming), which makes ECC key archival impossible on the server side
if you use non-certicom token with DRM (but we expect an HSM in most
product deployment). I have contacted Certicom for this issue, and they
confirmed that they indeed have such issue. We are waiting for their fix.
But then you might ask, "I thought I saw some ECC enrollment
profiles/javascripts being checked in? How were the tests done?" The
tests for those profiles were done against this ECC key
archival/recovery DRM prototype I implemented last year (needs to be
turned on manually in 8.1), where I "cheated" (yeah, that's why it's
called a prototype) by decrypting the private key in the CRMF on DRM,
and then manipulating the byte array to strip off the offending bytes
before archival.
In the real, non-prototype implementation, which is what's in this
patch, for security reasons, private keys are unwrapped directly onto
the token during key archival, so there is no way to manipulate the keys
in memory and bypass the Certicom issue.
A word about Kai's special version of Xulrunner/Firefox. It is not yet
publicly available.
* Problem 3- Firefox with nethsm issue:
Another option was to connect Kai's special version firefox with an HSM
to test my DRM/JSS code. However, for whatever reason, I could not get
SSL going between such Firefox and ECC CA ( I did not try very hard
though, as I have one other option -- writing my own ECC CRMF generation
tool. I might come back to try the nethsm Firefox idea later)
My solution (how I work on this official implementation):
* I hacked up a ECC CRMF tool by taking the CRMFPopClient (existing in
current releases), gutting out the RSA part of the code, and replacing
it with ECC code. I call it CRMFPopClientEC. Two types of ECC key
pairs could be generated: ECDSA or ECDH (That's another benefit of
writing my own tool -- I don't know if you can select which type to
generate in the Javascript... maybe you can, I just don't know). I'm in
no way condoning archival of signing keys!! This is just a test tool.
This tool takes a curve name as option (along with others), generates an
ECC key pair, crafts up an CRMF request with key archival option, and
sends request directly to the specified CA. You will see a "Deferred"
message in the HTML response (see attachment for example)
Once CA agent approves the request, the archival request goes to DRM and
the user private key is archived.
For recovery, DRM agent selects key recovery, etc, and you get your pkcs12.
I did some sanity test with the pkcs12 recovered:
* Import the recovered pkcs12 into a certicom library:
pk12util -d . -h "Certicom FIPS Cert/Key Services" -i userEC.p12
I also tested by retrieving a p12, importing it into a browser, and
adding the user as an agent and the user could act as agent via ssl
client auth to the CA.
Finally, much of the RSA-centric code had been cleared out of the way at
the time when I worked on the DRM ECC prototype, so you don't see much
of that in this round.
About SELinux:
I have a set of rules generated on my system to run in enforcing mode.
I do a writeup.
For QE:
How to set up the servers? The internal wiki is at
https://wiki.idm.lab.bos.redhat.com/export/idmwiki/Working_with_ECC. I
might have given someone a copy of how to set up ECC CA to publish on
fedora.org when I worked on phase1 back a couple years ago. I will put
an updated copy to cover both CA and DRM when I am checking in to dogtag.
How do you test? Well, unless you want to use my CRMFPopClientEC tool
hooked up with a nethsm (like I did), or write your own tool, you can't
really test it until Certicom fixes their issue. (BTW CRMFPopClientEC
can also be changed to work with ceriticom, although you would run into
the same issue I mentioned above)
It is not on my schedule to work on this tool; It is certainly not in
production quality to be released as a regular tool. However, if you
are interested in it, if we get enough request maybe we can think about
doing something with it.
Other test suggestion: I did not try to clone the ECC DRM. It's a good
idea to test it out.
For techpub:
TBD
7:41 p.m.
Took a look at this patch for ECC support.
Comments and questions below.
1.
KeyRecord.java line 273
public MetaInfo getMetaInfo() throws EBaseException {
return mMetaInfo;
}
Why does this throw the EBaseException?
All it does is return a value.
2.
CryptUtil.java line 186
if (sensitive == 1 )
keygen.sensitivePairs(true);
else if (sensitive == 0)
keygen.sensitivePairs(false);
if (extractable == 1 )
keygen.extractablePairs(true);
else if (extractable == 0)
keygen.extractablePairs(false);
keygen.initialize(keysize);
The values extractable and sensitive are sent in as integer?
Should they be a boolean? I see the function calls have -1,-1
for those two final params. How does not setting sensitive and
extractable different from setting them to false?
I can see this being normal.
3.
RecoveryService.java line 363
if (!isRSA) {
CMS.debug("RecoverService: recoverKey: key to recover is RSA");
} else {
CMS.debug("RecoverService: recoverKey: key to recover is not RSA");
}
Would it not be simpler to print out the value of isRSA?
4.
EnrollmentService.java line 451
metaInfo.set("EllipticCurve", oidDescription);
I think in KeyRecord.java you have a static public member variable for
"EllipticCurve"
Probably should use that here for clarity.
5.
ASN1Util.c line 58
SECItem *oid;
Initialize to null?
6.
ASN1Util.c line 78
oidTag = SECOID_FindOIDTag(oid);
if (oidTag == SEC_OID_UNKNOWN) {
JSS_throwMsg(env, INVALID_PARAMETER_EXCEPTION,
"JSS getTagDescriptionByOid: OID UNKNOWN");
goto finish;
}
What if oidTag is null? Perhaps the call can never return this but some well known
constant?
7.
ASN1Uti.java line 126
Question, it looks like the routine takes in an entire public key blob as input.
Would there be some simpler input that could end up giving us the same answer? I do not
know.
8.
Also, the routine throws a InvalidBERException exception or some such.
There are a bunch of calls to methods such as : Arrays.copyOfRange
That method appears to throw the following exceptions:
ArrayIndexOutOfBoundsException -
IllegalArgumentException -
NullPointerException -
Instead of catching only an "Exception" and forcing a
"InvalidBERException", would it make sense to
declare the function to also throw those exceptions?
8.
Should we check for X509PubKeyBytes input parameter for null?
9.
File displayBySerial.template line 103
if ((result.header.keyLength == null) || (result.header.keyLength <= 0)) {
It looks like we use the above check to assume an ECC key. Is this the best way to make
this determination? Is there any info that actually tells us we have an ECC key instead of
this
lack of information?
Actually it looks like this check is used everywhere in this part of the patch when a
decision is needed.
----- Original Message -----
From: "Christina" <cfu(a)redhat.com>
To: pki-devel(a)redhat.com
Sent: Wednesday, March 14, 2012 2:49:37 PM
Subject: [Pki-devel] patches for review - ECC key archival / recovery feature
implementation - Bug 745278 - [RFE] ECC encryption keys cannot be archived
This is the ECC phase 2 implementation (ECC key archival / recovery feature) in the JSS
and DRM (KRA)
Bug: Bug 745278 - [RFE] ECC encryption keys cannot be archived
Please review the following patches (see "BEFORE you review" at later part of
this email):
*
https://bugzilla.redhat.com/attachment.cgi?id=570109&action=diff&...
*
https://bugzilla.redhat.com/attachment.cgi?id=570110&action=diff&...
*
https://bugzilla.redhat.com/attachment.cgi?id=570112&action=diff&...
thanks,
Christina
==============
BEFORE you review:
For the ECC plan and design for the different phases, please refer to
http://pki.fedoraproject.org/wiki/ECC_in_Dogtag
Note: the designs beyond phase 2 were more like a brain dump. Although I said "Do Not
Review," you are free to take a peak at what's intended down the road. I will go
back and take a closer look and refine/adjust the designs when I begin implementation for
each new phase.
This patch contains code for the following packages:
JSS, pki-kra, pki-common, pki-util, and pki-kra-ui
What you need to know:
* Problem 1 - nethsm issue:
On the server side, if you turn on FIPS mode, in addition to nethsm, you need to attach
certicom as well to have ECC SSL working on the server side. This problem has already been
reported to Thales last year and they said they'd look into putting the item on their
next release. Recently through a different contact, we learned there might be a way to
"turn it on" (still waiting for their further instruction)
* Problem 2- Certicom issue:
This is a show-stopper. Initially, on the client side, I used Kai's special version of
Xulrunner/Firefox, attached to Certicom token, so that the CRMF requests can be generated
with key archival option. However, I encountered (or, re-encountered) an issue with
certicom token. Certicom generates ECC keys with the wrong format (not PKCS7 conforming),
which makes ECC key archival impossible on the server side if you use non-certicom token
with DRM (but we expect an HSM in most product deployment). I have contacted Certicom for
this issue, and they confirmed that they indeed have such issue. We are waiting for their
fix.
But then you might ask, "I thought I saw some ECC enrollment profiles/javascripts
being checked in? How were the tests done?" The tests for those profiles were done
against this ECC key archival/recovery DRM prototype I implemented last year (needs to be
turned on manually in 8.1), where I "cheated" (yeah, that's why it's
called a prototype) by decrypting the private key in the CRMF on DRM, and then
manipulating the byte array to strip off the offending bytes before archival.
In the real, non-prototype implementation, which is what's in this patch, for security
reasons, private keys are unwrapped directly onto the token during key archival, so there
is no way to manipulate the keys in memory and bypass the Certicom issue.
A word about Kai's special version of Xulrunner/Firefox. It is not yet publicly
available.
* Problem 3- Firefox with nethsm issue:
Another option was to connect Kai's special version firefox with an HSM to test my
DRM/JSS code. However, for whatever reason, I could not get SSL going between such Firefox
and ECC CA ( I did not try very hard though, as I have one other option -- writing my own
ECC CRMF generation tool. I might come back to try the nethsm Firefox idea later)
My solution (how I work on this official implementation):
* I hacked up a ECC CRMF tool by taking the CRMFPopClient (existing in current releases),
gutting out the RSA part of the code, and replacing it with ECC code. I call it
CRMFPopClientEC. Two types of ECC key pairs could be generated: ECDSA or ECDH (That's
another benefit of writing my own tool -- I don't know if you can select which type to
generate in the Javascript... maybe you can, I just don't know). I'm in no way
condoning archival of signing keys!! This is just a test tool.
This tool takes a curve name as option (along with others), generates an ECC key pair,
crafts up an CRMF request with key archival option, and sends request directly to the
specified CA. You will see a "Deferred" message in the HTML response (see
attachment for example)
Once CA agent approves the request, the archival request goes to DRM and the user private
key is archived.
For recovery, DRM agent selects key recovery, etc, and you get your pkcs12.
I did some sanity test with the pkcs12 recovered:
* Import the recovered pkcs12 into a certicom library:
pk12util -d . -h "Certicom FIPS Cert/Key Services" -i userEC.p12
I also tested by retrieving a p12, importing it into a browser, and adding the user as an
agent and the user could act as agent via ssl client auth to the CA.
Finally, much of the RSA-centric code had been cleared out of the way at the time when I
worked on the DRM ECC prototype, so you don't see much of that in this round.
About SELinux:
I have a set of rules generated on my system to run in enforcing mode. I do a writeup.
For QE:
How to set up the servers? The internal wiki is at
https://wiki.idm.lab.bos.redhat.com/export/idmwiki/Working_with_ECC . I might have given
someone a copy of how to set up ECC CA to publish on fedora.org when I worked on phase1
back a couple years ago. I will put an updated copy to cover both CA and DRM when I am
checking in to dogtag.
How do you test? Well, unless you want to use my CRMFPopClientEC tool hooked up with a
nethsm (like I did), or write your own tool, you can't really test it until Certicom
fixes their issue. (BTW CRMFPopClientEC can also be changed to work with ceriticom,
although you would run into the same issue I mentioned above)
It is not on my schedule to work on this tool; It is certainly not in production quality
to be released as a regular tool. However, if you are interested in it, if we get enough
request maybe we can think about doing something with it.
Other test suggestion: I did not try to clone the ECC DRM. It's a good idea to test it
out.
For techpub:
TBD
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
8 p.m.
On 03/19/2012 05:41 PM, John Magne wrote:
Took a look at this patch for ECC support.
Comments and questions below.
1.
KeyRecord.java line 273
public MetaInfo getMetaInfo() throws EBaseException {
return mMetaInfo;
}
Why does this throw the EBaseException?
All it does is return a value.
yes, it does not need to declare such throw of exception, even though it
is the trend in that file and I was just copying them. Will change.
2.
CryptUtil.java line 186
if (sensitive == 1 )
keygen.sensitivePairs(true);
else if (sensitive == 0)
keygen.sensitivePairs(false);
if (extractable == 1 )
keygen.extractablePairs(true);
else if (extractable == 0)
keygen.extractablePairs(false);
keygen.initialize(keysize);
The values extractable and sensitive are sent in as integer?
Should they be a boolean? I see the function calls have -1,-1
for those two final params. How does not setting sensitive and
extractable different from setting them to false?
I can see this being normal.
As explained in the comment, the definitions are defined in JSS
pkcs11/PK11KeyPairGenerator.java, so I just followed the definition.
For your information, here is what it says in JSS:
private boolean temporaryPairMode = false;
// 1: sensitive
// 0: insensitive
// -1: sensitive if temporaryPairMode is false,
// insensitive if temporaryPairMode is true
// (the default depends on temporaryPairMode for backward
// compatibility)
private int sensitivePairMode = -1;
// 1: extractable
// 0: unextractable
// -1: unspecified (token dependent)
private int extractablePairMode = -1;
3.
RecoveryService.java line 363
if (!isRSA) {
CMS.debug("RecoverService: recoverKey: key to recover is RSA");
} else {
CMS.debug("RecoverService: recoverKey: key to recover is not
RSA");
}
Would it not be simpler to print out the value of isRSA?
It really does not matter. A smarter question would be, why did I even
bother to have "isRSA" parameter for this method? The reason is that
there is an existing recoverKey for RSA, and the function signature does
not take return type into account so it doesn't allow polyinstantiation
of the function unless I add one more parameter to it. So I added this
isRSA param which really serves no purpose other than getting the JAVA
compiler to comply.
That said, I'll change it so that it prints isRSA instead.
4.
EnrollmentService.java line 451
metaInfo.set("EllipticCurve", oidDescription);
I think in KeyRecord.java you have a static public member variable for
"EllipticCurve"
Probably should use that here for clarity.
Will do
5.
ASN1Util.c line 58
SECItem *oid;
Initialize to null?
will do
6.
ASN1Util.c line 78
oidTag = SECOID_FindOIDTag(oid);
if (oidTag == SEC_OID_UNKNOWN) {
JSS_throwMsg(env, INVALID_PARAMETER_EXCEPTION,
"JSS getTagDescriptionByOid: OID UNKNOWN");
goto finish;
}
What if oidTag is null? Perhaps the call can never return this but some well known
constant?
It can't be. It was initialized to SEC_OID_UNKNOWN, and NSS returns
SEC_OID_UNKNOWN to you as well if it can't find it.
I will add comment to explain.
7.
ASN1Uti.java line 126
Question, it looks like the routine takes in an entire public key blob as input.
Would there be some simpler input that could end up giving us the same answer? I do not
know.
There isn't any. You would have to require callers to parse down the
pubkey if we take a smaller byte array for this function.
8.
Also, the routine throws a InvalidBERException exception or some such.
There are a bunch of calls to methods such as : Arrays.copyOfRange
That method appears to throw the following exceptions:
ArrayIndexOutOfBoundsException -
IllegalArgumentException -
NullPointerException -
Instead of catching only an "Exception" and forcing a
"InvalidBERException", would it make sense to
declare the function to also throw those exceptions?
will do
8.
Should we check for X509PubKeyBytes input parameter for null?
will do
9.
File displayBySerial.template line 103
if ((result.header.keyLength == null) || (result.header.keyLength<= 0)) {
It looks like we use the above check to assume an ECC key. Is this the best way to make
this determination? Is there any info that actually tells us we have an ECC key instead
of this
lack of information?
Actually it looks like this check is used everywhere in this part of the patch when a
decision is needed.
I wonder about that myself. I use rec.EllipticCurve != null at certain
places which worked well, but at some specific places, I couldn't for
some reason. That's why I used<=0 check.
I can try again.
----- Original Message -----
From: "Christina"<cfu(a)redhat.com>
To: pki-devel(a)redhat.com
Sent: Wednesday, March 14, 2012 2:49:37 PM
Subject: [Pki-devel] patches for review - ECC key archival / recovery feature
implementation - Bug 745278 - [RFE] ECC encryption keys cannot be archived
This is the ECC phase 2 implementation (ECC key archival / recovery feature) in the JSS
and DRM (KRA)
Bug: Bug 745278 - [RFE] ECC encryption keys cannot be archived
Please review the following patches (see "BEFORE you review" at later part of
this email):
*
https://bugzilla.redhat.com/attachment.cgi?id=570109&action=diff&...
*
https://bugzilla.redhat.com/attachment.cgi?id=570110&action=diff&...
*
https://bugzilla.redhat.com/attachment.cgi?id=570112&action=diff&...
thanks,
Christina
==============
BEFORE you review:
For the ECC plan and design for the different phases, please refer to
http://pki.fedoraproject.org/wiki/ECC_in_Dogtag
Note: the designs beyond phase 2 were more like a brain dump. Although I said "Do
Not Review," you are free to take a peak at what's intended down the road. I will
go back and take a closer look and refine/adjust the designs when I begin implementation
for each new phase.
This patch contains code for the following packages:
JSS, pki-kra, pki-common, pki-util, and pki-kra-ui
What you need to know:
* Problem 1 - nethsm issue:
On the server side, if you turn on FIPS mode, in addition to nethsm, you need to attach
certicom as well to have ECC SSL working on the server side. This problem has already been
reported to Thales last year and they said they'd look into putting the item on their
next release. Recently through a different contact, we learned there might be a way to
"turn it on" (still waiting for their further instruction)
* Problem 2- Certicom issue:
This is a show-stopper. Initially, on the client side, I used Kai's special version
of Xulrunner/Firefox, attached to Certicom token, so that the CRMF requests can be
generated with key archival option. However, I encountered (or, re-encountered) an issue
with certicom token. Certicom generates ECC keys with the wrong format (not PKCS7
conforming), which makes ECC key archival impossible on the server side if you use
non-certicom token with DRM (but we expect an HSM in most product deployment). I have
contacted Certicom for this issue, and they confirmed that they indeed have such issue. We
are waiting for their fix.
But then you might ask, "I thought I saw some ECC enrollment profiles/javascripts
being checked in? How were the tests done?" The tests for those profiles were done
against this ECC key archival/recovery DRM prototype I implemented last year (needs to be
turned on manually in 8.1), where I "cheated" (yeah, that's why it's
called a prototype) by decrypting the private key in the CRMF on DRM, and then
manipulating the byte array to strip off the offending bytes before archival.
In the real, non-prototype implementation, which is what's in this patch, for
security reasons, private keys are unwrapped directly onto the token during key archival,
so there is no way to manipulate the keys in memory and bypass the Certicom issue.
A word about Kai's special version of Xulrunner/Firefox. It is not yet publicly
available.
* Problem 3- Firefox with nethsm issue:
Another option was to connect Kai's special version firefox with an HSM to test my
DRM/JSS code. However, for whatever reason, I could not get SSL going between such Firefox
and ECC CA ( I did not try very hard though, as I have one other option -- writing my own
ECC CRMF generation tool. I might come back to try the nethsm Firefox idea later)
My solution (how I work on this official implementation):
* I hacked up a ECC CRMF tool by taking the CRMFPopClient (existing in current releases),
gutting out the RSA part of the code, and replacing it with ECC code. I call it
CRMFPopClientEC. Two types of ECC key pairs could be generated: ECDSA or ECDH (That's
another benefit of writing my own tool -- I don't know if you can select which type to
generate in the Javascript... maybe you can, I just don't know). I'm in no way
condoning archival of signing keys!! This is just a test tool.
This tool takes a curve name as option (along with others), generates an ECC key pair,
crafts up an CRMF request with key archival option, and sends request directly to the
specified CA. You will see a "Deferred" message in the HTML response (see
attachment for example)
Once CA agent approves the request, the archival request goes to DRM and the user private
key is archived.
For recovery, DRM agent selects key recovery, etc, and you get your pkcs12.
I did some sanity test with the pkcs12 recovered:
* Import the recovered pkcs12 into a certicom library:
pk12util -d . -h "Certicom FIPS Cert/Key Services" -i userEC.p12
I also tested by retrieving a p12, importing it into a browser, and adding the user as an
agent and the user could act as agent via ssl client auth to the CA.
Finally, much of the RSA-centric code had been cleared out of the way at the time when I
worked on the DRM ECC prototype, so you don't see much of that in this round.
About SELinux:
I have a set of rules generated on my system to run in enforcing mode. I do a writeup.
For QE:
How to set up the servers? The internal wiki is at
https://wiki.idm.lab.bos.redhat.com/export/idmwiki/Working_with_ECC . I might have given
someone a copy of how to set up ECC CA to publish on fedora.org when I worked on phase1
back a couple years ago. I will put an updated copy to cover both CA and DRM when I am
checking in to dogtag.
How do you test? Well, unless you want to use my CRMFPopClientEC tool hooked up with a
nethsm (like I did), or write your own tool, you can't really test it until Certicom
fixes their issue. (BTW CRMFPopClientEC can also be changed to work with ceriticom,
although you would run into the same issue I mentioned above)
It is not on my schedule to work on this tool; It is certainly not in production quality
to be released as a regular tool. However, if you are interested in it, if we get enough
request maybe we can think about doing something with it.
Other test suggestion: I did not try to clone the ECC DRM. It's a good idea to test
it out.
For techpub:
TBD
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
5:08 p.m.
JSS part rolled in comments, please review again:
https://bugzilla.redhat.com/attachment.cgi?id=571545&action=diff&...
Note: all other CS components have dependency on the availability of
this patch, so this one needs to be ack'd separately ahead of time.
thanks,
Christina
On 03/19/2012 06:00 PM, Christina Fu wrote:
On 03/19/2012 05:41 PM, John Magne wrote:
>
> Took a look at this patch for ECC support.
>
> Comments and questions below.
>
>
> 1.
>
> KeyRecord.java line 273
>
> public MetaInfo getMetaInfo() throws EBaseException {
>
> return mMetaInfo;
>
> }
>
>
> Why does this throw the EBaseException?
> All it does is return a value.
>
yes, it does not need to declare such throw of exception, even though
it is the trend in that file and I was just copying them. Will change.
> 2.
>
>
> CryptUtil.java line 186
>
> if (sensitive == 1 )
>
> keygen.sensitivePairs(true);
>
> else if (sensitive == 0)
>
> keygen.sensitivePairs(false);
>
> if (extractable == 1 )
>
> keygen.extractablePairs(true);
>
> else if (extractable == 0)
>
> keygen.extractablePairs(false);
>
>
> keygen.initialize(keysize);
>
>
>
>
> The values extractable and sensitive are sent in as integer?
> Should they be a boolean? I see the function calls have -1,-1
> for those two final params. How does not setting sensitive and
> extractable different from setting them to false?
>
> I can see this being normal.
As explained in the comment, the definitions are defined in JSS
pkcs11/PK11KeyPairGenerator.java, so I just followed the definition.
For your information, here is what it says in JSS:
private boolean temporaryPairMode = false;
// 1: sensitive
// 0: insensitive
// -1: sensitive if temporaryPairMode is false,
// insensitive if temporaryPairMode is true
// (the default depends on temporaryPairMode for backward
// compatibility)
private int sensitivePairMode = -1;
// 1: extractable
// 0: unextractable
// -1: unspecified (token dependent)
private int extractablePairMode = -1;
>
> 3.
>
> RecoveryService.java line 363
>
>
> if (!isRSA) {
> CMS.debug("RecoverService: recoverKey: key to recover is
> RSA");
> } else {
>
> CMS.debug("RecoverService: recoverKey: key to recover is
> not RSA");
> }
>
> Would it not be simpler to print out the value of isRSA?
It really does not matter. A smarter question would be, why did I even
bother to have "isRSA" parameter for this method? The reason is that
there is an existing recoverKey for RSA, and the function signature does
not take return type into account so it doesn't allow polyinstantiation
of the function unless I add one more parameter to it. So I added this
isRSA param which really serves no purpose other than getting the JAVA
compiler to comply.
That said, I'll change it so that it prints isRSA instead.
>
> 4.
>
> EnrollmentService.java line 451
>
> metaInfo.set("EllipticCurve", oidDescription);
>
> I think in KeyRecord.java you have a static public member variable
> for "EllipticCurve"
> Probably should use that here for clarity.
Will do
>
> 5.
>
> ASN1Util.c line 58
>
> SECItem *oid;
>
> Initialize to null?
will do
>
> 6.
>
>
> ASN1Util.c line 78
>
> oidTag = SECOID_FindOIDTag(oid);
>
> if (oidTag == SEC_OID_UNKNOWN) {
>
> JSS_throwMsg(env, INVALID_PARAMETER_EXCEPTION,
>
> "JSS getTagDescriptionByOid: OID UNKNOWN");
>
> goto finish;
>
> }
>
> What if oidTag is null? Perhaps the call can never return this but
> some well known constant?
It can't be. It was initialized to SEC_OID_UNKNOWN, and NSS returns
SEC_OID_UNKNOWN to you as well if it can't find it.
I will add comment to explain.
>
> 7.
>
>
> ASN1Uti.java line 126
>
> Question, it looks like the routine takes in an entire public key
> blob as input.
> Would there be some simpler input that could end up giving us the
> same answer? I do not know.
There isn't any. You would have to require callers to parse down the
pubkey if we take a smaller byte array for this function.
>
> 8.
>
> Also, the routine throws a InvalidBERException exception or some such.
>
> There are a bunch of calls to methods such as : Arrays.copyOfRange
>
> That method appears to throw the following exceptions:
>
> ArrayIndexOutOfBoundsException -
> IllegalArgumentException -
> NullPointerException -
>
> Instead of catching only an "Exception" and forcing a
> "InvalidBERException", would it make sense to
> declare the function to also throw those exceptions?
>
will do
> 8.
>
>
> Should we check for X509PubKeyBytes input parameter for null?
will do
>
> 9.
>
>
> File displayBySerial.template line 103
>
> if ((result.header.keyLength == null) || (result.header.keyLength<=
> 0)) {
>
> It looks like we use the above check to assume an ECC key. Is this
> the best way to make
> this determination? Is there any info that actually tells us we have
> an ECC key instead of this
> lack of information?
>
> Actually it looks like this check is used everywhere in this part of
> the patch when a decision is needed.
>
>
I wonder about that myself. I use rec.EllipticCurve != null at certain
places which worked well, but at some specific places, I couldn't for
some reason. That's why I used<=0 check.
I can try again.
>
>
>
>
> ----- Original Message -----
> From: "Christina"<cfu(a)redhat.com>
> To: pki-devel(a)redhat.com
> Sent: Wednesday, March 14, 2012 2:49:37 PM
> Subject: [Pki-devel] patches for review - ECC key archival / recovery
> feature implementation - Bug 745278 - [RFE] ECC encryption keys
> cannot be archived
>
>
>
> This is the ECC phase 2 implementation (ECC key archival / recovery
> feature) in the JSS and DRM (KRA)
>
> Bug: Bug 745278 - [RFE] ECC encryption keys cannot be archived
>
> Please review the following patches (see "BEFORE you review" at later
> part of this email):
>
> *
>
https://bugzilla.redhat.com/attachment.cgi?id=570109&action=diff&...
> *
>
https://bugzilla.redhat.com/attachment.cgi?id=570110&action=diff&...
> *
>
https://bugzilla.redhat.com/attachment.cgi?id=570112&action=diff&...
>
> thanks,
> Christina
> ==============
> BEFORE you review:
>
> For the ECC plan and design for the different phases, please refer to
> http://pki.fedoraproject.org/wiki/ECC_in_Dogtag
>
> Note: the designs beyond phase 2 were more like a brain dump.
> Although I said "Do Not Review," you are free to take a peak at
> what's intended down the road. I will go back and take a closer look
> and refine/adjust the designs when I begin implementation for each
> new phase.
>
> This patch contains code for the following packages:
> JSS, pki-kra, pki-common, pki-util, and pki-kra-ui
>
> What you need to know:
>
> * Problem 1 - nethsm issue:
> On the server side, if you turn on FIPS mode, in addition to nethsm,
> you need to attach certicom as well to have ECC SSL working on the
> server side. This problem has already been reported to Thales last
> year and they said they'd look into putting the item on their next
> release. Recently through a different contact, we learned there might
> be a way to "turn it on" (still waiting for their further instruction)
>
> * Problem 2- Certicom issue:
> This is a show-stopper. Initially, on the client side, I used Kai's
> special version of Xulrunner/Firefox, attached to Certicom token, so
> that the CRMF requests can be generated with key archival option.
> However, I encountered (or, re-encountered) an issue with certicom
> token. Certicom generates ECC keys with the wrong format (not PKCS7
> conforming), which makes ECC key archival impossible on the server
> side if you use non-certicom token with DRM (but we expect an HSM in
> most product deployment). I have contacted Certicom for this issue,
> and they confirmed that they indeed have such issue. We are waiting
> for their fix.
>
> But then you might ask, "I thought I saw some ECC enrollment
> profiles/javascripts being checked in? How were the tests done?" The
> tests for those profiles were done against this ECC key
> archival/recovery DRM prototype I implemented last year (needs to be
> turned on manually in 8.1), where I "cheated" (yeah, that's why
it's
> called a prototype) by decrypting the private key in the CRMF on DRM,
> and then manipulating the byte array to strip off the offending bytes
> before archival.
> In the real, non-prototype implementation, which is what's in this
> patch, for security reasons, private keys are unwrapped directly onto
> the token during key archival, so there is no way to manipulate the
> keys in memory and bypass the Certicom issue.
>
> A word about Kai's special version of Xulrunner/Firefox. It is not
> yet publicly available.
>
> * Problem 3- Firefox with nethsm issue:
> Another option was to connect Kai's special version firefox with an
> HSM to test my DRM/JSS code. However, for whatever reason, I could
> not get SSL going between such Firefox and ECC CA ( I did not try
> very hard though, as I have one other option -- writing my own ECC
> CRMF generation tool. I might come back to try the nethsm Firefox
> idea later)
>
> My solution (how I work on this official implementation):
> * I hacked up a ECC CRMF tool by taking the CRMFPopClient (existing
> in current releases), gutting out the RSA part of the code, and
> replacing it with ECC code. I call it CRMFPopClientEC. Two types of
> ECC key pairs could be generated: ECDSA or ECDH (That's another
> benefit of writing my own tool -- I don't know if you can select
> which type to generate in the Javascript... maybe you can, I just
> don't know). I'm in no way condoning archival of signing keys!! This
> is just a test tool.
> This tool takes a curve name as option (along with others), generates
> an ECC key pair, crafts up an CRMF request with key archival option,
> and sends request directly to the specified CA. You will see a
> "Deferred" message in the HTML response (see attachment for example)
> Once CA agent approves the request, the archival request goes to DRM
> and the user private key is archived.
> For recovery, DRM agent selects key recovery, etc, and you get your
> pkcs12.
>
> I did some sanity test with the pkcs12 recovered:
> * Import the recovered pkcs12 into a certicom library:
> pk12util -d . -h "Certicom FIPS Cert/Key Services" -i userEC.p12
>
> I also tested by retrieving a p12, importing it into a browser, and
> adding the user as an agent and the user could act as agent via ssl
> client auth to the CA.
>
> Finally, much of the RSA-centric code had been cleared out of the way
> at the time when I worked on the DRM ECC prototype, so you don't see
> much of that in this round.
>
> About SELinux:
> I have a set of rules generated on my system to run in enforcing
> mode. I do a writeup.
>
> For QE:
> How to set up the servers? The internal wiki is at
> https://wiki.idm.lab.bos.redhat.com/export/idmwiki/Working_with_ECC .
> I might have given someone a copy of how to set up ECC CA to publish
> on fedora.org when I worked on phase1 back a couple years ago. I will
> put an updated copy to cover both CA and DRM when I am checking in to
> dogtag.
>
> How do you test? Well, unless you want to use my CRMFPopClientEC tool
> hooked up with a nethsm (like I did), or write your own tool, you
> can't really test it until Certicom fixes their issue. (BTW
> CRMFPopClientEC can also be changed to work with ceriticom, although
> you would run into the same issue I mentioned above)
> It is not on my schedule to work on this tool; It is certainly not in
> production quality to be released as a regular tool. However, if you
> are interested in it, if we get enough request maybe we can think
> about doing something with it.
> Other test suggestion: I did not try to clone the ECC DRM. It's a
> good idea to test it out.
>
> For techpub:
> TBD
>
> _______________________________________________
> Pki-devel mailing list
> Pki-devel(a)redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
6:40 p.m.
Based on provided changes:
Attachment #571545: jss-ECC-Phase2KeyArchivalRecovery.patch only:
ACK
----- Original Message -----
From: "Christina Fu" <cfu(a)redhat.com>
To: pki-devel(a)redhat.com
Sent: Tuesday, March 20, 2012 3:08:33 PM
Subject: Re: [Pki-devel] patches for review - ECC key archival / recovery feature
implementation - Bug 745278 - [RFE] ECC encryption keys cannot be archived
JSS part rolled in comments, please review again:
https://bugzilla.redhat.com/attachment.cgi?id=571545&action=diff&...
Note: all other CS components have dependency on the availability of this patch, so this
one needs to be ack'd separately ahead of time.
thanks,
Christina
On 03/19/2012 06:00 PM, Christina Fu wrote:
On 03/19/2012 05:41 PM, John Magne wrote:
Took a look at this patch for ECC support.
Comments and questions below.
1.
KeyRecord.java line 273
public MetaInfo getMetaInfo() throws EBaseException {
return mMetaInfo;
}
Why does this throw the EBaseException?
All it does is return a value.
yes, it does not need to declare such throw of exception, even though it is the trend in
that file and I was just copying them. Will change.
2.
CryptUtil.java line 186
if (sensitive == 1 )
keygen.sensitivePairs(true);
else if (sensitive == 0)
keygen.sensitivePairs(false);
if (extractable == 1 )
keygen.extractablePairs(true);
else if (extractable == 0)
keygen.extractablePairs(false);
keygen.initialize(keysize);
The values extractable and sensitive are sent in as integer?
Should they be a boolean? I see the function calls have -1,-1
for those two final params. How does not setting sensitive and
extractable different from setting them to false?
I can see this being normal.
As explained in the comment, the definitions are defined in JSS
pkcs11/PK11KeyPairGenerator.java, so I just followed the definition.
For your information, here is what it says in JSS:
private boolean temporaryPairMode = false;
// 1: sensitive
// 0: insensitive
// -1: sensitive if temporaryPairMode is false,
// insensitive if temporaryPairMode is true
// (the default depends on temporaryPairMode for backward
// compatibility)
private int sensitivePairMode = -1;
// 1: extractable
// 0: unextractable
// -1: unspecified (token dependent)
private int extractablePairMode = -1;
3.
RecoveryService.java line 363
if (!isRSA) {
CMS.debug("RecoverService: recoverKey: key to recover is RSA");
} else {
CMS.debug("RecoverService: recoverKey: key to recover is not RSA");
}
Would it not be simpler to print out the value of isRSA?
It really does not matter. A smarter question would be, why did I even
bother to have "isRSA" parameter for this method? The reason is that
there is an existing recoverKey for RSA, and the function signature does
not take return type into account so it doesn't allow polyinstantiation
of the function unless I add one more parameter to it. So I added this
isRSA param which really serves no purpose other than getting the JAVA compiler to comply.
That said, I'll change it so that it prints isRSA instead.
4.
EnrollmentService.java line 451
metaInfo.set("EllipticCurve", oidDescription);
I think in KeyRecord.java you have a static public member variable for
"EllipticCurve"
Probably should use that here for clarity.
Will do
5.
ASN1Util.c line 58
SECItem *oid;
Initialize to null?
will do
6.
ASN1Util.c line 78
oidTag = SECOID_FindOIDTag(oid);
if (oidTag == SEC_OID_UNKNOWN) {
JSS_throwMsg(env, INVALID_PARAMETER_EXCEPTION,
"JSS getTagDescriptionByOid: OID UNKNOWN");
goto finish;
}
What if oidTag is null? Perhaps the call can never return this but some well known
constant?
It can't be. It was initialized to SEC_OID_UNKNOWN, and NSS returns
SEC_OID_UNKNOWN to you as well if it can't find it.
I will add comment to explain.
7.
ASN1Uti.java line 126
Question, it looks like the routine takes in an entire public key blob as input.
Would there be some simpler input that could end up giving us the same answer? I do not
know.
There isn't any. You would have to require callers to parse down the
pubkey if we take a smaller byte array for this function.
8.
Also, the routine throws a InvalidBERException exception or some such.
There are a bunch of calls to methods such as : Arrays.copyOfRange
That method appears to throw the following exceptions:
ArrayIndexOutOfBoundsException -
IllegalArgumentException -
NullPointerException -
Instead of catching only an "Exception" and forcing a
"InvalidBERException", would it make sense to
declare the function to also throw those exceptions?
will do
8.
Should we check for X509PubKeyBytes input parameter for null?
will do
9.
File displayBySerial.template line 103
if ((result.header.keyLength == null) || (result.header.keyLength<= 0)) {
It looks like we use the above check to assume an ECC key. Is this the best way to make
this determination? Is there any info that actually tells us we have an ECC key instead of
this
lack of information?
Actually it looks like this check is used everywhere in this part of the patch when a
decision is needed.
I wonder about that myself. I use rec.EllipticCurve != null at certain
places which worked well, but at some specific places, I couldn't for
some reason. That's why I used<=0 check.
I can try again.
----- Original Message -----
From: "Christina" <cfu(a)redhat.com>
To: pki-devel(a)redhat.com
Sent: Wednesday, March 14, 2012 2:49:37 PM
Subject: [Pki-devel] patches for review - ECC key archival / recovery feature
implementation - Bug 745278 - [RFE] ECC encryption keys cannot be archived
This is the ECC phase 2 implementation (ECC key archival / recovery feature) in the JSS
and DRM (KRA)
Bug: Bug 745278 - [RFE] ECC encryption keys cannot be archived
Please review the following patches (see "BEFORE you review" at later part of
this email):
*
https://bugzilla.redhat.com/attachment.cgi?id=570109&action=diff&...
*
https://bugzilla.redhat.com/attachment.cgi?id=570110&action=diff&...
*
https://bugzilla.redhat.com/attachment.cgi?id=570112&action=diff&...
thanks,
Christina
==============
BEFORE you review:
For the ECC plan and design for the different phases, please refer to
http://pki.fedoraproject.org/wiki/ECC_in_Dogtag
Note: the designs beyond phase 2 were more like a brain dump. Although I said "Do Not
Review," you are free to take a peak at what's intended down the road. I will go
back and take a closer look and refine/adjust the designs when I begin implementation for
each new phase.
This patch contains code for the following packages:
JSS, pki-kra, pki-common, pki-util, and pki-kra-ui
What you need to know:
* Problem 1 - nethsm issue:
On the server side, if you turn on FIPS mode, in addition to nethsm, you need to attach
certicom as well to have ECC SSL working on the server side. This problem has already been
reported to Thales last year and they said they'd look into putting the item on their
next release. Recently through a different contact, we learned there might be a way to
"turn it on" (still waiting for their further instruction)
* Problem 2- Certicom issue:
This is a show-stopper. Initially, on the client side, I used Kai's special version of
Xulrunner/Firefox, attached to Certicom token, so that the CRMF requests can be generated
with key archival option. However, I encountered (or, re-encountered) an issue with
certicom token. Certicom generates ECC keys with the wrong format (not PKCS7 conforming),
which makes ECC key archival impossible on the server side if you use non-certicom token
with DRM (but we expect an HSM in most product deployment). I have contacted Certicom for
this issue, and they confirmed that they indeed have such issue. We are waiting for their
fix.
But then you might ask, "I thought I saw some ECC enrollment profiles/javascripts
being checked in? How were the tests done?" The tests for those profiles were done
against this ECC key archival/recovery DRM prototype I implemented last year (needs to be
turned on manually in 8.1), where I "cheated" (yeah, that's why it's
called a prototype) by decrypting the private key in the CRMF on DRM, and then
manipulating the byte array to strip off the offending bytes before archival.
In the real, non-prototype implementation, which is what's in this patch, for security
reasons, private keys are unwrapped directly onto the token during key archival, so there
is no way to manipulate the keys in memory and bypass the Certicom issue.
A word about Kai's special version of Xulrunner/Firefox. It is not yet publicly
available.
* Problem 3- Firefox with nethsm issue:
Another option was to connect Kai's special version firefox with an HSM to test my
DRM/JSS code. However, for whatever reason, I could not get SSL going between such Firefox
and ECC CA ( I did not try very hard though, as I have one other option -- writing my own
ECC CRMF generation tool. I might come back to try the nethsm Firefox idea later)
My solution (how I work on this official implementation):
* I hacked up a ECC CRMF tool by taking the CRMFPopClient (existing in current releases),
gutting out the RSA part of the code, and replacing it with ECC code. I call it
CRMFPopClientEC. Two types of ECC key pairs could be generated: ECDSA or ECDH (That's
another benefit of writing my own tool -- I don't know if you can select which type to
generate in the Javascript... maybe you can, I just don't know). I'm in no way
condoning archival of signing keys!! This is just a test tool.
This tool takes a curve name as option (along with others), generates an ECC key pair,
crafts up an CRMF request with key archival option, and sends request directly to the
specified CA. You will see a "Deferred" message in the HTML response (see
attachment for example)
Once CA agent approves the request, the archival request goes to DRM and the user private
key is archived.
For recovery, DRM agent selects key recovery, etc, and you get your pkcs12.
I did some sanity test with the pkcs12 recovered:
* Import the recovered pkcs12 into a certicom library:
pk12util -d . -h "Certicom FIPS Cert/Key Services" -i userEC.p12
I also tested by retrieving a p12, importing it into a browser, and adding the user as an
agent and the user could act as agent via ssl client auth to the CA.
Finally, much of the RSA-centric code had been cleared out of the way at the time when I
worked on the DRM ECC prototype, so you don't see much of that in this round.
About SELinux:
I have a set of rules generated on my system to run in enforcing mode. I do a writeup.
For QE:
How to set up the servers? The internal wiki is at
https://wiki.idm.lab.bos.redhat.com/export/idmwiki/Working_with_ECC . I might have given
someone a copy of how to set up ECC CA to publish on fedora.org when I worked on phase1
back a couple years ago. I will put an updated copy to cover both CA and DRM when I am
checking in to dogtag.
How do you test? Well, unless you want to use my CRMFPopClientEC tool hooked up with a
nethsm (like I did), or write your own tool, you can't really test it until Certicom
fixes their issue. (BTW CRMFPopClientEC can also be changed to work with ceriticom,
although you would run into the same issue I mentioned above)
It is not on my schedule to work on this tool; It is certainly not in production quality
to be released as a regular tool. However, if you are interested in it, if we get enough
request maybe we can think about doing something with it.
Other test suggestion: I did not try to clone the ECC DRM. It's a good idea to test it
out.
For techpub:
TBD
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
_______________________________________________
Pki-devel mailing list Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
12:29 p.m.
Here are the CS patches with comments rolled in. Please review:
https://bugzilla.redhat.com/attachment.cgi?id=571781&action=diff&...
https://bugzilla.redhat.com/attachment.cgi?id=571782&action=diff&...
Please note that they will not be checked in until the new JSS build is
ready.
Also, a demo was conducted.
thanks,
Christina
On 03/20/2012 04:40 PM, John Magne wrote:
Based on provided changes:
Attachment #571545: jss-ECC-Phase2KeyArchivalRecovery.patch only:
ACK
----- Original Message -----
From: "Christina Fu"<cfu(a)redhat.com>
To: pki-devel(a)redhat.com
Sent: Tuesday, March 20, 2012 3:08:33 PM
Subject: Re: [Pki-devel] patches for review - ECC key archival / recovery feature
implementation - Bug 745278 - [RFE] ECC encryption keys cannot be archived
JSS part rolled in comments, please review again:
https://bugzilla.redhat.com/attachment.cgi?id=571545&action=diff&...
Note: all other CS components have dependency on the availability of this patch, so this
one needs to be ack'd separately ahead of time.
thanks,
Christina
On 03/19/2012 06:00 PM, Christina Fu wrote:
On 03/19/2012 05:41 PM, John Magne wrote:
Took a look at this patch for ECC support.
Comments and questions below.
1.
KeyRecord.java line 273
public MetaInfo getMetaInfo() throws EBaseException {
return mMetaInfo;
}
Why does this throw the EBaseException?
All it does is return a value.
yes, it does not need to declare such throw of exception, even though it is the trend in
that file and I was just copying them. Will change.
2.
CryptUtil.java line 186
if (sensitive == 1 )
keygen.sensitivePairs(true);
else if (sensitive == 0)
keygen.sensitivePairs(false);
if (extractable == 1 )
keygen.extractablePairs(true);
else if (extractable == 0)
keygen.extractablePairs(false);
keygen.initialize(keysize);
The values extractable and sensitive are sent in as integer?
Should they be a boolean? I see the function calls have -1,-1
for those two final params. How does not setting sensitive and
extractable different from setting them to false?
I can see this being normal.
As explained in the comment, the definitions are defined in JSS
pkcs11/PK11KeyPairGenerator.java, so I just followed the definition.
For your information, here is what it says in JSS:
private boolean temporaryPairMode = false;
// 1: sensitive
// 0: insensitive
// -1: sensitive if temporaryPairMode is false,
// insensitive if temporaryPairMode is true
// (the default depends on temporaryPairMode for backward
// compatibility)
private int sensitivePairMode = -1;
// 1: extractable
// 0: unextractable
// -1: unspecified (token dependent)
private int extractablePairMode = -1;
3.
RecoveryService.java line 363
if (!isRSA) {
CMS.debug("RecoverService: recoverKey: key to recover is RSA");
} else {
CMS.debug("RecoverService: recoverKey: key to recover is not RSA");
}
Would it not be simpler to print out the value of isRSA?
It really does not matter. A smarter question would be, why did I even
bother to have "isRSA" parameter for this method? The reason is that
there is an existing recoverKey for RSA, and the function signature does
not take return type into account so it doesn't allow polyinstantiation
of the function unless I add one more parameter to it. So I added this
isRSA param which really serves no purpose other than getting the JAVA compiler to
comply.
That said, I'll change it so that it prints isRSA instead.
4.
EnrollmentService.java line 451
metaInfo.set("EllipticCurve", oidDescription);
I think in KeyRecord.java you have a static public member variable for
"EllipticCurve"
Probably should use that here for clarity.
Will do
5.
ASN1Util.c line 58
SECItem *oid;
Initialize to null?
will do
6.
ASN1Util.c line 78
oidTag = SECOID_FindOIDTag(oid);
if (oidTag == SEC_OID_UNKNOWN) {
JSS_throwMsg(env, INVALID_PARAMETER_EXCEPTION,
"JSS getTagDescriptionByOid: OID UNKNOWN");
goto finish;
}
What if oidTag is null? Perhaps the call can never return this but some well known
constant?
It can't be. It was initialized to SEC_OID_UNKNOWN, and NSS returns
SEC_OID_UNKNOWN to you as well if it can't find it.
I will add comment to explain.
7.
ASN1Uti.java line 126
Question, it looks like the routine takes in an entire public key blob as input.
Would there be some simpler input that could end up giving us the same answer? I do not
know.
There isn't any. You would have to require callers to parse down the
pubkey if we take a smaller byte array for this function.
8.
Also, the routine throws a InvalidBERException exception or some such.
There are a bunch of calls to methods such as : Arrays.copyOfRange
That method appears to throw the following exceptions:
ArrayIndexOutOfBoundsException -
IllegalArgumentException -
NullPointerException -
Instead of catching only an "Exception" and forcing a
"InvalidBERException", would it make sense to
declare the function to also throw those exceptions?
will do
8.
Should we check for X509PubKeyBytes input parameter for null?
will do
9.
File displayBySerial.template line 103
if ((result.header.keyLength == null) || (result.header.keyLength<= 0)) {
It looks like we use the above check to assume an ECC key. Is this the best way to make
this determination? Is there any info that actually tells us we have an ECC key instead
of this
lack of information?
Actually it looks like this check is used everywhere in this part of the patch when a
decision is needed.
I wonder about that myself. I use rec.EllipticCurve != null at certain
places which worked well, but at some specific places, I couldn't for
some reason. That's why I used<=0 check.
I can try again.
----- Original Message -----
From: "Christina"<cfu(a)redhat.com>
To: pki-devel(a)redhat.com
Sent: Wednesday, March 14, 2012 2:49:37 PM
Subject: [Pki-devel] patches for review - ECC key archival / recovery feature
implementation - Bug 745278 - [RFE] ECC encryption keys cannot be archived
This is the ECC phase 2 implementation (ECC key archival / recovery feature) in the JSS
and DRM (KRA)
Bug: Bug 745278 - [RFE] ECC encryption keys cannot be archived
Please review the following patches (see "BEFORE you review" at later part of
this email):
*
https://bugzilla.redhat.com/attachment.cgi?id=570109&action=diff&...
*
https://bugzilla.redhat.com/attachment.cgi?id=570110&action=diff&...
*
https://bugzilla.redhat.com/attachment.cgi?id=570112&action=diff&...
thanks,
Christina
==============
BEFORE you review:
For the ECC plan and design for the different phases, please refer to
http://pki.fedoraproject.org/wiki/ECC_in_Dogtag
Note: the designs beyond phase 2 were more like a brain dump. Although I said "Do
Not Review," you are free to take a peak at what's intended down the road. I will
go back and take a closer look and refine/adjust the designs when I begin implementation
for each new phase.
This patch contains code for the following packages:
JSS, pki-kra, pki-common, pki-util, and pki-kra-ui
What you need to know:
* Problem 1 - nethsm issue:
On the server side, if you turn on FIPS mode, in addition to nethsm, you need to attach
certicom as well to have ECC SSL working on the server side. This problem has already been
reported to Thales last year and they said they'd look into putting the item on their
next release. Recently through a different contact, we learned there might be a way to
"turn it on" (still waiting for their further instruction)
* Problem 2- Certicom issue:
This is a show-stopper. Initially, on the client side, I used Kai's special version
of Xulrunner/Firefox, attached to Certicom token, so that the CRMF requests can be
generated with key archival option. However, I encountered (or, re-encountered) an issue
with certicom token. Certicom generates ECC keys with the wrong format (not PKCS7
conforming), which makes ECC key archival impossible on the server side if you use
non-certicom token with DRM (but we expect an HSM in most product deployment). I have
contacted Certicom for this issue, and they confirmed that they indeed have such issue. We
are waiting for their fix.
But then you might ask, "I thought I saw some ECC enrollment profiles/javascripts
being checked in? How were the tests done?" The tests for those profiles were done
against this ECC key archival/recovery DRM prototype I implemented last year (needs to be
turned on manually in 8.1), where I "cheated" (yeah, that's why it's
called a prototype) by decrypting the private key in the CRMF on DRM, and then
manipulating the byte array to strip off the offending bytes before archival.
In the real, non-prototype implementation, which is what's in this patch, for
security reasons, private keys are unwrapped directly onto the token during key archival,
so there is no way to manipulate the keys in memory and bypass the Certicom issue.
A word about Kai's special version of Xulrunner/Firefox. It is not yet publicly
available.
* Problem 3- Firefox with nethsm issue:
Another option was to connect Kai's special version firefox with an HSM to test my
DRM/JSS code. However, for whatever reason, I could not get SSL going between such Firefox
and ECC CA ( I did not try very hard though, as I have one other option -- writing my own
ECC CRMF generation tool. I might come back to try the nethsm Firefox idea later)
My solution (how I work on this official implementation):
* I hacked up a ECC CRMF tool by taking the CRMFPopClient (existing in current releases),
gutting out the RSA part of the code, and replacing it with ECC code. I call it
CRMFPopClientEC. Two types of ECC key pairs could be generated: ECDSA or ECDH (That's
another benefit of writing my own tool -- I don't know if you can select which type to
generate in the Javascript... maybe you can, I just don't know). I'm in no way
condoning archival of signing keys!! This is just a test tool.
This tool takes a curve name as option (along with others), generates an ECC key pair,
crafts up an CRMF request with key archival option, and sends request directly to the
specified CA. You will see a "Deferred" message in the HTML response (see
attachment for example)
Once CA agent approves the request, the archival request goes to DRM and the user private
key is archived.
For recovery, DRM agent selects key recovery, etc, and you get your pkcs12.
I did some sanity test with the pkcs12 recovered:
* Import the recovered pkcs12 into a certicom library:
pk12util -d . -h "Certicom FIPS Cert/Key Services" -i userEC.p12
I also tested by retrieving a p12, importing it into a browser, and adding the user as an
agent and the user could act as agent via ssl client auth to the CA.
Finally, much of the RSA-centric code had been cleared out of the way at the time when I
worked on the DRM ECC prototype, so you don't see much of that in this round.
About SELinux:
I have a set of rules generated on my system to run in enforcing mode. I do a writeup.
For QE:
How to set up the servers? The internal wiki is at
https://wiki.idm.lab.bos.redhat.com/export/idmwiki/Working_with_ECC . I might have given
someone a copy of how to set up ECC CA to publish on fedora.org when I worked on phase1
back a couple years ago. I will put an updated copy to cover both CA and DRM when I am
checking in to dogtag.
How do you test? Well, unless you want to use my CRMFPopClientEC tool hooked up with a
nethsm (like I did), or write your own tool, you can't really test it until Certicom
fixes their issue. (BTW CRMFPopClientEC can also be changed to work with ceriticom,
although you would run into the same issue I mentioned above)
It is not on my schedule to work on this tool; It is certainly not in production quality
to be released as a regular tool. However, if you are interested in it, if we get enough
request maybe we can think about doing something with it.
Other test suggestion: I did not try to clone the ECC DRM. It's a good idea to test
it out.
For techpub:
TBD
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
_______________________________________________
Pki-devel mailing list Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
12:39 p.m.
Based on changes completed:
ACK
----- Original Message -----
From: "Christina Fu" <cfu(a)redhat.com>
To: "John Magne" <jmagne(a)redhat.com>
Cc: pki-devel(a)redhat.com
Sent: Wednesday, March 21, 2012 10:29:06 AM
Subject: Re: [Pki-devel] patches for review - ECC key archival / recovery feature
implementation - Bug 745278 - [RFE] ECC encryption keys cannot be archived
Here are the CS patches with comments rolled in. Please review:
https://bugzilla.redhat.com/attachment.cgi?id=571781&action=diff&...
https://bugzilla.redhat.com/attachment.cgi?id=571782&action=diff&...
Please note that they will not be checked in until the new JSS build is
ready.
Also, a demo was conducted.
thanks,
Christina
On 03/20/2012 04:40 PM, John Magne wrote:
Based on provided changes:
Attachment #571545: jss-ECC-Phase2KeyArchivalRecovery.patch only:
ACK
----- Original Message -----
From: "Christina Fu"<cfu(a)redhat.com>
To: pki-devel(a)redhat.com
Sent: Tuesday, March 20, 2012 3:08:33 PM
Subject: Re: [Pki-devel] patches for review - ECC key archival / recovery feature
implementation - Bug 745278 - [RFE] ECC encryption keys cannot be archived
JSS part rolled in comments, please review again:
https://bugzilla.redhat.com/attachment.cgi?id=571545&action=diff&...
Note: all other CS components have dependency on the availability of this patch, so this
one needs to be ack'd separately ahead of time.
thanks,
Christina
On 03/19/2012 06:00 PM, Christina Fu wrote:
On 03/19/2012 05:41 PM, John Magne wrote:
Took a look at this patch for ECC support.
Comments and questions below.
1.
KeyRecord.java line 273
public MetaInfo getMetaInfo() throws EBaseException {
return mMetaInfo;
}
Why does this throw the EBaseException?
All it does is return a value.
yes, it does not need to declare such throw of exception, even though it is the trend in
that file and I was just copying them. Will change.
2.
CryptUtil.java line 186
if (sensitive == 1 )
keygen.sensitivePairs(true);
else if (sensitive == 0)
keygen.sensitivePairs(false);
if (extractable == 1 )
keygen.extractablePairs(true);
else if (extractable == 0)
keygen.extractablePairs(false);
keygen.initialize(keysize);
The values extractable and sensitive are sent in as integer?
Should they be a boolean? I see the function calls have -1,-1
for those two final params. How does not setting sensitive and
extractable different from setting them to false?
I can see this being normal.
As explained in the comment, the definitions are defined in JSS
pkcs11/PK11KeyPairGenerator.java, so I just followed the definition.
For your information, here is what it says in JSS:
private boolean temporaryPairMode = false;
// 1: sensitive
// 0: insensitive
// -1: sensitive if temporaryPairMode is false,
// insensitive if temporaryPairMode is true
// (the default depends on temporaryPairMode for backward
// compatibility)
private int sensitivePairMode = -1;
// 1: extractable
// 0: unextractable
// -1: unspecified (token dependent)
private int extractablePairMode = -1;
3.
RecoveryService.java line 363
if (!isRSA) {
CMS.debug("RecoverService: recoverKey: key to recover is RSA");
} else {
CMS.debug("RecoverService: recoverKey: key to recover is not RSA");
}
Would it not be simpler to print out the value of isRSA?
It really does not matter. A smarter question would be, why did I even
bother to have "isRSA" parameter for this method? The reason is that
there is an existing recoverKey for RSA, and the function signature does
not take return type into account so it doesn't allow polyinstantiation
of the function unless I add one more parameter to it. So I added this
isRSA param which really serves no purpose other than getting the JAVA compiler to
comply.
That said, I'll change it so that it prints isRSA instead.
4.
EnrollmentService.java line 451
metaInfo.set("EllipticCurve", oidDescription);
I think in KeyRecord.java you have a static public member variable for
"EllipticCurve"
Probably should use that here for clarity.
Will do
5.
ASN1Util.c line 58
SECItem *oid;
Initialize to null?
will do
6.
ASN1Util.c line 78
oidTag = SECOID_FindOIDTag(oid);
if (oidTag == SEC_OID_UNKNOWN) {
JSS_throwMsg(env, INVALID_PARAMETER_EXCEPTION,
"JSS getTagDescriptionByOid: OID UNKNOWN");
goto finish;
}
What if oidTag is null? Perhaps the call can never return this but some well known
constant?
It can't be. It was initialized to SEC_OID_UNKNOWN, and NSS returns
SEC_OID_UNKNOWN to you as well if it can't find it.
I will add comment to explain.
7.
ASN1Uti.java line 126
Question, it looks like the routine takes in an entire public key blob as input.
Would there be some simpler input that could end up giving us the same answer? I do not
know.
There isn't any. You would have to require callers to parse down the
pubkey if we take a smaller byte array for this function.
8.
Also, the routine throws a InvalidBERException exception or some such.
There are a bunch of calls to methods such as : Arrays.copyOfRange
That method appears to throw the following exceptions:
ArrayIndexOutOfBoundsException -
IllegalArgumentException -
NullPointerException -
Instead of catching only an "Exception" and forcing a
"InvalidBERException", would it make sense to
declare the function to also throw those exceptions?
will do
8.
Should we check for X509PubKeyBytes input parameter for null?
will do
9.
File displayBySerial.template line 103
if ((result.header.keyLength == null) || (result.header.keyLength<= 0)) {
It looks like we use the above check to assume an ECC key. Is this the best way to make
this determination? Is there any info that actually tells us we have an ECC key instead
of this
lack of information?
Actually it looks like this check is used everywhere in this part of the patch when a
decision is needed.
I wonder about that myself. I use rec.EllipticCurve != null at certain
places which worked well, but at some specific places, I couldn't for
some reason. That's why I used<=0 check.
I can try again.
----- Original Message -----
From: "Christina"<cfu(a)redhat.com>
To: pki-devel(a)redhat.com
Sent: Wednesday, March 14, 2012 2:49:37 PM
Subject: [Pki-devel] patches for review - ECC key archival / recovery feature
implementation - Bug 745278 - [RFE] ECC encryption keys cannot be archived
This is the ECC phase 2 implementation (ECC key archival / recovery feature) in the JSS
and DRM (KRA)
Bug: Bug 745278 - [RFE] ECC encryption keys cannot be archived
Please review the following patches (see "BEFORE you review" at later part of
this email):
*
https://bugzilla.redhat.com/attachment.cgi?id=570109&action=diff&...
*
https://bugzilla.redhat.com/attachment.cgi?id=570110&action=diff&...
*
https://bugzilla.redhat.com/attachment.cgi?id=570112&action=diff&...
thanks,
Christina
==============
BEFORE you review:
For the ECC plan and design for the different phases, please refer to
http://pki.fedoraproject.org/wiki/ECC_in_Dogtag
Note: the designs beyond phase 2 were more like a brain dump. Although I said "Do
Not Review," you are free to take a peak at what's intended down the road. I will
go back and take a closer look and refine/adjust the designs when I begin implementation
for each new phase.
This patch contains code for the following packages:
JSS, pki-kra, pki-common, pki-util, and pki-kra-ui
What you need to know:
* Problem 1 - nethsm issue:
On the server side, if you turn on FIPS mode, in addition to nethsm, you need to attach
certicom as well to have ECC SSL working on the server side. This problem has already been
reported to Thales last year and they said they'd look into putting the item on their
next release. Recently through a different contact, we learned there might be a way to
"turn it on" (still waiting for their further instruction)
* Problem 2- Certicom issue:
This is a show-stopper. Initially, on the client side, I used Kai's special version
of Xulrunner/Firefox, attached to Certicom token, so that the CRMF requests can be
generated with key archival option. However, I encountered (or, re-encountered) an issue
with certicom token. Certicom generates ECC keys with the wrong format (not PKCS7
conforming), which makes ECC key archival impossible on the server side if you use
non-certicom token with DRM (but we expect an HSM in most product deployment). I have
contacted Certicom for this issue, and they confirmed that they indeed have such issue. We
are waiting for their fix.
But then you might ask, "I thought I saw some ECC enrollment profiles/javascripts
being checked in? How were the tests done?" The tests for those profiles were done
against this ECC key archival/recovery DRM prototype I implemented last year (needs to be
turned on manually in 8.1), where I "cheated" (yeah, that's why it's
called a prototype) by decrypting the private key in the CRMF on DRM, and then
manipulating the byte array to strip off the offending bytes before archival.
In the real, non-prototype implementation, which is what's in this patch, for
security reasons, private keys are unwrapped directly onto the token during key archival,
so there is no way to manipulate the keys in memory and bypass the Certicom issue.
A word about Kai's special version of Xulrunner/Firefox. It is not yet publicly
available.
* Problem 3- Firefox with nethsm issue:
Another option was to connect Kai's special version firefox with an HSM to test my
DRM/JSS code. However, for whatever reason, I could not get SSL going between such Firefox
and ECC CA ( I did not try very hard though, as I have one other option -- writing my own
ECC CRMF generation tool. I might come back to try the nethsm Firefox idea later)
My solution (how I work on this official implementation):
* I hacked up a ECC CRMF tool by taking the CRMFPopClient (existing in current releases),
gutting out the RSA part of the code, and replacing it with ECC code. I call it
CRMFPopClientEC. Two types of ECC key pairs could be generated: ECDSA or ECDH (That's
another benefit of writing my own tool -- I don't know if you can select which type to
generate in the Javascript... maybe you can, I just don't know). I'm in no way
condoning archival of signing keys!! This is just a test tool.
This tool takes a curve name as option (along with others), generates an ECC key pair,
crafts up an CRMF request with key archival option, and sends request directly to the
specified CA. You will see a "Deferred" message in the HTML response (see
attachment for example)
Once CA agent approves the request, the archival request goes to DRM and the user private
key is archived.
For recovery, DRM agent selects key recovery, etc, and you get your pkcs12.
I did some sanity test with the pkcs12 recovered:
* Import the recovered pkcs12 into a certicom library:
pk12util -d . -h "Certicom FIPS Cert/Key Services" -i userEC.p12
I also tested by retrieving a p12, importing it into a browser, and adding the user as an
agent and the user could act as agent via ssl client auth to the CA.
Finally, much of the RSA-centric code had been cleared out of the way at the time when I
worked on the DRM ECC prototype, so you don't see much of that in this round.
About SELinux:
I have a set of rules generated on my system to run in enforcing mode. I do a writeup.
For QE:
How to set up the servers? The internal wiki is at
https://wiki.idm.lab.bos.redhat.com/export/idmwiki/Working_with_ECC . I might have given
someone a copy of how to set up ECC CA to publish on fedora.org when I worked on phase1
back a couple years ago. I will put an updated copy to cover both CA and DRM when I am
checking in to dogtag.
How do you test? Well, unless you want to use my CRMFPopClientEC tool hooked up with a
nethsm (like I did), or write your own tool, you can't really test it until Certicom
fixes their issue. (BTW CRMFPopClientEC can also be changed to work with ceriticom,
although you would run into the same issue I mentioned above)
It is not on my schedule to work on this tool; It is certainly not in production quality
to be released as a regular tool. However, if you are interested in it, if we get enough
request maybe we can think about doing something with it.
Other test suggestion: I did not try to clone the ECC DRM. It's a good idea to test
it out.
For techpub:
TBD
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
_______________________________________________
Pki-devel mailing list Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
4657
days inactive
4666
days old
6 comments
3 participants
participants (3)
-
Christina -
Christina Fu
-
John Magne