11:30 p.m.
Hi all,
The attached patches implement replication support for lightweight
CAs. These patches do not implement key replication via Custodia
(my next task) but they do implement the persistent search thread
and appropriate** API behaviour when the signing keys are not yet
available.
** In most cases, we respond 503 Service Unavailable; this is open
for discussion. ca-authority-find and ca-authority-show include
a boolean field indicating whether the CA is ready to sign.
There might be (probably are) endpoints I've missed.
Cheers,
Fraser
1 a.m.
On Fri, Mar 18, 2016 at 02:30:24PM +1000, Fraser Tweedale wrote:
Hi all,
The attached patches implement replication support for lightweight
CAs. These patches do not implement key replication via Custodia
(my next task) but they do implement the persistent search thread
and appropriate** API behaviour when the signing keys are not yet
available.
** In most cases, we respond 503 Service Unavailable; this is open
for discussion. ca-authority-find and ca-authority-show include
a boolean field indicating whether the CA is ready to sign.
There might be (probably are) endpoints I've missed.
Cheers,
Fraser
Updated patches attached - small change in patch 0084 to fix a race
condition when deleting an authority that can cause NPE.
Thanks,
Fraser
11:29 a.m.
A few comments.
1. One of the first things that struck me as odd was making
CertificateAuthority implement Runnable. I think it would be cleaner
to have a static inner class called AuthorityMonitor or similar to
which we pass in the CertificateAuthority.
2. I do like the fact that the caMap updates are done through a static
database connection factory created by the hostCA. How do you ensure
that the database connection factory is created before being used by
other CAs?
3. I'm not sure I understand how the initialLoadDone counter is
supposed to work. Are all the CA's supposed to stop until the hostCA
has completed its initial load? Because it looks like only the hostCA
calls await().
4. There is a lot of code in that initial patch. It would help review
to split that off into at least two patches, say one in which you add
the functions in CertificateAuthority that handle modifications in the
caMap based on persistent search results, and one which adds the new
monitor thread.
5. Some in-code documentation would not go amiss. For instance, I have
no idea why this code is correct --
String[] objectClasses =
entry.getAttribute("objectClass").getStringValueArray();
if
(Arrays.asList(objectClasses).contains("organizationalUnit")) {
initialNumAuthorities = new Integer(
entry.getAttribute("numSubordinates")
.getStringValueArray()[0]);
checkInitialLoadDone();
continue;
}
organizationalUnit?
There are lots of different variables like initialNumAuthorities etc. which could
potentially be hidden in an inner class, making this more understandable.
Ade
On Tue, 2016-03-22 at 16:00 +1000, Fraser Tweedale wrote:
On Fri, Mar 18, 2016 at 02:30:24PM +1000, Fraser Tweedale wrote:
> Hi all,
>
> The attached patches implement replication support for lightweight
> CAs. These patches do not implement key replication via Custodia
> (my next task) but they do implement the persistent search thread
> and appropriate** API behaviour when the signing keys are not yet
> available.
>
> ** In most cases, we respond 503 Service Unavailable; this is open
> for discussion. ca-authority-find and ca-authority-show include
> a boolean field indicating whether the CA is ready to sign.
> There might be (probably are) endpoints I've missed.
>
> Cheers,
> Fraser
>
Updated patches attached - small change in patch 0084 to fix a race
condition when deleting an authority that can cause NPE.
Thanks,
Fraser
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
1:37 a.m.
Ade, thanks for the review. I address your comments inline.
Updated patches are attached, and there are new patches implementing
schema changes and the key acquisition framework. Pursuant to
comment (4), I have split patch 0084 into six different patches. To
keep things in order I retire patch numbers 0084-0086 and start over
from 0087:
/-0087 add CAMissingKeyException class
/ 0088 use static db connection factory
0084 ___/ 0089 avoid repeat definition of authorities DN
\ 0090 move host authority creation out of load method
\ 0091 extract LDAP commit/delete methods
\_0092 monitor database for changes
0085 -> 0093 set DN based on data from LDAP
0086 -> 0094 indicate when CA does not yet have keys
(new) -> 0095 authority schema changes
(new) -> 0096 add key retrieval framework
On Thu, Mar 24, 2016 at 12:29:24PM -0400, Ade Lee wrote:
A few comments.
1. One of the first things that struck me as odd was making
CertificateAuthority implement Runnable. I think it would be cleaner
to have a static inner class called AuthorityMonitor or similar to
which we pass in the CertificateAuthority.
This was originally the way that LDAPProfileSubsystem was
structured, and the review feedback there was that we accessed so
many members of the enclosing class that it would be better to make
the enclosing class Runnable and migrate the methods there.
So... grass is always greener? :) Based on the LDAPProfileSubsystem
experience, I'm not inclined to make this change.
2. I do like the fact that the caMap updates are done through a
static
database connection factory created by the hostCA. How do you ensure
that the database connection factory is created before being used by
other CAs?
It is initialised before other CertificateAuthority instances are
instantiated. I added a comment.
3. I'm not sure I understand how the initialLoadDone counter is
supposed to work. Are all the CA's supposed to stop until the hostCA
has completed its initial load? Because it looks like only the hostCA
calls await().
Yep, it's used to block only the host CA's 'init' method while
lightweight CAs are loaded by the monitor thread. I added a
comment.
4. There is a lot of code in that initial patch. It would help
review
to split that off into at least two patches, say one in which you add
the functions in CertificateAuthority that handle modifications in the
caMap based on persistent search results, and one which adds the new
monitor thread.
Split into six patches, as discussed above.
5. Some in-code documentation would not go amiss. For instance, I
have
no idea why this code is correct --
String[] objectClasses =
entry.getAttribute("objectClass").getStringValueArray();
if
(Arrays.asList(objectClasses).contains("organizationalUnit")) {
initialNumAuthorities = new Integer(
entry.getAttribute("numSubordinates")
.getStringValueArray()[0]);
checkInitialLoadDone();
continue;
}
organizationalUnit?
I added detailed commentary about what's going on here.
There are lots of different variables like initialNumAuthorities etc.
which could
potentially be hidden in an inner class, making this more understandable.
The declarations are grouped together, and hopefully things are made
clearer by the split patches. If you still feel the updated patches
would benefit substantially from grouping tracking vars in an inner
record type, I will make the change.
Many thanks again for the review!
Cheers,
Fraser
Ade
On Tue, 2016-03-22 at 16:00 +1000, Fraser Tweedale wrote:
> On Fri, Mar 18, 2016 at 02:30:24PM +1000, Fraser Tweedale wrote:
> > Hi all,
> >
> > The attached patches implement replication support for lightweight
> > CAs. These patches do not implement key replication via Custodia
> > (my next task) but they do implement the persistent search thread
> > and appropriate** API behaviour when the signing keys are not yet
> > available.
> >
> > ** In most cases, we respond 503 Service Unavailable; this is open
> > for discussion. ca-authority-find and ca-authority-show include
> > a boolean field indicating whether the CA is ready to sign.
> > There might be (probably are) endpoints I've missed.
> >
> > Cheers,
> > Fraser
> >
> Updated patches attached - small change in patch 0084 to fix a race
> condition when deleting an authority that can cause NPE.
>
> Thanks,
> Fraser
> _______________________________________________
> Pki-devel mailing list
> Pki-devel(a)redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel
10:16 a.m.
On Thu, 2016-03-31 at 16:37 +1000, Fraser Tweedale wrote:
Ade, thanks for the review. I address your comments inline.
Updated patches are attached, and there are new patches implementing
schema changes and the key acquisition framework. Pursuant to
comment (4), I have split patch 0084 into six different patches. To
keep things in order I retire patch numbers 0084-0086 and start over
from 0087:
/-0087 add CAMissingKeyException class
/ 0088 use static db connection factory
0084 ___/ 0089 avoid repeat definition of authorities DN
\ 0090 move host authority creation out of load method
\ 0091 extract LDAP commit/delete methods
\_0092 monitor database for changes
0085 -> 0093 set DN based on data from LDAP
0086 -> 0094 indicate when CA does not yet have keys
(new) -> 0095 authority schema changes
(new) -> 0096 add key retrieval framework
0087
1. In SigningUnit.java -- you catch an ObjectNotFound exception and
rethrow that as a CAMissingKey exception. Is that the only way the
ObjectNotFound exception can be thrown? What if the key is present but
the cert is not? Can we refactor here to ensure that the correct
exception is thrown?
0088:
2. What does dbFactory.reset() do and does it need to be called in a
cleanup routine somewhere? Are we leaking resources?
Answered I think on IRC. It just terminates any current connections -
but do we need to call it on CA shutdown?
0089: ACK
0090: ACK
0091: ACK (with proviso below)
3. Not super-crazy about the names of the methods commitAuthority(),
commitModifyAuthority and deleteAuthorityEntry(). They are not very
consistent. I would suggest addAuthorityEntry(),
modifyAuthorityEntry() and deleteAuthorityEntry() instead.
0092: ACK (with following proviso)
4. Talking with Nathan about this, he suggested that syncrepl is then
more modern and preferred method to perform persistent searches. In
fact, I see IPA tickets to replace persistent searches with syncrepl
instead.
We could replace the persistent search with a separate follow-on patch
if you prefer, or just do it now.
0093 : ACK
0094: ACK
0095: ACK
0096: Looks good in general.
5. One thing to keep in mind though. It is perfectly possible to have
more than one dogtag instance on a host. What determines the
uniqueness of the instance is the host:port.
Ade
On Thu, Mar 24, 2016 at 12:29:24PM -0400, Ade Lee wrote:
> A few comments.
>
> 1. One of the first things that struck me as odd was making
> CertificateAuthority implement Runnable. I think it would be
> cleaner
> to have a static inner class called AuthorityMonitor or similar to
> which we pass in the CertificateAuthority.
>
This was originally the way that LDAPProfileSubsystem was
structured, and the review feedback there was that we accessed so
many members of the enclosing class that it would be better to make
the enclosing class Runnable and migrate the methods there.
So... grass is always greener? :) Based on the LDAPProfileSubsystem
experience, I'm not inclined to make this change.
> 2. I do like the fact that the caMap updates are done through a
> static
> database connection factory created by the hostCA. How do you
> ensure
> that the database connection factory is created before being used
> by
> other CAs?
>
It is initialised before other CertificateAuthority instances are
instantiated. I added a comment.
> 3. I'm not sure I understand how the initialLoadDone counter is
> supposed to work. Are all the CA's supposed to stop until the
> hostCA
> has completed its initial load? Because it looks like only the
> hostCA
> calls await().
>
Yep, it's used to block only the host CA's 'init' method while
lightweight CAs are loaded by the monitor thread. I added a
comment.
> 4. There is a lot of code in that initial patch. It would help
> review
> to split that off into at least two patches, say one in which you
> add
> the functions in CertificateAuthority that handle modifications in
> the
> caMap based on persistent search results, and one which adds the
> new
> monitor thread.
>
Split into six patches, as discussed above.
> 5. Some in-code documentation would not go amiss. For instance, I
> have
> no idea why this code is correct --
>
> String[] objectClasses =
>
> entry.getAttribute("objectClass").getStringValueArray();
> if
> (Arrays.asList(objectClasses).contains("organizationalUnit")) {
> initialNumAuthorities = new Integer(
> entry.getAttribute("numSubordinates")
> .getStringValueArray()[0]);
> checkInitialLoadDone();
> continue;
> }
> organizationalUnit?
>
I added detailed commentary about what's going on here.
> There are lots of different variables like initialNumAuthorities
> etc. which could
> potentially be hidden in an inner class, making this more
> understandable.
>
The declarations are grouped together, and hopefully things are made
clearer by the split patches. If you still feel the updated patches
would benefit substantially from grouping tracking vars in an inner
record type, I will make the change.
Many thanks again for the review!
Cheers,
Fraser
> Ade
>
> On Tue, 2016-03-22 at 16:00 +1000, Fraser Tweedale wrote:
> > On Fri, Mar 18, 2016 at 02:30:24PM +1000, Fraser Tweedale wrote:
> > > Hi all,
> > >
> > > The attached patches implement replication support for
> > > lightweight
> > > CAs. These patches do not implement key replication via
> > > Custodia
> > > (my next task) but they do implement the persistent search
> > > thread
> > > and appropriate** API behaviour when the signing keys are not
> > > yet
> > > available.
> > >
> > > ** In most cases, we respond 503 Service Unavailable; this is
> > > open
> > > for discussion. ca-authority-find and ca-authority-show
> > > include
> > > a boolean field indicating whether the CA is ready to sign.
> > > There might be (probably are) endpoints I've missed.
> > >
> > > Cheers,
> > > Fraser
> > >
> > Updated patches attached - small change in patch 0084 to fix a
> > race
> > condition when deleting an authority that can cause NPE.
> >
> > Thanks,
> > Fraser
> > _______________________________________________
> > Pki-devel mailing list
> > Pki-devel(a)redhat.com
> > https://www.redhat.com/mailman/listinfo/pki-devel
9:32 p.m.
Thanks for review, Ade. Comments to specific feedback inline.
Rebased and updated patches attached. The substantive changes are:
- KeyRetriever implementations are now required NOT to import the
key themselves. Instead the API is updated with
KeyRetriever.retrieveKey returning a Result, which contains PKCS
#12 data and password for same.
- KeyRetrieverRunner reads the Result and imports the PKCS #12 into
NSSDB.
- Added new patch 0097 which provides the IPACustodiaKeyRetriever
and assoicated Python helper script. It depends on an unmerged
FreeIPA patch[1] as well as a particular principal and associated
keytab and Custodia keys existing. I'm working on FreeIPA updates
to satisfy these requirements automatically on install or upgrade
but if you want to test this patch LMK and I'll provide detailed
instructions.
[1] https://www.redhat.com/archives/freeipa-devel/2016-April/msg00055.html
Other comments inline.
Cheers,
Fraser
On Fri, Apr 08, 2016 at 11:16:19AM -0400, Ade Lee wrote:
0087
1. In SigningUnit.java -- you catch an ObjectNotFound exception and
rethrow that as a CAMissingKey exception. Is that the only way the
ObjectNotFound exception can be thrown? What if the key is present but
the cert is not? Can we refactor here to ensure that the correct
exception is thrown?
One can't get additional info out of ObjectNotFound without
inspecting the String message, which I'm not comfortable doing. The
key retrieval system should import key and cert at same time so I've
renamed the exception to CAMissingKeyOrCert for clarity.
0088:
2. What does dbFactory.reset() do and does it need to be called in a
cleanup routine somewhere? Are we leaking resources?
Answered I think on IRC. It just terminates any current connections -
but do we need to call it on CA shutdown?
dbFactory.reset() is already called in the shutdown() method. (Only
the host authority calls it).
0089: ACK
0090: ACK
0091: ACK (with proviso below)
3. Not super-crazy about the names of the methods commitAuthority(),
commitModifyAuthority and deleteAuthorityEntry(). They are not very
consistent. I would suggest addAuthorityEntry(),
modifyAuthorityEntry() and deleteAuthorityEntry() instead.
Done.
0092: ACK (with following proviso)
4. Talking with Nathan about this, he suggested that syncrepl is then
more modern and preferred method to perform persistent searches. In
fact, I see IPA tickets to replace persistent searches with syncrepl
instead.
We could replace the persistent search with a separate follow-on patch
if you prefer, or just do it now.
Syncrepl is not supported by ldapjdk (iirc). If/when it is, and if
syncrepl provides a tangible advantage over persistent search in our
use case (where it can be assumed that disconnections from DS are
infrequent and brief, and full refresh of local view is tolerable),
then I am happy to change it - in a separate commit (because
LDAPProfileSubsystem is also affected).
0093 : ACK
0094: ACK
0095: ACK
0096: Looks good in general.
5. One thing to keep in mind though. It is perfectly possible to have
more than one dogtag instance on a host. What determines the
uniqueness of the instance is the host:port.
Noted. KeyRetriever implementations can access instance info via
the `CMS' class.
Cheers,
Fraser
4:26 p.m.
Still reviewing ..
See comment on 87. ACK on 88,89,90,91,92,93, 94, 95.
Ade
On Mon, 2016-04-11 at 12:32 +1000, Fraser Tweedale wrote:
Thanks for review, Ade. Comments to specific feedback inline.
Rebased and updated patches attached. The substantive changes are:
- KeyRetriever implementations are now required NOT to import the
key themselves. Instead the API is updated with
KeyRetriever.retrieveKey returning a Result, which contains PKCS
#12 data and password for same.
- KeyRetrieverRunner reads the Result and imports the PKCS #12 into
NSSDB.
- Added new patch 0097 which provides the IPACustodiaKeyRetriever
and assoicated Python helper script. It depends on an unmerged
FreeIPA patch[1] as well as a particular principal and associated
keytab and Custodia keys existing. I'm working on FreeIPA updates
to satisfy these requirements automatically on install or upgrade
but if you want to test this patch LMK and I'll provide detailed
instructions.
[1] https://www.redhat.com/archives/freeipa-devel/2016-April/msg000
55.html
Other comments inline.
Cheers,
Fraser
On Fri, Apr 08, 2016 at 11:16:19AM -0400, Ade Lee wrote:
>
> 0087
>
> 1. In SigningUnit.java -- you catch an ObjectNotFound exception and
> rethrow that as a CAMissingKey exception. Is that the only way the
> ObjectNotFound exception can be thrown? What if the key is present
> but
> the cert is not? Can we refactor here to ensure that the correct
> exception is thrown?
>
One can't get additional info out of ObjectNotFound without
inspecting the String message, which I'm not comfortable doing. The
key retrieval system should import key and cert at same time so I've
renamed the exception to CAMissingKeyOrCert for clarity.
Well, you can always nest exceptions like so :
mToken.login(cb); // ONE_TIME by default.
try {
mCert = mManager.findCertByNickname(mNickname);
CMS.debug("Found cert by nickname: '" + mNickname +
"' with serial number: " + mCert.getSerialNumber());
mCertImpl = new X509CertImpl(mCert.getEncoded());
CMS.debug("converted to x509CertImpl");
} catch (ObjectNotFoundException e) {
throw new CAMissingCertException();
}
try {
mPrivk = mManager.findPrivKeyByCert(mCert);
CMS.debug("Got private key from cert");
} catch (ObjectNotFoundException e) {
throw new CAMissingKeyException();
}
....
The only reason that I suggest this is that I could imagine this kind
of differentiation being useful in debugging failed custodia
replications. If you think otherwise, I'm prepare to be convinced
otherwise.
> 0088:
>
> 2. What does dbFactory.reset() do and does it need to be called in
> a
> cleanup routine somewhere? Are we leaking resources?
>
> Answered I think on IRC. It just terminates any current
> connections -
> but do we need to call it on CA shutdown?
>
dbFactory.reset() is already called in the shutdown() method. (Only
the host authority calls it).
> 0089: ACK
>
> 0090: ACK
>
> 0091: ACK (with proviso below)
>
> 3. Not super-crazy about the names of the methods
> commitAuthority(),
> commitModifyAuthority and deleteAuthorityEntry(). They are not
> very
> consistent. I would suggest addAuthorityEntry(),
> modifyAuthorityEntry() and deleteAuthorityEntry() instead.
>
Done.
> 0092: ACK (with following proviso)
>
> 4. Talking with Nathan about this, he suggested that syncrepl is
> then
> more modern and preferred method to perform persistent searches.
> In
> fact, I see IPA tickets to replace persistent searches with
> syncrepl
> instead.
>
> We could replace the persistent search with a separate follow-on
> patch
> if you prefer, or just do it now.
>
Syncrepl is not supported by ldapjdk (iirc). If/when it is, and if
syncrepl provides a tangible advantage over persistent search in our
use case (where it can be assumed that disconnections from DS are
infrequent and brief, and full refresh of local view is tolerable),
then I am happy to change it - in a separate commit (because
LDAPProfileSubsystem is also affected).
> 0093 : ACK
>
> 0094: ACK
>
> 0095: ACK
>
> 0096: Looks good in general.
>
> 5. One thing to keep in mind though. It is perfectly possible to
> have
> more than one dogtag instance on a host. What determines the
> uniqueness of the instance is the host:port.
>
Noted. KeyRetriever implementations can access instance info via
the `CMS' class.
Cheers,
Fraser
6:04 p.m.
On Wed, Apr 13, 2016 at 05:26:44PM -0400, Ade Lee wrote:
Still reviewing ..
See comment on 87. ACK on 88,89,90,91,92,93, 94, 95.
Ade
On Mon, 2016-04-11 at 12:32 +1000, Fraser Tweedale wrote:
> Thanks for review, Ade. Comments to specific feedback inline.
> Rebased and updated patches attached. The substantive changes are:
>
> - KeyRetriever implementations are now required NOT to import the
> key themselves. Instead the API is updated with
> KeyRetriever.retrieveKey returning a Result, which contains PKCS
> #12 data and password for same.
>
> - KeyRetrieverRunner reads the Result and imports the PKCS #12 into
> NSSDB.
>
> - Added new patch 0097 which provides the IPACustodiaKeyRetriever
> and assoicated Python helper script. It depends on an unmerged
> FreeIPA patch[1] as well as a particular principal and associated
> keytab and Custodia keys existing. I'm working on FreeIPA updates
> to satisfy these requirements automatically on install or upgrade
> but if you want to test this patch LMK and I'll provide detailed
> instructions.
>
> [1] https://www.redhat.com/archives/freeipa-devel/2016-April/msg000
> 55.html
>
> Other comments inline.
>
> Cheers,
> Fraser
>
> On Fri, Apr 08, 2016 at 11:16:19AM -0400, Ade Lee wrote:
> >
> > 0087
> >
> > 1. In SigningUnit.java -- you catch an ObjectNotFound exception and
> > rethrow that as a CAMissingKey exception. Is that the only way the
> > ObjectNotFound exception can be thrown? What if the key is present
> > but
> > the cert is not? Can we refactor here to ensure that the correct
> > exception is thrown?
> >
> One can't get additional info out of ObjectNotFound without
> inspecting the String message, which I'm not comfortable doing. The
> key retrieval system should import key and cert at same time so I've
> renamed the exception to CAMissingKeyOrCert for clarity.
>
Well, you can always nest exceptions like so :
mToken.login(cb); // ONE_TIME by default.
try {
mCert = mManager.findCertByNickname(mNickname);
CMS.debug("Found cert by nickname: '" + mNickname +
"' with serial number: " + mCert.getSerialNumber());
mCertImpl = new X509CertImpl(mCert.getEncoded());
CMS.debug("converted to x509CertImpl");
} catch (ObjectNotFoundException e) {
throw new CAMissingCertException();
}
try {
mPrivk = mManager.findPrivKeyByCert(mCert);
CMS.debug("Got private key from cert");
} catch (ObjectNotFoundException e) {
throw new CAMissingKeyException();
}
....
The only reason that I suggest this is that I could imagine this kind
of differentiation being useful in debugging failed custodia
replications. If you think otherwise, I'm prepare to be convinced
otherwise.
I think a scenario where we get key but not cert, or vice versa, is
unlikely (Custodia gives us a PKCS #12 file with both). However,
your suggestion should work and it is a relatively small change.
I'll cut a new patchset with this change today, along with the
rebase.
Cheers,
Fraser
> > 0088:
> >
> > 2. What does dbFactory.reset() do and does it need to be called in
> > a
> > cleanup routine somewhere? Are we leaking resources?
> >
> > Answered I think on IRC. It just terminates any current
> > connections -
> > but do we need to call it on CA shutdown?
> >
> dbFactory.reset() is already called in the shutdown() method. (Only
> the host authority calls it).
>
> > 0089: ACK
> >
> > 0090: ACK
> >
> > 0091: ACK (with proviso below)
> >
> > 3. Not super-crazy about the names of the methods
> > commitAuthority(),
> > commitModifyAuthority and deleteAuthorityEntry(). They are not
> > very
> > consistent. I would suggest addAuthorityEntry(),
> > modifyAuthorityEntry() and deleteAuthorityEntry() instead.
> >
> Done.
>
> > 0092: ACK (with following proviso)
> >
> > 4. Talking with Nathan about this, he suggested that syncrepl is
> > then
> > more modern and preferred method to perform persistent searches.
> > In
> > fact, I see IPA tickets to replace persistent searches with
> > syncrepl
> > instead.
> >
> > We could replace the persistent search with a separate follow-on
> > patch
> > if you prefer, or just do it now.
> >
> Syncrepl is not supported by ldapjdk (iirc). If/when it is, and if
> syncrepl provides a tangible advantage over persistent search in our
> use case (where it can be assumed that disconnections from DS are
> infrequent and brief, and full refresh of local view is tolerable),
> then I am happy to change it - in a separate commit (because
> LDAPProfileSubsystem is also affected).
>
> > 0093 : ACK
> >
> > 0094: ACK
> >
> > 0095: ACK
> >
> > 0096: Looks good in general.
> >
> > 5. One thing to keep in mind though. It is perfectly possible to
> > have
> > more than one dogtag instance on a host. What determines the
> > uniqueness of the instance is the host:port.
> >
> Noted. KeyRetriever implementations can access instance info via
> the `CMS' class.
>
> Cheers,
> Fraser
1:18 a.m.
On Thu, Apr 14, 2016 at 09:04:31AM +1000, Fraser Tweedale wrote:
On Wed, Apr 13, 2016 at 05:26:44PM -0400, Ade Lee wrote:
> Still reviewing ..
>
> See comment on 87. ACK on 88,89,90,91,92,93, 94, 95.
>
> Ade
>
> On Mon, 2016-04-11 at 12:32 +1000, Fraser Tweedale wrote:
> > Thanks for review, Ade. Comments to specific feedback inline.
> > Rebased and updated patches attached. The substantive changes are:
> >
> > - KeyRetriever implementations are now required NOT to import the
> > key themselves. Instead the API is updated with
> > KeyRetriever.retrieveKey returning a Result, which contains PKCS
> > #12 data and password for same.
> >
> > - KeyRetrieverRunner reads the Result and imports the PKCS #12 into
> > NSSDB.
> >
> > - Added new patch 0097 which provides the IPACustodiaKeyRetriever
> > and assoicated Python helper script. It depends on an unmerged
> > FreeIPA patch[1] as well as a particular principal and associated
> > keytab and Custodia keys existing. I'm working on FreeIPA updates
> > to satisfy these requirements automatically on install or upgrade
> > but if you want to test this patch LMK and I'll provide detailed
> > instructions.
> >
> > [1] https://www.redhat.com/archives/freeipa-devel/2016-April/msg000
> > 55.html
> >
> > Other comments inline.
> >
> > Cheers,
> > Fraser
> >
> > On Fri, Apr 08, 2016 at 11:16:19AM -0400, Ade Lee wrote:
> > >
> > > 0087
> > >
> > > 1. In SigningUnit.java -- you catch an ObjectNotFound exception and
> > > rethrow that as a CAMissingKey exception. Is that the only way the
> > > ObjectNotFound exception can be thrown? What if the key is present
> > > but
> > > the cert is not? Can we refactor here to ensure that the correct
> > > exception is thrown?
> > >
> > One can't get additional info out of ObjectNotFound without
> > inspecting the String message, which I'm not comfortable doing. The
> > key retrieval system should import key and cert at same time so I've
> > renamed the exception to CAMissingKeyOrCert for clarity.
> >
>
> Well, you can always nest exceptions like so :
>
> mToken.login(cb); // ONE_TIME by default.
>
> try {
> mCert = mManager.findCertByNickname(mNickname);
> CMS.debug("Found cert by nickname: '" + mNickname +
"' with serial number: " + mCert.getSerialNumber());
>
> mCertImpl = new X509CertImpl(mCert.getEncoded());
> CMS.debug("converted to x509CertImpl");
> } catch (ObjectNotFoundException e) {
> throw new CAMissingCertException();
> }
>
> try {
> mPrivk = mManager.findPrivKeyByCert(mCert);
> CMS.debug("Got private key from cert");
> } catch (ObjectNotFoundException e) {
> throw new CAMissingKeyException();
> }
> ....
>
> The only reason that I suggest this is that I could imagine this kind
> of differentiation being useful in debugging failed custodia
> replications. If you think otherwise, I'm prepare to be convinced
> otherwise.
>
I think a scenario where we get key but not cert, or vice versa, is
unlikely (Custodia gives us a PKCS #12 file with both). However,
your suggestion should work and it is a relatively small change.
I'll cut a new patchset with this change today, along with the
rebase.
Updated and rebased patches attached.
The suggested changes were made to 0087. This also resulted in
changes to patch 0094 (indicate when CA does not yet have keys).
No substantive changes to any other patches.
Cheers,
Fraser
3:35 p.m.
Still reviewing .. ACK on 87-95 (inclusive).
On Thu, 2016-04-14 at 16:18 +1000, Fraser Tweedale wrote:
On Thu, Apr 14, 2016 at 09:04:31AM +1000, Fraser Tweedale wrote:
> On Wed, Apr 13, 2016 at 05:26:44PM -0400, Ade Lee wrote:
> > Still reviewing ..
> >
> > See comment on 87. ACK on 88,89,90,91,92,93, 94, 95.
> >
> > Ade
> >
> > On Mon, 2016-04-11 at 12:32 +1000, Fraser Tweedale wrote:
> > > Thanks for review, Ade. Comments to specific feedback inline.
> > > Rebased and updated patches attached. The substantive changes
> > > are:
> > >
> > > - KeyRetriever implementations are now required NOT to import
> > > the
> > > key themselves. Instead the API is updated with
> > > KeyRetriever.retrieveKey returning a Result, which contains
> > > PKCS
> > > #12 data and password for same.
> > >
> > > - KeyRetrieverRunner reads the Result and imports the PKCS #12
> > > into
> > > NSSDB.
> > >
> > > - Added new patch 0097 which provides the
> > > IPACustodiaKeyRetriever
> > > and assoicated Python helper script. It depends on an
> > > unmerged
> > > FreeIPA patch[1] as well as a particular principal and
> > > associated
> > > keytab and Custodia keys existing. I'm working on FreeIPA
> > > updates
> > > to satisfy these requirements automatically on install or
> > > upgrade
> > > but if you want to test this patch LMK and I'll provide
> > > detailed
> > > instructions.
> > >
> > > [1]
> > > https://www.redhat.com/archives/freeipa-devel/2016-April/msg000
> > > 55.html
> > >
> > > Other comments inline.
> > >
> > > Cheers,
> > > Fraser
> > >
> > > On Fri, Apr 08, 2016 at 11:16:19AM -0400, Ade Lee wrote:
> > > >
> > > > 0087
> > > >
> > > > 1. In SigningUnit.java -- you catch an ObjectNotFound
> > > > exception and
> > > > rethrow that as a CAMissingKey exception. Is that the only
> > > > way the
> > > > ObjectNotFound exception can be thrown? What if the key is
> > > > present
> > > > but
> > > > the cert is not? Can we refactor here to ensure that the
> > > > correct
> > > > exception is thrown?
> > > >
> > > One can't get additional info out of ObjectNotFound without
> > > inspecting the String message, which I'm not comfortable doing.
> > > The
> > > key retrieval system should import key and cert at same time so
> > > I've
> > > renamed the exception to CAMissingKeyOrCert for clarity.
> > >
> >
> > Well, you can always nest exceptions like so :
> >
> > mToken.login(cb); // ONE_TIME by default.
> >
> > try {
> > mCert = mManager.findCertByNickname(mNickname);
> > CMS.debug("Found cert by nickname: '" +
mNickname
> > + "' with serial number: " + mCert.getSerialNumber());
> >
> > mCertImpl = new X509CertImpl(mCert.getEncoded());
> > CMS.debug("converted to x509CertImpl");
> > } catch (ObjectNotFoundException e) {
> > throw new CAMissingCertException();
> > }
> >
> > try {
> > mPrivk = mManager.findPrivKeyByCert(mCert);
> > CMS.debug("Got private key from cert");
> > } catch (ObjectNotFoundException e) {
> > throw new CAMissingKeyException();
> > }
> > ....
> >
> > The only reason that I suggest this is that I could imagine this
> > kind
> > of differentiation being useful in debugging failed custodia
> > replications. If you think otherwise, I'm prepare to be
> > convinced
> > otherwise.
> >
> I think a scenario where we get key but not cert, or vice versa, is
> unlikely (Custodia gives us a PKCS #12 file with both). However,
> your suggestion should work and it is a relatively small change.
> I'll cut a new patchset with this change today, along with the
> rebase.
>
Updated and rebased patches attached.
The suggested changes were made to 0087. This also resulted in
changes to patch 0094 (indicate when CA does not yet have keys).
No substantive changes to any other patches.
Cheers,
Fraser
4:34 p.m.
Couple of points on 96/97.
1. First off, I'm not sure you followed my concern about being able to
distinguish between CA instances.
On an IPA system, this is not an issue because there is only one CA on
the server. In this case, I imagine there will be a well known
directory which custodia would work with.
In general though, we have to imagine that someone could end up
installing two different dogtag ca instances on the same server.
CMS.getEEHost() would result in the same value (the hostname) for both
CAs. How does your helper program (or custodia) know which key to
retrieve?
The way to distinguish Dogtag instances is host AND port.
2. So, we're very careful that the signing keys are never in memory in
the server. All accesses to the system certs are through JSS/NSS which
essentially provides us handles to the keys.
Now, I see a case where we import PKCS12 data AND the password into
memory, so that we can import it into NSS? Say it ain't so ..
With custodia, we have a secure mechanism of transferring the keys from
one server to another. It makes more sense to me to have the server
kick off the custodia transfer and then have that process also import
into the NSS db. The server would then need to await status from the
custodia/retriever process - and then initialize the signing unit from
the NSS DB. Or am I completely confused?
Ade
On Thu, 2016-04-14 at 16:35 -0400, Ade Lee wrote:
Still reviewing .. ACK on 87-95 (inclusive).
On Thu, 2016-04-14 at 16:18 +1000, Fraser Tweedale wrote:
> On Thu, Apr 14, 2016 at 09:04:31AM +1000, Fraser Tweedale wrote:
> > On Wed, Apr 13, 2016 at 05:26:44PM -0400, Ade Lee wrote:
> > > Still reviewing ..
> > >
> > > See comment on 87. ACK on 88,89,90,91,92,93, 94, 95.
> > >
> > > Ade
> > >
> > > On Mon, 2016-04-11 at 12:32 +1000, Fraser Tweedale wrote:
> > > > Thanks for review, Ade. Comments to specific feedback
> > > > inline.
> > > > Rebased and updated patches attached. The substantive
> > > > changes
> > > > are:
> > > >
> > > > - KeyRetriever implementations are now required NOT to import
> > > > the
> > > > key themselves. Instead the API is updated with
> > > > KeyRetriever.retrieveKey returning a Result, which contains
> > > > PKCS
> > > > #12 data and password for same.
> > > >
> > > > - KeyRetrieverRunner reads the Result and imports the PKCS
> > > > #12
> > > > into
> > > > NSSDB.
> > > >
> > > > - Added new patch 0097 which provides the
> > > > IPACustodiaKeyRetriever
> > > > and assoicated Python helper script. It depends on an
> > > > unmerged
> > > > FreeIPA patch[1] as well as a particular principal and
> > > > associated
> > > > keytab and Custodia keys existing. I'm working on FreeIPA
> > > > updates
> > > > to satisfy these requirements automatically on install or
> > > > upgrade
> > > > but if you want to test this patch LMK and I'll provide
> > > > detailed
> > > > instructions.
> > > >
> > > > [1]
> > > > https://www.redhat.com/archives/freeipa-devel/2016-April/msg0
> > > > 00
> > > > 55.html
> > > >
> > > > Other comments inline.
> > > >
> > > > Cheers,
> > > > Fraser
> > > >
> > > > On Fri, Apr 08, 2016 at 11:16:19AM -0400, Ade Lee wrote:
> > > > >
> > > > > 0087
> > > > >
> > > > > 1. In SigningUnit.java -- you catch an ObjectNotFound
> > > > > exception and
> > > > > rethrow that as a CAMissingKey exception. Is that the only
> > > > > way the
> > > > > ObjectNotFound exception can be thrown? What if the key is
> > > > > present
> > > > > but
> > > > > the cert is not? Can we refactor here to ensure that the
> > > > > correct
> > > > > exception is thrown?
> > > > >
> > > > One can't get additional info out of ObjectNotFound without
> > > > inspecting the String message, which I'm not comfortable
> > > > doing.
> > > > The
> > > > key retrieval system should import key and cert at same time
> > > > so
> > > > I've
> > > > renamed the exception to CAMissingKeyOrCert for clarity.
> > > >
> > >
> > > Well, you can always nest exceptions like so :
> > >
> > > mToken.login(cb); // ONE_TIME by default.
> > >
> > > try {
> > > mCert = mManager.findCertByNickname(mNickname);
> > > CMS.debug("Found cert by nickname: '" +
> > > mNickname
> > > + "' with serial number: " + mCert.getSerialNumber());
> > >
> > > mCertImpl = new
> > > X509CertImpl(mCert.getEncoded());
> > > CMS.debug("converted to x509CertImpl");
> > > } catch (ObjectNotFoundException e) {
> > > throw new CAMissingCertException();
> > > }
> > >
> > > try {
> > > mPrivk = mManager.findPrivKeyByCert(mCert);
> > > CMS.debug("Got private key from cert");
> > > } catch (ObjectNotFoundException e) {
> > > throw new CAMissingKeyException();
> > > }
> > > ....
> > >
> > > The only reason that I suggest this is that I could imagine
> > > this
> > > kind
> > > of differentiation being useful in debugging failed custodia
> > > replications. If you think otherwise, I'm prepare to be
> > > convinced
> > > otherwise.
> > >
> > I think a scenario where we get key but not cert, or vice versa,
> > is
> > unlikely (Custodia gives us a PKCS #12 file with both). However,
> > your suggestion should work and it is a relatively small change.
> > I'll cut a new patchset with this change today, along with the
> > rebase.
> >
> Updated and rebased patches attached.
>
> The suggested changes were made to 0087. This also resulted in
> changes to patch 0094 (indicate when CA does not yet have keys).
>
> No substantive changes to any other patches.
>
> Cheers,
7:33 p.m.
On Thu, Apr 14, 2016 at 05:34:45PM -0400, Ade Lee wrote:
Couple of points on 96/97.
1. First off, I'm not sure you followed my concern about being able to
distinguish between CA instances.
On an IPA system, this is not an issue because there is only one CA on
the server. In this case, I imagine there will be a well known
directory which custodia would work with.
In general though, we have to imagine that someone could end up
installing two different dogtag ca instances on the same server.
CMS.getEEHost() would result in the same value (the hostname) for both
CAs. How does your helper program (or custodia) know which key to
retrieve?
Because it is running in IPA deployment, it contacts the Custodia
instance at https://<hostname>/ipa/keys/... . Note that this is
IPACustodiaKeyRetriever which is design for this purpose (and no
other). Different setups will use a different KeyRetriever
implementation. It is conceivable that IPACustodiaKeyRetriever
could be generalised in future.
The way to distinguish Dogtag instances is host AND port.
Re multiple Dogtag instances, uh yes I overlooked this. For the IPA
use case it doesn't matter but I will update the code to write
`CMS.getEEHost() + ":" CMS.getEEPort()` into the authority entry.
Alternative KeyRetriever implementations can use this to
distinguish between instances.
(Or do you think separate attributes for host and port would be
better? Not much work either way.)
2. So, we're very careful that the signing keys are never in
memory in
the server. All accesses to the system certs are through JSS/NSS which
essentially provides us handles to the keys.
Now, I see a case where we import PKCS12 data AND the password into
memory, so that we can import it into NSS? Say it ain't so ..
With custodia, we have a secure mechanism of transferring the keys from
one server to another. It makes more sense to me to have the server
kick off the custodia transfer and then have that process also import
into the NSS db. The server would then need to await status from the
custodia/retriever process - and then initialize the signing unit from
the NSS DB. Or am I completely confused?
In the original implementation, Custodia put the key directly into
the NSSDB. Unfortunately, Dogtag could not observe the key unless
restarted (highly undesirable). I did not deeply investigate why (I
guess some sort of caching or locking) - but I did not find a
workaround. Even logging out and back into the Token did not help
(and caused other issues, like dropped or failed TLS connections in
other threads).
So I reluctantly redesigned it to what you see now, which works
- but I did not see the (obvious, in hindsight) problem.
Let's chat on IRC about it. I will probably need the help of
NSS/JSS gurus to make the former approach work.
Cheers,
Fraser
4:32 a.m.
Both issues addressed in latest patchset. Two new patches in the
mix; the order is:
0095-4, 0098, 0099, 0096-4, 0097-3 (tip)
I also added another attribute to schema for the authority
certificate serial number. It is not used in current code but I
have a hunch it may be needed for renewal, so I'm adding it now.
Thanks,
Fraser
On Thu, Apr 14, 2016 at 05:34:45PM -0400, Ade Lee wrote:
Couple of points on 96/97.
1. First off, I'm not sure you followed my concern about being able to
distinguish between CA instances.
On an IPA system, this is not an issue because there is only one CA on
the server. In this case, I imagine there will be a well known
directory which custodia would work with.
In general though, we have to imagine that someone could end up
installing two different dogtag ca instances on the same server.
CMS.getEEHost() would result in the same value (the hostname) for both
CAs. How does your helper program (or custodia) know which key to
retrieve?
The way to distinguish Dogtag instances is host AND port.
2. So, we're very careful that the signing keys are never in memory in
the server. All accesses to the system certs are through JSS/NSS which
essentially provides us handles to the keys.
Now, I see a case where we import PKCS12 data AND the password into
memory, so that we can import it into NSS? Say it ain't so ..
With custodia, we have a secure mechanism of transferring the keys from
one server to another. It makes more sense to me to have the server
kick off the custodia transfer and then have that process also import
into the NSS db. The server would then need to await status from the
custodia/retriever process - and then initialize the signing unit from
the NSS DB. Or am I completely confused?
Ade
On Thu, 2016-04-14 at 16:35 -0400, Ade Lee wrote:
> Still reviewing .. ACK on 87-95 (inclusive).
>
> On Thu, 2016-04-14 at 16:18 +1000, Fraser Tweedale wrote:
> > On Thu, Apr 14, 2016 at 09:04:31AM +1000, Fraser Tweedale wrote:
> > > On Wed, Apr 13, 2016 at 05:26:44PM -0400, Ade Lee wrote:
> > > > Still reviewing ..
> > > >
> > > > See comment on 87. ACK on 88,89,90,91,92,93, 94, 95.
> > > >
> > > > Ade
> > > >
> > > > On Mon, 2016-04-11 at 12:32 +1000, Fraser Tweedale wrote:
> > > > > Thanks for review, Ade. Comments to specific feedback
> > > > > inline.
> > > > > Rebased and updated patches attached. The substantive
> > > > > changes
> > > > > are:
> > > > >
> > > > > - KeyRetriever implementations are now required NOT to import
> > > > > the
> > > > > key themselves. Instead the API is updated with
> > > > > KeyRetriever.retrieveKey returning a Result, which contains
> > > > > PKCS
> > > > > #12 data and password for same.
> > > > >
> > > > > - KeyRetrieverRunner reads the Result and imports the PKCS
> > > > > #12
> > > > > into
> > > > > NSSDB.
> > > > >
> > > > > - Added new patch 0097 which provides the
> > > > > IPACustodiaKeyRetriever
> > > > > and assoicated Python helper script. It depends on an
> > > > > unmerged
> > > > > FreeIPA patch[1] as well as a particular principal and
> > > > > associated
> > > > > keytab and Custodia keys existing. I'm working on
FreeIPA
> > > > > updates
> > > > > to satisfy these requirements automatically on install or
> > > > > upgrade
> > > > > but if you want to test this patch LMK and I'll provide
> > > > > detailed
> > > > > instructions.
> > > > >
> > > > > [1]
> > > > > https://www.redhat.com/archives/freeipa-devel/2016-April/msg0
> > > > > 00
> > > > > 55.html
> > > > >
> > > > > Other comments inline.
> > > > >
> > > > > Cheers,
> > > > > Fraser
> > > > >
> > > > > On Fri, Apr 08, 2016 at 11:16:19AM -0400, Ade Lee wrote:
> > > > > >
> > > > > > 0087
> > > > > >
> > > > > > 1. In SigningUnit.java -- you catch an ObjectNotFound
> > > > > > exception and
> > > > > > rethrow that as a CAMissingKey exception. Is that the
only
> > > > > > way the
> > > > > > ObjectNotFound exception can be thrown? What if the key
is
> > > > > > present
> > > > > > but
> > > > > > the cert is not? Can we refactor here to ensure that the
> > > > > > correct
> > > > > > exception is thrown?
> > > > > >
> > > > > One can't get additional info out of ObjectNotFound without
> > > > > inspecting the String message, which I'm not comfortable
> > > > > doing.
> > > > > The
> > > > > key retrieval system should import key and cert at same time
> > > > > so
> > > > > I've
> > > > > renamed the exception to CAMissingKeyOrCert for clarity.
> > > > >
> > > >
> > > > Well, you can always nest exceptions like so :
> > > >
> > > > mToken.login(cb); // ONE_TIME by default.
> > > >
> > > > try {
> > > > mCert = mManager.findCertByNickname(mNickname);
> > > > CMS.debug("Found cert by nickname: '"
+
> > > > mNickname
> > > > + "' with serial number: " + mCert.getSerialNumber());
> > > >
> > > > mCertImpl = new
> > > > X509CertImpl(mCert.getEncoded());
> > > > CMS.debug("converted to x509CertImpl");
> > > > } catch (ObjectNotFoundException e) {
> > > > throw new CAMissingCertException();
> > > > }
> > > >
> > > > try {
> > > > mPrivk = mManager.findPrivKeyByCert(mCert);
> > > > CMS.debug("Got private key from cert");
> > > > } catch (ObjectNotFoundException e) {
> > > > throw new CAMissingKeyException();
> > > > }
> > > > ....
> > > >
> > > > The only reason that I suggest this is that I could imagine
> > > > this
> > > > kind
> > > > of differentiation being useful in debugging failed custodia
> > > > replications. If you think otherwise, I'm prepare to be
> > > > convinced
> > > > otherwise.
> > > >
> > > I think a scenario where we get key but not cert, or vice versa,
> > > is
> > > unlikely (Custodia gives us a PKCS #12 file with both). However,
> > > your suggestion should work and it is a relatively small change.
> > > I'll cut a new patchset with this change today, along with the
> > > rebase.
> > >
> > Updated and rebased patches attached.
> >
> > The suggested changes were made to 0087. This also resulted in
> > changes to patch 0094 (indicate when CA does not yet have keys).
> >
> > No substantive changes to any other patches.
> >
> > Cheers,
1:15 a.m.
New version of 0097 attached (0097-4). The only change is some
minor improvements to the pki-ipa-retrieve-key Python program.
Cheers,
Fraser
On Tue, Apr 19, 2016 at 07:32:16PM +1000, Fraser Tweedale wrote:
Both issues addressed in latest patchset. Two new patches in the
mix; the order is:
0095-4, 0098, 0099, 0096-4, 0097-3 (tip)
I also added another attribute to schema for the authority
certificate serial number. It is not used in current code but I
have a hunch it may be needed for renewal, so I'm adding it now.
Thanks,
Fraser
On Thu, Apr 14, 2016 at 05:34:45PM -0400, Ade Lee wrote:
> Couple of points on 96/97.
>
> 1. First off, I'm not sure you followed my concern about being able to
> distinguish between CA instances.
>
> On an IPA system, this is not an issue because there is only one CA on
> the server. In this case, I imagine there will be a well known
> directory which custodia would work with.
>
> In general though, we have to imagine that someone could end up
> installing two different dogtag ca instances on the same server.
> CMS.getEEHost() would result in the same value (the hostname) for both
> CAs. How does your helper program (or custodia) know which key to
> retrieve?
>
> The way to distinguish Dogtag instances is host AND port.
>
> 2. So, we're very careful that the signing keys are never in memory in
> the server. All accesses to the system certs are through JSS/NSS which
> essentially provides us handles to the keys.
>
> Now, I see a case where we import PKCS12 data AND the password into
> memory, so that we can import it into NSS? Say it ain't so ..
>
> With custodia, we have a secure mechanism of transferring the keys from
> one server to another. It makes more sense to me to have the server
> kick off the custodia transfer and then have that process also import
> into the NSS db. The server would then need to await status from the
> custodia/retriever process - and then initialize the signing unit from
> the NSS DB. Or am I completely confused?
>
> Ade
>
>
>
> On Thu, 2016-04-14 at 16:35 -0400, Ade Lee wrote:
> > Still reviewing .. ACK on 87-95 (inclusive).
> >
> > On Thu, 2016-04-14 at 16:18 +1000, Fraser Tweedale wrote:
> > > On Thu, Apr 14, 2016 at 09:04:31AM +1000, Fraser Tweedale wrote:
> > > > On Wed, Apr 13, 2016 at 05:26:44PM -0400, Ade Lee wrote:
> > > > > Still reviewing ..
> > > > >
> > > > > See comment on 87. ACK on 88,89,90,91,92,93, 94, 95.
> > > > >
> > > > > Ade
> > > > >
> > > > > On Mon, 2016-04-11 at 12:32 +1000, Fraser Tweedale wrote:
> > > > > > Thanks for review, Ade. Comments to specific feedback
> > > > > > inline.
> > > > > > Rebased and updated patches attached. The substantive
> > > > > > changes
> > > > > > are:
> > > > > >
> > > > > > - KeyRetriever implementations are now required NOT to
import
> > > > > > the
> > > > > > key themselves. Instead the API is updated with
> > > > > > KeyRetriever.retrieveKey returning a Result, which
contains
> > > > > > PKCS
> > > > > > #12 data and password for same.
> > > > > >
> > > > > > - KeyRetrieverRunner reads the Result and imports the PKCS
> > > > > > #12
> > > > > > into
> > > > > > NSSDB.
> > > > > >
> > > > > > - Added new patch 0097 which provides the
> > > > > > IPACustodiaKeyRetriever
> > > > > > and assoicated Python helper script. It depends on an
> > > > > > unmerged
> > > > > > FreeIPA patch[1] as well as a particular principal and
> > > > > > associated
> > > > > > keytab and Custodia keys existing. I'm working on
FreeIPA
> > > > > > updates
> > > > > > to satisfy these requirements automatically on install
or
> > > > > > upgrade
> > > > > > but if you want to test this patch LMK and I'll
provide
> > > > > > detailed
> > > > > > instructions.
> > > > > >
> > > > > > [1]
> > > > > >
https://www.redhat.com/archives/freeipa-devel/2016-April/msg0
> > > > > > 00
> > > > > > 55.html
> > > > > >
> > > > > > Other comments inline.
> > > > > >
> > > > > > Cheers,
> > > > > > Fraser
> > > > > >
> > > > > > On Fri, Apr 08, 2016 at 11:16:19AM -0400, Ade Lee wrote:
> > > > > > >
> > > > > > > 0087
> > > > > > >
> > > > > > > 1. In SigningUnit.java -- you catch an ObjectNotFound
> > > > > > > exception and
> > > > > > > rethrow that as a CAMissingKey exception. Is that the
only
> > > > > > > way the
> > > > > > > ObjectNotFound exception can be thrown? What if the
key is
> > > > > > > present
> > > > > > > but
> > > > > > > the cert is not? Can we refactor here to ensure that
the
> > > > > > > correct
> > > > > > > exception is thrown?
> > > > > > >
> > > > > > One can't get additional info out of ObjectNotFound
without
> > > > > > inspecting the String message, which I'm not
comfortable
> > > > > > doing.
> > > > > > The
> > > > > > key retrieval system should import key and cert at same
time
> > > > > > so
> > > > > > I've
> > > > > > renamed the exception to CAMissingKeyOrCert for clarity.
> > > > > >
> > > > >
> > > > > Well, you can always nest exceptions like so :
> > > > >
> > > > > mToken.login(cb); // ONE_TIME by default.
> > > > >
> > > > > try {
> > > > > mCert = mManager.findCertByNickname(mNickname);
> > > > > CMS.debug("Found cert by nickname:
'" +
> > > > > mNickname
> > > > > + "' with serial number: " +
mCert.getSerialNumber());
> > > > >
> > > > > mCertImpl = new
> > > > > X509CertImpl(mCert.getEncoded());
> > > > > CMS.debug("converted to
x509CertImpl");
> > > > > } catch (ObjectNotFoundException e) {
> > > > > throw new CAMissingCertException();
> > > > > }
> > > > >
> > > > > try {
> > > > > mPrivk = mManager.findPrivKeyByCert(mCert);
> > > > > CMS.debug("Got private key from
cert");
> > > > > } catch (ObjectNotFoundException e) {
> > > > > throw new CAMissingKeyException();
> > > > > }
> > > > > ....
> > > > >
> > > > > The only reason that I suggest this is that I could imagine
> > > > > this
> > > > > kind
> > > > > of differentiation being useful in debugging failed custodia
> > > > > replications. If you think otherwise, I'm prepare to be
> > > > > convinced
> > > > > otherwise.
> > > > >
> > > > I think a scenario where we get key but not cert, or vice versa,
> > > > is
> > > > unlikely (Custodia gives us a PKCS #12 file with both). However,
> > > > your suggestion should work and it is a relatively small change.
> > > > I'll cut a new patchset with this change today, along with the
> > > > rebase.
> > > >
> > > Updated and rebased patches attached.
> > >
> > > The suggested changes were made to 0087. This also resulted in
> > > changes to patch 0094 (indicate when CA does not yet have keys).
> > >
> > > No substantive changes to any other patches.
> > >
> > > Cheers,
From f5322fdf885196275a93696cc31a489c9b4aab1d Mon Sep 17 00:00:00
2001
From: Fraser Tweedale <ftweedal(a)redhat.com>
Date: Wed, 30 Mar 2016 16:06:25 +1100
Subject: [PATCH] Lightweight CAs: authority schema changes
Add the 'authorityKeyHost' attribute which will contain names of
hosts that possess the authority's signing keys.
Add the 'authoritySerial' attribute which may contain the serial
number of the certificate most recently issued for the authority.
Change other attributes to be single-valued.
Part of: https://fedorahosted.org/pki/ticket/1625
---
base/server/share/conf/schema-authority.ldif | 16 +++++++++-------
base/server/share/conf/schema.ldif | 15 ++++++++-------
2 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/base/server/share/conf/schema-authority.ldif
b/base/server/share/conf/schema-authority.ldif
index 7d261f18fbc9475983bf93b1cddcc184d7f9d178..fd3c4fa225b036142a9aa4e99c65697365160dfd
100644
--- a/base/server/share/conf/schema-authority.ldif
+++ b/base/server/share/conf/schema-authority.ldif
@@ -1,8 +1,10 @@
dn: cn=schema
-attributeTypes: ( authorityID-oid NAME 'authorityID' DESC 'Authority ID'
SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 X-ORIGIN 'user defined' )
-attributeTypes: ( authorityKeyNickname-oid NAME 'authorityKeyNickname' DESC
'Authority key nickname' SYNTAX 1.3.6.1.4.1.1466.115.121.1.44 X-ORIGIN
'user-defined' )
-attributeTypes: ( authorityParentID-oid NAME 'authorityParentID' DESC
'Authority Parent ID' SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 X-ORIGIN 'user
defined' )
-attributeTypes: ( authorityEnabled-oid NAME 'authorityEnabled' DESC
'Authority Enabled' SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 X-ORIGIN 'user
defined' )
-attributeTypes: ( authorityDN-oid NAME 'authorityDN' DESC 'Authority
Enabled' SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 X-ORIGIN 'user defined' )
-attributeTypes: ( authorityParentDN-oid NAME 'authorityParentDN' DESC
'Authority Enabled' SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 X-ORIGIN 'user
defined' )
-objectClasses: ( authority-oid NAME 'authority' DESC 'Certificate
Authority' SUP top STRUCTURAL MUST ( cn $ authorityID $ authorityKeyNickname $
authorityEnabled $ authorityDN ) MAY ( authorityParentID $ authorityParentDN $ description
) X-ORIGIN 'user defined' )
+attributeTypes: ( authorityID-oid NAME 'authorityID' DESC 'Authority ID'
SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 SINGLE-VALUE X-ORIGIN 'user defined' )
+attributeTypes: ( authorityKeyNickname-oid NAME 'authorityKeyNickname' DESC
'Authority key nickname' SYNTAX 1.3.6.1.4.1.1466.115.121.1.44 SINGLE-VALUE
X-ORIGIN 'user-defined' )
+attributeTypes: ( authorityParentID-oid NAME 'authorityParentID' DESC
'Authority Parent ID' SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 SINGLE-VALUE X-ORIGIN
'user defined' )
+attributeTypes: ( authorityEnabled-oid NAME 'authorityEnabled' DESC
'Authority Enabled' SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 SINGLE-VALUE X-ORIGIN
'user defined' )
+attributeTypes: ( authorityDN-oid NAME 'authorityDN' DESC 'Authority DN'
SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 SINGLE-VALUE X-ORIGIN 'user defined' )
+attributeTypes: ( authoritySerial-oid NAME 'authoritySerial' DESC 'Authority
certificate serial number' SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN
'user defined' )
+attributeTypes: ( authorityParentDN-oid NAME 'authorityParentDN' DESC
'Authority Parent DN' SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 SINGLE-VALUE X-ORIGIN
'user defined' )
+attributeTypes: ( authorityKeyHost-oid NAME 'authorityKeyHost' DESC
'Authority Key Hosts' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'user
defined' )
+objectClasses: ( authority-oid NAME 'authority' DESC 'Certificate
Authority' SUP top STRUCTURAL MUST ( cn $ authorityID $ authorityKeyNickname $
authorityEnabled $ authorityDN ) MAY ( authoritySerial $ authorityParentID $
authorityParentDN $ authorityKeyHost $ description ) X-ORIGIN 'user defined' )
diff --git a/base/server/share/conf/schema.ldif b/base/server/share/conf/schema.ldif
index a15601ae7a362635bc398b92b9bfda1c72f0dfc8..5e4118d328ebe1fcac2743b3f51fb5ca9d57f9eb
100644
--- a/base/server/share/conf/schema.ldif
+++ b/base/server/share/conf/schema.ldif
@@ -671,12 +671,13 @@ objectClasses: ( certProfile-oid NAME 'certProfile' DESC
'Certificate profile' S
dn: cn=schema
changetype: modify
add: attributeTypes
-attributeTypes: ( authorityID-oid NAME 'authorityID' DESC 'Authority ID'
SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 X-ORIGIN 'user defined' )
-attributeTypes: ( authorityKeyNickname-oid NAME 'authorityKeyNickname' DESC
'Authority key nickname' SYNTAX 1.3.6.1.4.1.1466.115.121.1.44 X-ORIGIN
'user-defined' )
-attributeTypes: ( authorityParentID-oid NAME 'authorityParentID' DESC
'Authority Parent ID' SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 X-ORIGIN 'user
defined' )
-attributeTypes: ( authorityEnabled-oid NAME 'authorityEnabled' DESC
'Authority Enabled' SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 X-ORIGIN 'user
defined' )
-attributeTypes: ( authorityDN-oid NAME 'authorityDN' DESC 'Authority
Enabled' SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 X-ORIGIN 'user defined' )
-attributeTypes: ( authorityParentDN-oid NAME 'authorityParentDN' DESC
'Authority Enabled' SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 X-ORIGIN 'user
defined' )
+attributeTypes: ( authorityID-oid NAME 'authorityID' DESC 'Authority ID'
SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 SINGLE-VALUE X-ORIGIN 'user defined' )
+attributeTypes: ( authorityKeyNickname-oid NAME 'authorityKeyNickname' DESC
'Authority key nickname' SYNTAX 1.3.6.1.4.1.1466.115.121.1.44 SINGLE-VALUE
X-ORIGIN 'user-defined' )
+attributeTypes: ( authorityParentID-oid NAME 'authorityParentID' DESC
'Authority Parent ID' SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 SINGLE-VALUE X-ORIGIN
'user defined' )
+attributeTypes: ( authorityEnabled-oid NAME 'authorityEnabled' DESC
'Authority Enabled' SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 SINGLE-VALUE X-ORIGIN
'user defined' )
+attributeTypes: ( authorityDN-oid NAME 'authorityDN' DESC 'Authority DN'
SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 SINGLE-VALUE X-ORIGIN 'user defined' )
+attributeTypes: ( authorityParentDN-oid NAME 'authorityParentDN' DESC
'Authority Parent DN' SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 SINGLE-VALUE X-ORIGIN
'user defined' )
+attributeTypes: ( authorityKeyHost-oid NAME 'authorityKeyHost' DESC
'Authority Key Hosts' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'user
defined' )
-
add: objectClasses
-objectClasses: ( authority-oid NAME 'authority' DESC 'Certificate
Authority' SUP top STRUCTURAL MUST ( cn $ authorityID $ authorityKeyNickname $
authorityEnabled $ authorityDN ) MAY ( authorityParentID $ authorityParentDN $ description
) X-ORIGIN 'user defined' )
+objectClasses: ( authority-oid NAME 'authority' DESC 'Certificate
Authority' SUP top STRUCTURAL MUST ( cn $ authorityID $ authorityKeyNickname $
authorityEnabled $ authorityDN ) MAY ( authorityParentID $ authorityParentDN $
authorityKeyHost $ description ) X-ORIGIN 'user defined' )
--
2.5.5
From 3d42d650abf5b4862570620ef7b130bb511bb17d Mon Sep 17 00:00:00
2001
From: Fraser Tweedale <ftweedal(a)redhat.com>
Date: Tue, 19 Apr 2016 13:25:12 +1000
Subject: [PATCH 98/99] Add method CryptoUtil.importPKIArchiveOptions
Add the method CryptoUtil.importPKIArchiveOptions for importing a
wrapped key from a PKIArchiveOptions object. Also add another
variant of the createPKIArchiveOptions method, with a narrower API.
Part of: https://fedorahosted.org/pki/ticket/1625
---
.../com/netscape/cmsutil/crypto/CryptoUtil.java | 58 ++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
b/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
index 06caa0242ab192c5bbc14845dd7abc772601bd58..7ac3e4e9ad2702e09c704638b6ab773bb2dccb49
100644
--- a/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
+++ b/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
@@ -80,7 +80,9 @@ import org.mozilla.jss.CryptoManager;
import org.mozilla.jss.CryptoManager.NotInitializedException;
import org.mozilla.jss.NoSuchTokenException;
import org.mozilla.jss.SecretDecoderRing.KeyManager;
+import org.mozilla.jss.asn1.ANY;
import org.mozilla.jss.asn1.ASN1Util;
+import org.mozilla.jss.asn1.ASN1Value;
import org.mozilla.jss.asn1.BIT_STRING;
import org.mozilla.jss.asn1.InvalidBERException;
import org.mozilla.jss.asn1.OBJECT_IDENTIFIER;
@@ -1971,6 +1973,31 @@ public class CryptoUtil {
// wrap session key using transport key
byte[] session_data = wrapSymmetricKey(manager, token, transportCert, sk);
+ return createPKIArchiveOptions(IV, session_data, key_data);
+ }
+
+ public static byte[] createPKIArchiveOptions(
+ CryptoToken token, PublicKey wrappingKey, PrivateKey toBeWrapped,
+ KeyGenAlgorithm keyGenAlg, int symKeySize, IVParameterSpec IV)
+ throws TokenException, NoSuchAlgorithmException,
+ InvalidAlgorithmParameterException, InvalidKeyException,
+ IOException, InvalidBERException {
+ SymmetricKey sessionKey = CryptoUtil.generateKey(token, keyGenAlg, symKeySize);
+
+ KeyWrapper wrapper = token.getKeyWrapper(KeyWrapAlgorithm.DES3_CBC_PAD);
+ wrapper.initWrap(sessionKey, IV);
+ byte[] key_data = wrapper.wrap(toBeWrapped);
+
+ wrapper = token.getKeyWrapper(KeyWrapAlgorithm.RSA);
+ wrapper.initWrap(wrappingKey, null);
+ byte session_data[] = wrapper.wrap(sessionKey);
+
+ return createPKIArchiveOptions(IV, session_data, key_data);
+ }
+
+ private static byte[] createPKIArchiveOptions(
+ IVParameterSpec IV, byte[] session_data, byte[] key_data)
+ throws IOException, InvalidBERException {
// create PKIArchiveOptions structure
AlgorithmIdentifier algS = new AlgorithmIdentifier(new
OBJECT_IDENTIFIER("1.2.840.113549.3.7"),
new OCTET_STRING(IV.getIV()));
@@ -1996,6 +2023,37 @@ public class CryptoUtil {
return encoded;
}
+ public static PrivateKey importPKIArchiveOptions(
+ CryptoToken token, PrivateKey unwrappingKey,
+ PublicKey pubkey, byte[] data)
+ throws InvalidBERException, Exception {
+ ByteArrayInputStream in = new ByteArrayInputStream(data);
+ PKIArchiveOptions options = (PKIArchiveOptions)
+ (new PKIArchiveOptions.Template()).decode(in);
+ EncryptedKey encKey = options.getEncryptedKey();
+ EncryptedValue encVal = encKey.getEncryptedValue();
+ AlgorithmIdentifier algId = encVal.getSymmAlg();
+ BIT_STRING encSymKey = encVal.getEncSymmKey();
+ BIT_STRING encPrivKey = encVal.getEncValue();
+
+ KeyWrapper wrapper = token.getKeyWrapper(KeyWrapAlgorithm.RSA);
+ wrapper.initUnwrap(unwrappingKey, null);
+ SymmetricKey sk = wrapper.unwrapSymmetric(
+ encSymKey.getBits(), SymmetricKey.Type.DES3, 0);
+
+ ASN1Value v = algId.getParameters();
+ v = ((ANY) v).decodeWith(new OCTET_STRING.Template());
+ byte iv[] = ((OCTET_STRING) v).toByteArray();
+ IVParameterSpec ivps = new IVParameterSpec(iv);
+
+ wrapper = token.getKeyWrapper(KeyWrapAlgorithm.DES3_CBC_PAD);
+ wrapper.initUnwrap(sk, ivps);
+ PrivateKey.Type keyType = pubkey.getAlgorithm().equals("EC")
+ ? PrivateKey.Type.EC
+ : PrivateKey.Type.RSA;
+ return wrapper.unwrapPrivate(encPrivKey.getBits(), keyType, pubkey);
+ }
+
public static boolean sharedSecretExists(String nickname) throws
NotInitializedException, TokenException {
CryptoManager cm = CryptoManager.getInstance();
CryptoToken token = cm.getInternalKeyStorageToken();
--
2.5.5
From ba32660ee26fc00132882809ce9365690f375c6c Mon Sep 17 00:00:00
2001
From: Fraser Tweedale <ftweedal(a)redhat.com>
Date: Tue, 19 Apr 2016 13:28:56 +1000
Subject: [PATCH 99/99] Add ca-authority-key-export command
Add the 'pki ca-authority-key-export' CLI command for exporting a
PKIArchiveOptions object containing a nominated target key, wrapped
by a nominated wrapping key. This command is to be used by Custodia
to export key data for transmission to a requesting clone.
Part of: https://fedorahosted.org/pki/ticket/1625
---
.../netscape/cmstools/authority/AuthorityCLI.java | 1 +
.../cmstools/authority/AuthorityKeyExportCLI.java | 109 +++++++++++++++++++++
2 files changed, 110 insertions(+)
create mode 100644
base/java-tools/src/com/netscape/cmstools/authority/AuthorityKeyExportCLI.java
diff --git a/base/java-tools/src/com/netscape/cmstools/authority/AuthorityCLI.java
b/base/java-tools/src/com/netscape/cmstools/authority/AuthorityCLI.java
index ac06ea24ce824ad1b4be29a4176658caa9302e89..f42660d6727059bc76ab7ccd0bd0b22a87bc5f9a
100644
--- a/base/java-tools/src/com/netscape/cmstools/authority/AuthorityCLI.java
+++ b/base/java-tools/src/com/netscape/cmstools/authority/AuthorityCLI.java
@@ -18,6 +18,7 @@ public class AuthorityCLI extends CLI {
addModule(new AuthorityDisableCLI(this));
addModule(new AuthorityEnableCLI(this));
addModule(new AuthorityRemoveCLI(this));
+ addModule(new AuthorityKeyExportCLI(this));
}
public String getFullName() {
diff --git
a/base/java-tools/src/com/netscape/cmstools/authority/AuthorityKeyExportCLI.java
b/base/java-tools/src/com/netscape/cmstools/authority/AuthorityKeyExportCLI.java
new file mode 100644
index 0000000000000000000000000000000000000000..a3dee82c8d7ef3ad923aa53635b0825f3d272998
--- /dev/null
+++ b/base/java-tools/src/com/netscape/cmstools/authority/AuthorityKeyExportCLI.java
@@ -0,0 +1,109 @@
+package com.netscape.cmstools.authority;
+
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.security.PublicKey;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.ParseException;
+
+import org.mozilla.jss.CryptoManager;
+import org.mozilla.jss.crypto.CryptoToken;
+import org.mozilla.jss.crypto.IVParameterSpec;
+import org.mozilla.jss.crypto.KeyGenAlgorithm;
+import org.mozilla.jss.crypto.PrivateKey;
+import org.mozilla.jss.crypto.X509Certificate;
+
+import com.netscape.cmstools.cli.CLI;
+import com.netscape.cmsutil.crypto.CryptoUtil;
+
+public class AuthorityKeyExportCLI extends CLI {
+
+ public AuthorityCLI authorityCLI;
+
+ public AuthorityKeyExportCLI(AuthorityCLI authorityCLI) {
+ super("key-export", "Export wrapped CA signing key",
authorityCLI);
+ this.authorityCLI = authorityCLI;
+
+ options.addOption(null, "help", false, "Show usage");
+
+ Option option = new Option("o", "output", true, "Output
file");
+ option.setArgName("filename");
+ options.addOption(option);
+
+ option = new Option(null, "wrap-nickname", true, "Nickname of
wrapping key");
+ option.setArgName("nickname");
+ options.addOption(option);
+
+ option = new Option(null, "target-nickname", true, "Nickname of
target key");
+ option.setArgName("nickname");
+ options.addOption(option);
+ }
+
+ public void printHelp() {
+ formatter.printHelp(getFullName() + "--wrap-nickname NICKNAME
--target-nickname NICKNAME -o FILENAME", options);
+ }
+
+ public void execute(String[] args) throws Exception {
+ CommandLine cmd = null;
+
+ try {
+ cmd = parser.parse(options, args);
+ } catch (ParseException e) {
+ System.err.println("Error: " + e.getMessage());
+ printHelp();
+ System.exit(-1);
+ }
+
+ if (cmd.hasOption("help")) {
+ // Display usage
+ printHelp();
+ System.exit(0);
+ }
+
+ String filename = cmd.getOptionValue("output");
+ if (filename == null) {
+ System.err.println("Error: No output file specified.");
+ printHelp();
+ System.exit(-1);
+ }
+
+ String wrapNick = cmd.getOptionValue("wrap-nickname");
+ if (wrapNick == null) {
+ System.err.println("Error: no wrapping key nickname specified.");
+ printHelp();
+ System.exit(-1);
+ }
+
+ String targetNick = cmd.getOptionValue("target-nickname");
+ if (targetNick == null) {
+ System.err.println("Error: no target key nickname specified.");
+ printHelp();
+ System.exit(-1);
+ }
+
+ try {
+ CryptoManager cm = CryptoManager.getInstance();
+ X509Certificate wrapCert = cm.findCertByNickname(wrapNick);
+ X509Certificate targetCert = cm.findCertByNickname(targetNick);
+
+ PublicKey wrappingKey = wrapCert.getPublicKey();
+ PrivateKey toBeWrapped = cm.findPrivKeyByCert(targetCert);
+ CryptoToken token = cm.getInternalKeyStorageToken();
+
+ byte iv[] = { 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1 };
+ IVParameterSpec ivps = new IVParameterSpec(iv);
+
+ byte[] data = CryptoUtil.createPKIArchiveOptions(
+ token, wrappingKey, toBeWrapped,
+ KeyGenAlgorithm.DES3, 0, ivps);
+
+ Files.newOutputStream(Paths.get(filename)).write(data);
+ } catch (Throwable e) {
+ e.printStackTrace();
+ System.exit(-1);
+ }
+
+ }
+}
--
2.5.5
From 4c8fdc1b11e052d469aee944e491cd8725b7a1e9 Mon Sep 17 00:00:00
2001
From: Fraser Tweedale <ftweedal(a)redhat.com>
Date: Wed, 30 Mar 2016 12:38:24 +1100
Subject: [PATCH] Lightweight CAs: add key retrieval framework
Add the framework for key retrieval when a lightweight CA is missing
its signing key. This includes all the bits for loading a
KeyRetriever implementation, initiating retrieval in a thread and
updating the record of which clones possess the key if retrieval was
successful.
It does not include a KeyRetriever implementation. A subsequent
commit will provide this.
Part of: https://fedorahosted.org/pki/ticket/1625
---
.../src/com/netscape/ca/CertificateAuthority.java | 162 ++++++++++++++++++++-
base/ca/src/com/netscape/ca/KeyRetriever.java | 56 +++++++
.../src/netscape/security/pkcs/PKCS12Util.java | 3 +
3 files changed, 215 insertions(+), 6 deletions(-)
create mode 100644 base/ca/src/com/netscape/ca/KeyRetriever.java
diff --git a/base/ca/src/com/netscape/ca/CertificateAuthority.java
b/base/ca/src/com/netscape/ca/CertificateAuthority.java
index 37f1e95fc97f3d21ec6dc379962e27b42fb5b074..ac45141662aea732389353814e5a7e7b3ba516a7
100644
--- a/base/ca/src/com/netscape/ca/CertificateAuthority.java
+++ b/base/ca/src/com/netscape/ca/CertificateAuthority.java
@@ -35,6 +35,7 @@ import java.security.cert.CertificateException;
import java.security.cert.CertificateParsingException;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
import java.util.Date;
import java.util.Enumeration;
@@ -62,8 +63,10 @@ import org.mozilla.jss.crypto.CryptoToken;
import org.mozilla.jss.crypto.KeyPairAlgorithm;
import org.mozilla.jss.crypto.KeyPairGenerator;
import org.mozilla.jss.crypto.NoSuchItemOnTokenException;
+import org.mozilla.jss.crypto.PrivateKey;
import org.mozilla.jss.crypto.SignatureAlgorithm;
import org.mozilla.jss.crypto.TokenException;
+import org.mozilla.jss.crypto.X509Certificate;
import org.mozilla.jss.pkix.cert.Extension;
import org.mozilla.jss.pkix.primitive.Name;
@@ -205,6 +208,7 @@ public class CertificateAuthority
protected AuthorityID authorityID = null;
protected AuthorityID authorityParentID = null;
protected String authorityDescription = null;
+ protected Collection<String> authorityKeyHosts = null;
protected boolean authorityEnabled = true;
private boolean hasKeys = false;
private ECAException signingUnitException = null;
@@ -340,6 +344,7 @@ public class CertificateAuthority
AuthorityID aid,
AuthorityID parentAID,
String signingKeyNickname,
+ Collection<String> authorityKeyHosts,
String authorityDescription,
boolean authorityEnabled
) throws EBaseException {
@@ -355,6 +360,7 @@ public class CertificateAuthority
this.authorityDescription = authorityDescription;
this.authorityEnabled = authorityEnabled;
mNickname = signingKeyNickname;
+ this.authorityKeyHosts = authorityKeyHosts;
init(hostCA.mOwner, hostCA.mConfig);
}
@@ -504,7 +510,7 @@ public class CertificateAuthority
// init signing unit & CA cert.
try {
- initSigUnit();
+ initSigUnit(/* retrieveKeys */ true);
// init default CA attributes like cert version, validity.
initDefCaAttrs();
} catch (EBaseException e) {
@@ -1446,7 +1452,7 @@ public class CertificateAuthority
/**
* init CA signing unit & cert chain.
*/
- private void initSigUnit()
+ private boolean initSigUnit(boolean retrieveKeys)
throws EBaseException {
try {
// init signing unit
@@ -1476,7 +1482,14 @@ public class CertificateAuthority
} catch (CAMissingCertException | CAMissingKeyException e) {
CMS.debug("CA signing key and cert not (yet) present in
NSSDB");
signingUnitException = e;
- return;
+ if (retrieveKeys == true) {
+ CMS.debug("Starting KeyRetrieverRunner thread");
+ new Thread(
+ new KeyRetrieverRunner(this),
+ "KeyRetrieverRunner-" + authorityID
+ ).start();
+ }
+ return false;
}
CMS.debug("CA signing unit inited");
@@ -1601,6 +1614,8 @@ public class CertificateAuthority
mNickname = mSigningUnit.getNickname();
CMS.debug("in init - got CA name " + mName);
+ return true;
+
} catch (CryptoManager.NotInitializedException e) {
log(ILogger.LL_FAILURE,
CMS.getLogMessage("CMSCORE_CA_CA_OCSP_SIGNING", e.toString()));
throw new
ECAException(CMS.getUserMessage("CMS_CA_CRYPTO_NOT_INITIALIZED"));
@@ -2527,11 +2542,14 @@ public class CertificateAuthority
throw new EBaseException("Failed to convert issuer DN to string: "
+ e);
}
+ String thisClone = CMS.getEEHost() + ":" + CMS.getEESSLPort();
+
LDAPAttribute[] attrs = {
new LDAPAttribute("objectclass", "authority"),
new LDAPAttribute("cn", aidString),
new LDAPAttribute("authorityID", aidString),
new LDAPAttribute("authorityKeyNickname", nickname),
+ new LDAPAttribute("authorityKeyHost", thisClone),
new LDAPAttribute("authorityEnabled", "TRUE"),
new LDAPAttribute("authorityDN", subjectDN),
new LDAPAttribute("authorityParentDN", parentDNString)
@@ -2612,7 +2630,9 @@ public class CertificateAuthority
return new CertificateAuthority(
hostCA, subjectX500Name,
- aid, this.authorityID, nickname, description, true);
+ aid, this.authorityID,
+ nickname, Collections.singleton(thisClone),
+ description, true);
}
/**
@@ -2785,6 +2805,23 @@ public class CertificateAuthority
}
}
+ /**
+ * Add this instance to the authorityKeyHosts
+ */
+ private void addInstanceToAuthorityKeyHosts() throws ELdapException {
+ String hostname = CMS.getEEHost();
+ if (authorityKeyHosts.contains(hostname)) {
+ // already there; nothing to do
+ return;
+ }
+ LDAPModificationSet mods = new LDAPModificationSet();
+ mods.add(
+ LDAPModification.ADD,
+ new LDAPAttribute("authorityKeyHost", hostname));
+ modifyAuthorityEntry(mods);
+ authorityKeyHosts.add(hostname);
+ }
+
public synchronized void deleteAuthority() throws EBaseException {
if (isHostAuthority())
throw new CATypeException("Cannot delete the host CA");
@@ -2933,7 +2970,6 @@ public class CertificateAuthority
case LDAPPersistSearchControl.ADD:
CMS.debug("authorityMonitor: ADD");
readAuthority(entry);
- // TODO kick off signing key replication via custodia
break;
case LDAPPersistSearchControl.DELETE:
CMS.debug("authorityMonitor: DELETE");
@@ -2990,6 +3026,7 @@ public class CertificateAuthority
LDAPAttribute aidAttr = entry.getAttribute("authorityID");
LDAPAttribute nickAttr = entry.getAttribute("authorityKeyNickname");
+ LDAPAttribute keyHostsAttr = entry.getAttribute("authorityKeyHost");
LDAPAttribute dnAttr = entry.getAttribute("authorityDN");
LDAPAttribute parentAIDAttr =
entry.getAttribute("authorityParentID");
LDAPAttribute parentDNAttr = entry.getAttribute("authorityParentDN");
@@ -3052,6 +3089,16 @@ public class CertificateAuthority
}
String keyNick = (String) nickAttr.getStringValues().nextElement();
+
+ Collection<String> keyHosts;
+ if (keyHostsAttr == null) {
+ keyHosts = Collections.emptyList();
+ } else {
+ @SuppressWarnings("unchecked")
+ Enumeration<String> keyHostsEnum = keyHostsAttr.getStringValues();
+ keyHosts = Collections.list(keyHostsEnum);
+ }
+
AuthorityID parentAID = null;
if (parentAIDAttr != null)
parentAID = new AuthorityID((String)
@@ -3067,7 +3114,7 @@ public class CertificateAuthority
try {
CertificateAuthority ca = new CertificateAuthority(
- hostCA, dn, aid, parentAID, keyNick, desc, enabled);
+ hostCA, dn, aid, parentAID, keyNick, keyHosts, desc, enabled);
caMap.put(aid, ca);
entryUSNs.put(aid, newEntryUSN);
nsUniqueIds.put(aid, nsUniqueId);
@@ -3117,4 +3164,107 @@ public class CertificateAuthority
}
}
+ private class KeyRetrieverRunner implements Runnable {
+ private CertificateAuthority ca;
+
+ public KeyRetrieverRunner(CertificateAuthority ca) {
+ this.ca = ca;
+ }
+
+ public void run() {
+ String KR_CLASS_KEY = "features.authority.keyRetrieverClass";
+ String className = null;
+ try {
+ className = CMS.getConfigStore().getString(KR_CLASS_KEY);
+ } catch (EBaseException e) {
+ CMS.debug("Unable to read key retriever class from CS.cfg: " +
e);
+ return;
+ }
+
+ KeyRetriever kr = null;
+ try {
+ kr = Class.forName(className)
+ .asSubclass(KeyRetriever.class)
+ .newInstance();
+ } catch (ClassNotFoundException e) {
+ CMS.debug("Could not find class: " + className);
+ CMS.debug(e);
+ return;
+ } catch (ClassCastException e) {
+ CMS.debug("Class is not an instance of KeyRetriever: " +
className);
+ CMS.debug(e);
+ return;
+ } catch (InstantiationException | IllegalAccessException e) {
+ CMS.debug("Could not instantiate class: " + className);
+ CMS.debug(e);
+ return;
+ }
+
+ KeyRetriever.Result krr = null;
+ try {
+ krr = kr.retrieveKey(ca.mNickname, ca.authorityKeyHosts);
+ } catch (Throwable e) {
+ CMS.debug("Caught exception during execution of
KeyRetriever.retrieveKey");
+ CMS.debug(e);
+ return;
+ }
+
+ if (krr == null) {
+ CMS.debug("KeyRetriever did not return a result.");
+ return;
+ }
+
+ CMS.debug("Importing key and cert");
+ byte[] certBytes = krr.getCertificate();
+ byte[] paoData = krr.getPKIArchiveOptions();
+ try {
+ CryptoManager manager = CryptoManager.getInstance();
+ CryptoToken token = manager.getInternalKeyStorageToken();
+
+ X509Certificate cert = manager.importCACertPackage(certBytes);
+ PublicKey pubkey = cert.getPublicKey();
+ token.getCryptoStore().deleteCert(cert);
+
+ PrivateKey unwrappingKey = hostCA.mSigningUnit.getPrivateKey();
+
+ CryptoUtil.importPKIArchiveOptions(
+ token, unwrappingKey, pubkey, paoData);
+
+ cert = manager.importUserCACertPackage(certBytes, ca.mNickname);
+ } catch (Throwable e) {
+ CMS.debug("Caught exception during cert/key import");
+ CMS.debug(e);
+ return;
+ }
+
+ boolean initSigUnitSucceeded = false;
+ try {
+ CMS.debug("Reinitialising SigningUnit");
+ // re-init signing unit, but avoid triggering
+ // key replication if initialisation fails again
+ // for some reason
+ //
+ initSigUnitSucceeded = ca.initSigUnit(/* retrieveKeys */ false);
+ } catch (Throwable e) {
+ CMS.debug("Caught exception during SigningUnit re-init");
+ CMS.debug(e);
+ return;
+ }
+
+ if (!initSigUnitSucceeded) {
+ CMS.debug("Failed to re-init SigningUnit");
+ return;
+ }
+
+ CMS.debug("Adding self to authorityKeyHosts attribute");
+ try {
+ ca.addInstanceToAuthorityKeyHosts();
+ } catch (Throwable e) {
+ CMS.debug("Failed to add self to authorityKeyHosts");
+ CMS.debug(e);
+ return;
+ }
+ }
+ }
+
}
diff --git a/base/ca/src/com/netscape/ca/KeyRetriever.java
b/base/ca/src/com/netscape/ca/KeyRetriever.java
new file mode 100644
index 0000000000000000000000000000000000000000..7c0df0bf56578b81062de77de47aa516b5c9d949
--- /dev/null
+++ b/base/ca/src/com/netscape/ca/KeyRetriever.java
@@ -0,0 +1,56 @@
+// --- BEGIN COPYRIGHT BLOCK ---
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; version 2 of the License.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this program; if not, write to the Free Software Foundation, Inc.,
+// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+//
+// (C) 2016 Red Hat, Inc.
+// All rights reserved.
+// --- END COPYRIGHT BLOCK ---
+
+package com.netscape.ca;
+
+import java.util.Collection;
+
+public interface KeyRetriever {
+ /**
+ * Retrieve the specified signing key from specified clone and
+ * return to the KeyRetrieverRunner.
+ *
+ * A KeyRetriever MUST NOT import the cert and key to the NSSDB
+ * itself. It SHALL, if successful in retrieving the key and
+ * certificate, return a Result which contains a PEM-encoded
+ * X.509 certificate and a DER-encoded PKIArchiveOptions object
+ * containing an EncryptedValue of the target private key
+ * wrapped by the host authority's public key.
+ *
+ * Upon failure the KeyRetriever SHALL return null.
+ */
+ Result retrieveKey(String nickname, Collection<String> hostPorts);
+
+ class Result {
+ private byte[] certificate;
+ private byte[] pkiArchiveOptions;
+
+ public Result(byte[] certificate, byte[] pkiArchiveOptions) {
+ this.certificate = certificate;
+ this.pkiArchiveOptions = pkiArchiveOptions;
+ }
+
+ public byte[] getCertificate() {
+ return certificate;
+ }
+
+ public byte[] getPKIArchiveOptions() {
+ return pkiArchiveOptions;
+ }
+ }
+}
diff --git a/base/util/src/netscape/security/pkcs/PKCS12Util.java
b/base/util/src/netscape/security/pkcs/PKCS12Util.java
index 43435c822c9400248fe556bf066cd2659e18ae17..9931027dacfe88e636b694b7c490ffc6804068dd
100644
--- a/base/util/src/netscape/security/pkcs/PKCS12Util.java
+++ b/base/util/src/netscape/security/pkcs/PKCS12Util.java
@@ -536,7 +536,10 @@ public class PKCS12Util {
Path path = Paths.get(filename);
byte[] b = Files.readAllBytes(path);
+ return loadFromByteArray(b, password);
+ }
+ public PKCS12 loadFromByteArray(byte[] b, Password password) throws Exception {
ByteArrayInputStream bis = new ByteArrayInputStream(b);
PFX pfx = (PFX) (new PFX.Template()).decode(bis);
--
2.5.5
From d2e57290b2d3392082335955be72dfaf3111afd0 Mon Sep 17 00:00:00
2001
From: Fraser Tweedale <ftweedal(a)redhat.com>
Date: Fri, 8 Apr 2016 22:23:42 +1000
Subject: [PATCH] Lightweight CAs: add IPACustodiaKeyRetriever
Add 'IPACustodiaKeyRetriever', a 'KeyRetriever' implementation for
use when Dogtag is deployed as a FreeIPA CA. The Java class invokes
'pki-ipa-key-retriever', a Python script that retrieves lightweight
CA keys from the Custodia server on a replica that possesses the
keys. 'pki-ipa-key-retriever' depends on FreeIPA libraries, FreeIPA
server configuration, and Kerberos and Custodia keys owned by
'pkiuser'.
Part of: https://fedorahosted.org/pki/ticket/1625
---
base/ca/src/CMakeLists.txt | 9 ++-
.../com/netscape/ca/IPACustodiaKeyRetriever.java | 75 ++++++++++++++++++++++
base/server/CMakeLists.txt | 11 ++++
base/server/libexec/pki-ipa-retrieve-key | 42 ++++++++++++
specs/pki-core.spec | 1 +
5 files changed, 137 insertions(+), 1 deletion(-)
create mode 100644 base/ca/src/com/netscape/ca/IPACustodiaKeyRetriever.java
create mode 100755 base/server/libexec/pki-ipa-retrieve-key
diff --git a/base/ca/src/CMakeLists.txt b/base/ca/src/CMakeLists.txt
index 5b805e1b3a5eddb46d17178ca2ac204c36ae5680..1817dacfbacaeb2635db2550e32ff62c26d628ef
100644
--- a/base/ca/src/CMakeLists.txt
+++ b/base/ca/src/CMakeLists.txt
@@ -24,6 +24,13 @@ find_file(COMMONS_CODEC_JAR
/usr/share/java
)
+find_file(COMMONS_IO_JAR
+ NAMES
+ commons-io.jar
+ PATHS
+ /usr/share/java
+)
+
find_file(COMMONS_LANG_JAR
NAMES
commons-lang.jar
@@ -73,7 +80,7 @@ javac(pki-ca-classes
com/netscape/ca/*.java
org/dogtagpki/server/ca/*.java
CLASSPATH
- ${COMMONS_CODEC_JAR} ${COMMONS_LANG_JAR}
+ ${COMMONS_CODEC_JAR} ${COMMONS_IO_JAR} ${COMMONS_LANG_JAR}
${JSS_JAR} ${SYMKEY_JAR}
${LDAPJDK_JAR}
${SERVLET_JAR} ${TOMCAT_CATALINA_JAR}
diff --git a/base/ca/src/com/netscape/ca/IPACustodiaKeyRetriever.java
b/base/ca/src/com/netscape/ca/IPACustodiaKeyRetriever.java
new file mode 100644
index 0000000000000000000000000000000000000000..4a162d3702fccc19dfef792a5213c653286930f3
--- /dev/null
+++ b/base/ca/src/com/netscape/ca/IPACustodiaKeyRetriever.java
@@ -0,0 +1,75 @@
+// --- BEGIN COPYRIGHT BLOCK ---
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; version 2 of the License.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this program; if not, write to the Free Software Foundation, Inc.,
+// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+//
+// (C) 2016 Red Hat, Inc.
+// All rights reserved.
+// --- END COPYRIGHT BLOCK ---
+
+package com.netscape.ca;
+
+import java.lang.Process;
+import java.lang.ProcessBuilder;
+import java.util.Collection;
+import java.util.Stack;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang.ArrayUtils;
+
+import com.netscape.certsrv.apps.CMS;
+
+public class IPACustodiaKeyRetriever implements KeyRetriever {
+ public Result retrieveKey(String nickname, Collection<String> hostPorts) {
+ CMS.debug("Running IPACustodiaKeyRetriever");
+
+ Stack<String> command = new Stack<>();
+ command.push("/usr/libexec/pki-ipa-retrieve-key");
+ command.push(nickname);
+
+ for (String hostPort : hostPorts) {
+ String host = hostPort.split(":")[0];
+ command.push(host);
+ CMS.debug("About to execute command: " + command);
+ ProcessBuilder pb = new ProcessBuilder(command);
+ try {
+ Process p = pb.start();
+ int exitValue = p.waitFor();
+ if (exitValue != 0)
+ continue;
+
+ /* Custodia returns a PEM-encoded certificate and a
+ * base64-encoded PKIArchiveOptions containing the
+ * wrapped private key. These values are output by
+ * the Python 'pki-ipa-retrieve-key' program,
+ * separated by a null byte (password first)
+ */
+ byte[] output = IOUtils.toByteArray(p.getInputStream());
+ int splitIndex = ArrayUtils.indexOf(output, (byte) 0);
+ if (splitIndex == ArrayUtils.INDEX_NOT_FOUND) {
+ CMS.debug("Invalid output: null byte not found");
+ continue;
+ }
+ return new Result(
+ ArrayUtils.subarray(output, 0, splitIndex),
+ ArrayUtils.subarray(output, splitIndex + 1, output.length)
+ );
+ } catch (Throwable e) {
+ CMS.debug("Caught exception while executing command: " + e);
+ } finally {
+ command.pop();
+ }
+ }
+ CMS.debug("Failed to retrieve key from any host.");
+ return null;
+ }
+}
diff --git a/base/server/CMakeLists.txt b/base/server/CMakeLists.txt
index 5a6aea96a2317655fb454967f9f218020443bcb8..9e5b27833c8d023e63320c43d64ad64b0055c254
100644
--- a/base/server/CMakeLists.txt
+++ b/base/server/CMakeLists.txt
@@ -81,6 +81,17 @@ install(
install(
DIRECTORY
+ libexec/
+ DESTINATION
+ ${LIBEXEC_INSTALL_DIR}
+ FILE_PERMISSIONS
+ OWNER_EXECUTE OWNER_WRITE OWNER_READ
+ GROUP_EXECUTE GROUP_READ
+ WORLD_EXECUTE WORLD_READ
+)
+
+install(
+ DIRECTORY
upgrade
DESTINATION
${DATA_INSTALL_DIR}/server/
diff --git a/base/server/libexec/pki-ipa-retrieve-key
b/base/server/libexec/pki-ipa-retrieve-key
new file mode 100755
index 0000000000000000000000000000000000000000..9305150a446bb4deed339b89daad821a19b9e7df
--- /dev/null
+++ b/base/server/libexec/pki-ipa-retrieve-key
@@ -0,0 +1,42 @@
+#!/usr/bin/python
+
+from __future__ import print_function
+
+import ConfigParser
+import base64
+import sys
+
+from jwcrypto.common import json_decode
+
+from ipaplatform.paths import paths
+from ipapython.secrets.client import CustodiaClient
+
+conf = ConfigParser.ConfigParser()
+conf.read(paths.IPA_DEFAULT_CONF)
+hostname = conf.get('global', 'host')
+realm = conf.get('global', 'realm')
+
+keyname = "ca_wrapped/" + sys.argv[1]
+servername = sys.argv[2]
+
+client_keyfile = "/etc/pki/pki-tomcat/dogtag-ipa-custodia.keys"
+client_keytab = "/etc/pki/pki-tomcat/dogtag-ipa-custodia.keytab"
+
+client = CustodiaClient(
+ client=hostname, server=servername, realm=realm,
+ ldap_uri="ldaps://" + hostname,
+ client_servicename='dogtag-ipa-custodia',
+ keyfile=client_keyfile, keytab=client_keytab,
+ )
+
+result_json = client.fetch_key(keyname, store=False)
+result = json_decode(result_json)
+certificate = result["certificate"]
+wrapped_key = base64.b64decode(result["wrapped_key"])
+
+# Custodia returns a PEM-encoded certificate and a base64-encoded
+# DER PKIArchiveOptions object. Output these values, separated by a
+# null byte (certificate first), to be read by the Java
+# IPACustodiaKeyRetriever that invoked this program.
+
+print(certificate, wrapped_key, sep='\0', end='')
diff --git a/specs/pki-core.spec b/specs/pki-core.spec
index bce6bd2d298bc8e1ad5cb40618f982b5ba23b27d..509ecdafa4e5d0e651ce22497db06420abcdf259
100644
--- a/specs/pki-core.spec
+++ b/specs/pki-core.spec
@@ -1007,6 +1007,7 @@ systemctl daemon-reload
%{_sbindir}/pki-server
%{_sbindir}/pki-server-nuxwdog
%{_sbindir}/pki-server-upgrade
+%{_libexecdir}/pki-ipa-retrieve-key
%{python2_sitelib}/pki/server/
%dir %{_datadir}/pki/deployment
%{_datadir}/pki/deployment/config/
--
2.5.5
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
10:30 a.m.
Comments:
95 - ack
96 -
1. You have made the return type of initSigUnit() to be boolean.
Should you be checking the return value in init()?
2. In addInstanceToAuthorityKeyHosts(), you are still using only the
hostname. Should be host:port
3. The logic in the KeyRetrieverRunner class looks OK to me, but I'd
like cfu and/or jmagne to check it and make sure we are calling the
right primitives to wrap/unwrap inside the cryptographic token.
Also I'd like them to confirm that this would wor for an HSM.
Statements like the following make me question that:
CryptoToken token = manager.getInternalKeyStorageToken()
4. Can you explain what happens if for some reason the script fails to
retrieve the key? Do we end up retrying later and if so, when?
97- ACK
98 - ACK
On Wed, 2016-04-20 at 16:15 +1000, Fraser Tweedale wrote:
New version of 0097 attached (0097-4). The only change is some
minor improvements to the pki-ipa-retrieve-key Python program.
Cheers,
Fraser
On Tue, Apr 19, 2016 at 07:32:16PM +1000, Fraser Tweedale wrote:
> Both issues addressed in latest patchset. Two new patches in the
> mix; the order is:
>
> 0095-4, 0098, 0099, 0096-4, 0097-3 (tip)
>
> I also added another attribute to schema for the authority
> certificate serial number. It is not used in current code but I
> have a hunch it may be needed for renewal, so I'm adding it now.
>
> Thanks,
> Fraser
>
> On Thu, Apr 14, 2016 at 05:34:45PM -0400, Ade Lee wrote:
> > Couple of points on 96/97.
> >
> > 1. First off, I'm not sure you followed my concern about being
> > able to
> > distinguish between CA instances.
> >
> > On an IPA system, this is not an issue because there is only one
> > CA on
> > the server. In this case, I imagine there will be a well known
> > directory which custodia would work with.
> >
> > In general though, we have to imagine that someone could end up
> > installing two different dogtag ca instances on the same server.
> > CMS.getEEHost() would result in the same value (the hostname)
> > for both
> > CAs. How does your helper program (or custodia) know which key
> > to
> > retrieve?
> >
> > The way to distinguish Dogtag instances is host AND port.
> >
> > 2. So, we're very careful that the signing keys are never in
> > memory in
> > the server. All accesses to the system certs are through JSS/NSS
> > which
> > essentially provides us handles to the keys.
> >
> > Now, I see a case where we import PKCS12 data AND the password
> > into
> > memory, so that we can import it into NSS? Say it ain't so ..
> >
> > With custodia, we have a secure mechanism of transferring the
> > keys from
> > one server to another. It makes more sense to me to have the
> > server
> > kick off the custodia transfer and then have that process also
> > import
> > into the NSS db. The server would then need to await status from
> > the
> > custodia/retriever process - and then initialize the signing unit
> > from
> > the NSS DB. Or am I completely confused?
> >
> > Ade
> >
> >
> >
> > On Thu, 2016-04-14 at 16:35 -0400, Ade Lee wrote:
> > > Still reviewing .. ACK on 87-95 (inclusive).
> > >
> > > On Thu, 2016-04-14 at 16:18 +1000, Fraser Tweedale wrote:
> > > > On Thu, Apr 14, 2016 at 09:04:31AM +1000, Fraser Tweedale
> > > > wrote:
> > > > > On Wed, Apr 13, 2016 at 05:26:44PM -0400, Ade Lee wrote:
> > > > > > Still reviewing ..
> > > > > >
> > > > > > See comment on 87. ACK on 88,89,90,91,92,93, 94, 95.
> > > > > >
> > > > > > Ade
> > > > > >
> > > > > > On Mon, 2016-04-11 at 12:32 +1000, Fraser Tweedale wrote:
> > > > > > > Thanks for review, Ade. Comments to specific
feedback
> > > > > > > inline.
> > > > > > > Rebased and updated patches attached. The
substantive
> > > > > > > changes
> > > > > > > are:
> > > > > > >
> > > > > > > - KeyRetriever implementations are now required NOT
to
> > > > > > > import
> > > > > > > the
> > > > > > > key themselves. Instead the API is updated with
> > > > > > > KeyRetriever.retrieveKey returning a Result, which
> > > > > > > contains
> > > > > > > PKCS
> > > > > > > #12 data and password for same.
> > > > > > >
> > > > > > > - KeyRetrieverRunner reads the Result and imports the
> > > > > > > PKCS
> > > > > > > #12
> > > > > > > into
> > > > > > > NSSDB.
> > > > > > >
> > > > > > > - Added new patch 0097 which provides the
> > > > > > > IPACustodiaKeyRetriever
> > > > > > > and assoicated Python helper script. It depends on
> > > > > > > an
> > > > > > > unmerged
> > > > > > > FreeIPA patch[1] as well as a particular principal
> > > > > > > and
> > > > > > > associated
> > > > > > > keytab and Custodia keys existing. I'm working
on
> > > > > > > FreeIPA
> > > > > > > updates
> > > > > > > to satisfy these requirements automatically on
> > > > > > > install or
> > > > > > > upgrade
> > > > > > > but if you want to test this patch LMK and I'll
> > > > > > > provide
> > > > > > > detailed
> > > > > > > instructions.
> > > > > > >
> > > > > > > [1]
> > > > > > >
https://www.redhat.com/archives/freeipa-devel/2016-Apri
> > > > > > > l/msg0
> > > > > > > 00
> > > > > > > 55.html
> > > > > > >
> > > > > > > Other comments inline.
> > > > > > >
> > > > > > > Cheers,
> > > > > > > Fraser
> > > > > > >
> > > > > > > On Fri, Apr 08, 2016 at 11:16:19AM -0400, Ade Lee
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > 0087
> > > > > > > >
> > > > > > > > 1. In SigningUnit.java -- you catch an
ObjectNotFound
> > > > > > > > exception and
> > > > > > > > rethrow that as a CAMissingKey exception. Is
that
> > > > > > > > the only
> > > > > > > > way the
> > > > > > > > ObjectNotFound exception can be thrown? What if
the
> > > > > > > > key is
> > > > > > > > present
> > > > > > > > but
> > > > > > > > the cert is not? Can we refactor here to ensure
that
> > > > > > > > the
> > > > > > > > correct
> > > > > > > > exception is thrown?
> > > > > > > >
> > > > > > > One can't get additional info out of
ObjectNotFound
> > > > > > > without
> > > > > > > inspecting the String message, which I'm not
> > > > > > > comfortable
> > > > > > > doing.
> > > > > > > The
> > > > > > > key retrieval system should import key and cert at
same
> > > > > > > time
> > > > > > > so
> > > > > > > I've
> > > > > > > renamed the exception to CAMissingKeyOrCert for
> > > > > > > clarity.
> > > > > > >
> > > > > >
> > > > > > Well, you can always nest exceptions like so :
> > > > > >
> > > > > > mToken.login(cb); // ONE_TIME by default.
> > > > > >
> > > > > > try {
> > > > > > mCert =
> > > > > > mManager.findCertByNickname(mNickname);
> > > > > > CMS.debug("Found cert by nickname:
'" +
> > > > > > mNickname
> > > > > > + "' with serial number: " +
mCert.getSerialNumber());
> > > > > >
> > > > > > mCertImpl = new
> > > > > > X509CertImpl(mCert.getEncoded());
> > > > > > CMS.debug("converted to
x509CertImpl");
> > > > > > } catch (ObjectNotFoundException e) {
> > > > > > throw new CAMissingCertException();
> > > > > > }
> > > > > >
> > > > > > try {
> > > > > > mPrivk =
> > > > > > mManager.findPrivKeyByCert(mCert);
> > > > > > CMS.debug("Got private key from
cert");
> > > > > > } catch (ObjectNotFoundException e) {
> > > > > > throw new CAMissingKeyException();
> > > > > > }
> > > > > > ....
> > > > > >
> > > > > > The only reason that I suggest this is that I could
> > > > > > imagine
> > > > > > this
> > > > > > kind
> > > > > > of differentiation being useful in debugging failed
> > > > > > custodia
> > > > > > replications. If you think otherwise, I'm prepare to
be
> > > > > > convinced
> > > > > > otherwise.
> > > > > >
> > > > > I think a scenario where we get key but not cert, or vice
> > > > > versa,
> > > > > is
> > > > > unlikely (Custodia gives us a PKCS #12 file with both).
> > > > > However,
> > > > > your suggestion should work and it is a relatively small
> > > > > change.
> > > > > I'll cut a new patchset with this change today, along with
> > > > > the
> > > > > rebase.
> > > > >
> > > > Updated and rebased patches attached.
> > > >
> > > > The suggested changes were made to 0087. This also resulted
> > > > in
> > > > changes to patch 0094 (indicate when CA does not yet have
> > > > keys).
> > > >
> > > > No substantive changes to any other patches.
> > > >
> > > > Cheers,
> From f5322fdf885196275a93696cc31a489c9b4aab1d Mon Sep 17 00:00:00
> 2001
> From: Fraser Tweedale <ftweedal(a)redhat.com>
> Date: Wed, 30 Mar 2016 16:06:25 +1100
> Subject: [PATCH] Lightweight CAs: authority schema changes
>
> Add the 'authorityKeyHost' attribute which will contain names of
> hosts that possess the authority's signing keys.
>
> Add the 'authoritySerial' attribute which may contain the serial
> number of the certificate most recently issued for the authority.
>
> Change other attributes to be single-valued.
>
> Part of: https://fedorahosted.org/pki/ticket/1625
> ---
> base/server/share/conf/schema-authority.ldif | 16 +++++++++-------
> base/server/share/conf/schema.ldif | 15 ++++++++-------
> 2 files changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/base/server/share/conf/schema-authority.ldif
> b/base/server/share/conf/schema-authority.ldif
> index
> 7d261f18fbc9475983bf93b1cddcc184d7f9d178..fd3c4fa225b036142a9aa4e99
> c65697365160dfd 100644
> --- a/base/server/share/conf/schema-authority.ldif
> +++ b/base/server/share/conf/schema-authority.ldif
> @@ -1,8 +1,10 @@
> dn: cn=schema
> -attributeTypes: ( authorityID-oid NAME 'authorityID' DESC
> 'Authority ID' SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 X-ORIGIN 'user
> defined' )
> -attributeTypes: ( authorityKeyNickname-oid NAME
> 'authorityKeyNickname' DESC 'Authority key nickname' SYNTAX
> 1.3.6.1.4.1.1466.115.121.1.44 X-ORIGIN 'user-defined' )
> -attributeTypes: ( authorityParentID-oid NAME 'authorityParentID'
> DESC 'Authority Parent ID' SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 X
> -ORIGIN 'user defined' )
> -attributeTypes: ( authorityEnabled-oid NAME 'authorityEnabled'
> DESC 'Authority Enabled' SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 X
> -ORIGIN 'user defined' )
> -attributeTypes: ( authorityDN-oid NAME 'authorityDN' DESC
> 'Authority Enabled' SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 X-ORIGIN
> 'user defined' )
> -attributeTypes: ( authorityParentDN-oid NAME 'authorityParentDN'
> DESC 'Authority Enabled' SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 X
> -ORIGIN 'user defined' )
> -objectClasses: ( authority-oid NAME 'authority' DESC 'Certificate
> Authority' SUP top STRUCTURAL MUST ( cn $ authorityID $
> authorityKeyNickname $ authorityEnabled $ authorityDN ) MAY (
> authorityParentID $ authorityParentDN $ description ) X-ORIGIN
> 'user defined' )
> +attributeTypes: ( authorityID-oid NAME 'authorityID' DESC
> 'Authority ID' SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 SINGLE-VALUE X
> -ORIGIN 'user defined' )
> +attributeTypes: ( authorityKeyNickname-oid NAME
> 'authorityKeyNickname' DESC 'Authority key nickname' SYNTAX
> 1.3.6.1.4.1.1466.115.121.1.44 SINGLE-VALUE X-ORIGIN 'user-defined'
> )
> +attributeTypes: ( authorityParentID-oid NAME 'authorityParentID'
> DESC 'Authority Parent ID' SYNTAX 1.3.6.1.4.1.1466.115.121.1.40
> SINGLE-VALUE X-ORIGIN 'user defined' )
> +attributeTypes: ( authorityEnabled-oid NAME 'authorityEnabled'
> DESC 'Authority Enabled' SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 SINGLE
> -VALUE X-ORIGIN 'user defined' )
> +attributeTypes: ( authorityDN-oid NAME 'authorityDN' DESC
> 'Authority DN' SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 SINGLE-VALUE X
> -ORIGIN 'user defined' )
> +attributeTypes: ( authoritySerial-oid NAME 'authoritySerial' DESC
> 'Authority certificate serial number' SYNTAX
> 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'user defined'
> )
> +attributeTypes: ( authorityParentDN-oid NAME 'authorityParentDN'
> DESC 'Authority Parent DN' SYNTAX 1.3.6.1.4.1.1466.115.121.1.12
> SINGLE-VALUE X-ORIGIN 'user defined' )
> +attributeTypes: ( authorityKeyHost-oid NAME 'authorityKeyHost'
> DESC 'Authority Key Hosts' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X
> -ORIGIN 'user defined' )
> +objectClasses: ( authority-oid NAME 'authority' DESC 'Certificate
> Authority' SUP top STRUCTURAL MUST ( cn $ authorityID $
> authorityKeyNickname $ authorityEnabled $ authorityDN ) MAY (
> authoritySerial $ authorityParentID $ authorityParentDN $
> authorityKeyHost $ description ) X-ORIGIN 'user defined' )
> diff --git a/base/server/share/conf/schema.ldif
> b/base/server/share/conf/schema.ldif
> index
> a15601ae7a362635bc398b92b9bfda1c72f0dfc8..5e4118d328ebe1fcac2743b3f
> 51fb5ca9d57f9eb 100644
> --- a/base/server/share/conf/schema.ldif
> +++ b/base/server/share/conf/schema.ldif
> @@ -671,12 +671,13 @@ objectClasses: ( certProfile-oid NAME
> 'certProfile' DESC 'Certificate profile' S
> dn: cn=schema
> changetype: modify
> add: attributeTypes
> -attributeTypes: ( authorityID-oid NAME 'authorityID' DESC
> 'Authority ID' SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 X-ORIGIN 'user
> defined' )
> -attributeTypes: ( authorityKeyNickname-oid NAME
> 'authorityKeyNickname' DESC 'Authority key nickname' SYNTAX
> 1.3.6.1.4.1.1466.115.121.1.44 X-ORIGIN 'user-defined' )
> -attributeTypes: ( authorityParentID-oid NAME 'authorityParentID'
> DESC 'Authority Parent ID' SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 X
> -ORIGIN 'user defined' )
> -attributeTypes: ( authorityEnabled-oid NAME 'authorityEnabled'
> DESC 'Authority Enabled' SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 X
> -ORIGIN 'user defined' )
> -attributeTypes: ( authorityDN-oid NAME 'authorityDN' DESC
> 'Authority Enabled' SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 X-ORIGIN
> 'user defined' )
> -attributeTypes: ( authorityParentDN-oid NAME 'authorityParentDN'
> DESC 'Authority Enabled' SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 X
> -ORIGIN 'user defined' )
> +attributeTypes: ( authorityID-oid NAME 'authorityID' DESC
> 'Authority ID' SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 SINGLE-VALUE X
> -ORIGIN 'user defined' )
> +attributeTypes: ( authorityKeyNickname-oid NAME
> 'authorityKeyNickname' DESC 'Authority key nickname' SYNTAX
> 1.3.6.1.4.1.1466.115.121.1.44 SINGLE-VALUE X-ORIGIN 'user-defined'
> )
> +attributeTypes: ( authorityParentID-oid NAME 'authorityParentID'
> DESC 'Authority Parent ID' SYNTAX 1.3.6.1.4.1.1466.115.121.1.40
> SINGLE-VALUE X-ORIGIN 'user defined' )
> +attributeTypes: ( authorityEnabled-oid NAME 'authorityEnabled'
> DESC 'Authority Enabled' SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 SINGLE
> -VALUE X-ORIGIN 'user defined' )
> +attributeTypes: ( authorityDN-oid NAME 'authorityDN' DESC
> 'Authority DN' SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 SINGLE-VALUE X
> -ORIGIN 'user defined' )
> +attributeTypes: ( authorityParentDN-oid NAME 'authorityParentDN'
> DESC 'Authority Parent DN' SYNTAX 1.3.6.1.4.1.1466.115.121.1.12
> SINGLE-VALUE X-ORIGIN 'user defined' )
> +attributeTypes: ( authorityKeyHost-oid NAME 'authorityKeyHost'
> DESC 'Authority Key Hosts' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X
> -ORIGIN 'user defined' )
> -
> add: objectClasses
> -objectClasses: ( authority-oid NAME 'authority' DESC 'Certificate
> Authority' SUP top STRUCTURAL MUST ( cn $ authorityID $
> authorityKeyNickname $ authorityEnabled $ authorityDN ) MAY (
> authorityParentID $ authorityParentDN $ description ) X-ORIGIN
> 'user defined' )
> +objectClasses: ( authority-oid NAME 'authority' DESC 'Certificate
> Authority' SUP top STRUCTURAL MUST ( cn $ authorityID $
> authorityKeyNickname $ authorityEnabled $ authorityDN ) MAY (
> authorityParentID $ authorityParentDN $ authorityKeyHost $
> description ) X-ORIGIN 'user defined' )
> --
> 2.5.5
>
> From 3d42d650abf5b4862570620ef7b130bb511bb17d Mon Sep 17 00:00:00
> 2001
> From: Fraser Tweedale <ftweedal(a)redhat.com>
> Date: Tue, 19 Apr 2016 13:25:12 +1000
> Subject: [PATCH 98/99] Add method
> CryptoUtil.importPKIArchiveOptions
>
> Add the method CryptoUtil.importPKIArchiveOptions for importing a
> wrapped key from a PKIArchiveOptions object. Also add another
> variant of the createPKIArchiveOptions method, with a narrower API.
>
> Part of: https://fedorahosted.org/pki/ticket/1625
> ---
> .../com/netscape/cmsutil/crypto/CryptoUtil.java | 58
> ++++++++++++++++++++++
> 1 file changed, 58 insertions(+)
>
> diff --git
> a/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
> b/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
> index
> 06caa0242ab192c5bbc14845dd7abc772601bd58..7ac3e4e9ad2702e09c704638b
> 6ab773bb2dccb49 100644
> --- a/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
> +++ b/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
> @@ -80,7 +80,9 @@ import org.mozilla.jss.CryptoManager;
> import org.mozilla.jss.CryptoManager.NotInitializedException;
> import org.mozilla.jss.NoSuchTokenException;
> import org.mozilla.jss.SecretDecoderRing.KeyManager;
> +import org.mozilla.jss.asn1.ANY;
> import org.mozilla.jss.asn1.ASN1Util;
> +import org.mozilla.jss.asn1.ASN1Value;
> import org.mozilla.jss.asn1.BIT_STRING;
> import org.mozilla.jss.asn1.InvalidBERException;
> import org.mozilla.jss.asn1.OBJECT_IDENTIFIER;
> @@ -1971,6 +1973,31 @@ public class CryptoUtil {
> // wrap session key using transport key
> byte[] session_data = wrapSymmetricKey(manager, token,
> transportCert, sk);
>
> + return createPKIArchiveOptions(IV, session_data,
> key_data);
> + }
> +
> + public static byte[] createPKIArchiveOptions(
> + CryptoToken token, PublicKey wrappingKey, PrivateKey
> toBeWrapped,
> + KeyGenAlgorithm keyGenAlg, int symKeySize,
> IVParameterSpec IV)
> + throws TokenException, NoSuchAlgorithmException,
> + InvalidAlgorithmParameterException,
> InvalidKeyException,
> + IOException, InvalidBERException {
> + SymmetricKey sessionKey = CryptoUtil.generateKey(token,
> keyGenAlg, symKeySize);
> +
> + KeyWrapper wrapper =
> token.getKeyWrapper(KeyWrapAlgorithm.DES3_CBC_PAD);
> + wrapper.initWrap(sessionKey, IV);
> + byte[] key_data = wrapper.wrap(toBeWrapped);
> +
> + wrapper = token.getKeyWrapper(KeyWrapAlgorithm.RSA);
> + wrapper.initWrap(wrappingKey, null);
> + byte session_data[] = wrapper.wrap(sessionKey);
> +
> + return createPKIArchiveOptions(IV, session_data,
> key_data);
> + }
> +
> + private static byte[] createPKIArchiveOptions(
> + IVParameterSpec IV, byte[] session_data, byte[]
> key_data)
> + throws IOException, InvalidBERException {
> // create PKIArchiveOptions structure
> AlgorithmIdentifier algS = new AlgorithmIdentifier(new
> OBJECT_IDENTIFIER("1.2.840.113549.3.7"),
> new OCTET_STRING(IV.getIV()));
> @@ -1996,6 +2023,37 @@ public class CryptoUtil {
> return encoded;
> }
>
> + public static PrivateKey importPKIArchiveOptions(
> + CryptoToken token, PrivateKey unwrappingKey,
> + PublicKey pubkey, byte[] data)
> + throws InvalidBERException, Exception {
> + ByteArrayInputStream in = new ByteArrayInputStream(data);
> + PKIArchiveOptions options = (PKIArchiveOptions)
> + (new PKIArchiveOptions.Template()).decode(in);
> + EncryptedKey encKey = options.getEncryptedKey();
> + EncryptedValue encVal = encKey.getEncryptedValue();
> + AlgorithmIdentifier algId = encVal.getSymmAlg();
> + BIT_STRING encSymKey = encVal.getEncSymmKey();
> + BIT_STRING encPrivKey = encVal.getEncValue();
> +
> + KeyWrapper wrapper =
> token.getKeyWrapper(KeyWrapAlgorithm.RSA);
> + wrapper.initUnwrap(unwrappingKey, null);
> + SymmetricKey sk = wrapper.unwrapSymmetric(
> + encSymKey.getBits(), SymmetricKey.Type.DES3, 0);
> +
> + ASN1Value v = algId.getParameters();
> + v = ((ANY) v).decodeWith(new OCTET_STRING.Template());
> + byte iv[] = ((OCTET_STRING) v).toByteArray();
> + IVParameterSpec ivps = new IVParameterSpec(iv);
> +
> + wrapper =
> token.getKeyWrapper(KeyWrapAlgorithm.DES3_CBC_PAD);
> + wrapper.initUnwrap(sk, ivps);
> + PrivateKey.Type keyType =
> pubkey.getAlgorithm().equals("EC")
> + ? PrivateKey.Type.EC
> + : PrivateKey.Type.RSA;
> + return wrapper.unwrapPrivate(encPrivKey.getBits(),
> keyType, pubkey);
> + }
> +
> public static boolean sharedSecretExists(String nickname)
> throws NotInitializedException, TokenException {
> CryptoManager cm = CryptoManager.getInstance();
> CryptoToken token = cm.getInternalKeyStorageToken();
> --
> 2.5.5
>
> From ba32660ee26fc00132882809ce9365690f375c6c Mon Sep 17 00:00:00
> 2001
> From: Fraser Tweedale <ftweedal(a)redhat.com>
> Date: Tue, 19 Apr 2016 13:28:56 +1000
> Subject: [PATCH 99/99] Add ca-authority-key-export command
>
> Add the 'pki ca-authority-key-export' CLI command for exporting a
> PKIArchiveOptions object containing a nominated target key, wrapped
> by a nominated wrapping key. This command is to be used by
> Custodia
> to export key data for transmission to a requesting clone.
>
> Part of: https://fedorahosted.org/pki/ticket/1625
> ---
> .../netscape/cmstools/authority/AuthorityCLI.java | 1 +
> .../cmstools/authority/AuthorityKeyExportCLI.java | 109
> +++++++++++++++++++++
> 2 files changed, 110 insertions(+)
> create mode 100644 base/java
> -tools/src/com/netscape/cmstools/authority/AuthorityKeyExportCLI.ja
> va
>
> diff --git a/base/java
> -tools/src/com/netscape/cmstools/authority/AuthorityCLI.java
> b/base/java
> -tools/src/com/netscape/cmstools/authority/AuthorityCLI.java
> index
> ac06ea24ce824ad1b4be29a4176658caa9302e89..f42660d6727059bc76ab7ccd0
> bd0b22a87bc5f9a 100644
> --- a/base/java
> -tools/src/com/netscape/cmstools/authority/AuthorityCLI.java
> +++ b/base/java
> -tools/src/com/netscape/cmstools/authority/AuthorityCLI.java
> @@ -18,6 +18,7 @@ public class AuthorityCLI extends CLI {
> addModule(new AuthorityDisableCLI(this));
> addModule(new AuthorityEnableCLI(this));
> addModule(new AuthorityRemoveCLI(this));
> + addModule(new AuthorityKeyExportCLI(this));
> }
>
> public String getFullName() {
> diff --git a/base/java
> -tools/src/com/netscape/cmstools/authority/AuthorityKeyExportCLI.ja
> va b/base/java
> -tools/src/com/netscape/cmstools/authority/AuthorityKeyExportCLI.ja
> va
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..a3dee82c8d7ef3ad923aa5363
> 5b0825f3d272998
> --- /dev/null
> +++ b/base/java
> -tools/src/com/netscape/cmstools/authority/AuthorityKeyExportCLI.ja
> va
> @@ -0,0 +1,109 @@
> +package com.netscape.cmstools.authority;
> +
> +import java.nio.file.Files;
> +import java.nio.file.Paths;
> +import java.security.PublicKey;
> +
> +import org.apache.commons.cli.CommandLine;
> +import org.apache.commons.cli.Option;
> +import org.apache.commons.cli.ParseException;
> +
> +import org.mozilla.jss.CryptoManager;
> +import org.mozilla.jss.crypto.CryptoToken;
> +import org.mozilla.jss.crypto.IVParameterSpec;
> +import org.mozilla.jss.crypto.KeyGenAlgorithm;
> +import org.mozilla.jss.crypto.PrivateKey;
> +import org.mozilla.jss.crypto.X509Certificate;
> +
> +import com.netscape.cmstools.cli.CLI;
> +import com.netscape.cmsutil.crypto.CryptoUtil;
> +
> +public class AuthorityKeyExportCLI extends CLI {
> +
> + public AuthorityCLI authorityCLI;
> +
> + public AuthorityKeyExportCLI(AuthorityCLI authorityCLI) {
> + super("key-export", "Export wrapped CA signing key",
> authorityCLI);
> + this.authorityCLI = authorityCLI;
> +
> + options.addOption(null, "help", false, "Show usage");
> +
> + Option option = new Option("o", "output", true,
"Output
> file");
> + option.setArgName("filename");
> + options.addOption(option);
> +
> + option = new Option(null, "wrap-nickname", true, "Nickname
> of wrapping key");
> + option.setArgName("nickname");
> + options.addOption(option);
> +
> + option = new Option(null, "target-nickname", true,
> "Nickname of target key");
> + option.setArgName("nickname");
> + options.addOption(option);
> + }
> +
> + public void printHelp() {
> + formatter.printHelp(getFullName() + "--wrap-nickname
> NICKNAME --target-nickname NICKNAME -o FILENAME", options);
> + }
> +
> + public void execute(String[] args) throws Exception {
> + CommandLine cmd = null;
> +
> + try {
> + cmd = parser.parse(options, args);
> + } catch (ParseException e) {
> + System.err.println("Error: " + e.getMessage());
> + printHelp();
> + System.exit(-1);
> + }
> +
> + if (cmd.hasOption("help")) {
> + // Display usage
> + printHelp();
> + System.exit(0);
> + }
> +
> + String filename = cmd.getOptionValue("output");
> + if (filename == null) {
> + System.err.println("Error: No output file
> specified.");
> + printHelp();
> + System.exit(-1);
> + }
> +
> + String wrapNick = cmd.getOptionValue("wrap-nickname");
> + if (wrapNick == null) {
> + System.err.println("Error: no wrapping key nickname
> specified.");
> + printHelp();
> + System.exit(-1);
> + }
> +
> + String targetNick = cmd.getOptionValue("target-nickname");
> + if (targetNick == null) {
> + System.err.println("Error: no target key nickname
> specified.");
> + printHelp();
> + System.exit(-1);
> + }
> +
> + try {
> + CryptoManager cm = CryptoManager.getInstance();
> + X509Certificate wrapCert =
> cm.findCertByNickname(wrapNick);
> + X509Certificate targetCert =
> cm.findCertByNickname(targetNick);
> +
> + PublicKey wrappingKey = wrapCert.getPublicKey();
> + PrivateKey toBeWrapped =
> cm.findPrivKeyByCert(targetCert);
> + CryptoToken token = cm.getInternalKeyStorageToken();
> +
> + byte iv[] = { 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1
> };
> + IVParameterSpec ivps = new IVParameterSpec(iv);
> +
> + byte[] data = CryptoUtil.createPKIArchiveOptions(
> + token, wrappingKey, toBeWrapped,
> + KeyGenAlgorithm.DES3, 0, ivps);
> +
> +
> Files.newOutputStream(Paths.get(filename)).write(data);
> + } catch (Throwable e) {
> + e.printStackTrace();
> + System.exit(-1);
> + }
> +
> + }
> +}
> --
> 2.5.5
>
> From 4c8fdc1b11e052d469aee944e491cd8725b7a1e9 Mon Sep 17 00:00:00
> 2001
> From: Fraser Tweedale <ftweedal(a)redhat.com>
> Date: Wed, 30 Mar 2016 12:38:24 +1100
> Subject: [PATCH] Lightweight CAs: add key retrieval framework
>
> Add the framework for key retrieval when a lightweight CA is
> missing
> its signing key. This includes all the bits for loading a
> KeyRetriever implementation, initiating retrieval in a thread and
> updating the record of which clones possess the key if retrieval
> was
> successful.
>
> It does not include a KeyRetriever implementation. A subsequent
> commit will provide this.
>
> Part of: https://fedorahosted.org/pki/ticket/1625
> ---
> .../src/com/netscape/ca/CertificateAuthority.java | 162
> ++++++++++++++++++++-
> base/ca/src/com/netscape/ca/KeyRetriever.java | 56 +++++++
> .../src/netscape/security/pkcs/PKCS12Util.java | 3 +
> 3 files changed, 215 insertions(+), 6 deletions(-)
> create mode 100644 base/ca/src/com/netscape/ca/KeyRetriever.java
>
> diff --git a/base/ca/src/com/netscape/ca/CertificateAuthority.java
> b/base/ca/src/com/netscape/ca/CertificateAuthority.java
> index
> 37f1e95fc97f3d21ec6dc379962e27b42fb5b074..ac45141662aea732389353814
> e5a7e7b3ba516a7 100644
> --- a/base/ca/src/com/netscape/ca/CertificateAuthority.java
> +++ b/base/ca/src/com/netscape/ca/CertificateAuthority.java
> @@ -35,6 +35,7 @@ import java.security.cert.CertificateException;
> import java.security.cert.CertificateParsingException;
> import java.util.ArrayList;
> import java.util.Arrays;
> +import java.util.Collection;
> import java.util.Collections;
> import java.util.Date;
> import java.util.Enumeration;
> @@ -62,8 +63,10 @@ import org.mozilla.jss.crypto.CryptoToken;
> import org.mozilla.jss.crypto.KeyPairAlgorithm;
> import org.mozilla.jss.crypto.KeyPairGenerator;
> import org.mozilla.jss.crypto.NoSuchItemOnTokenException;
> +import org.mozilla.jss.crypto.PrivateKey;
> import org.mozilla.jss.crypto.SignatureAlgorithm;
> import org.mozilla.jss.crypto.TokenException;
> +import org.mozilla.jss.crypto.X509Certificate;
> import org.mozilla.jss.pkix.cert.Extension;
> import org.mozilla.jss.pkix.primitive.Name;
>
> @@ -205,6 +208,7 @@ public class CertificateAuthority
> protected AuthorityID authorityID = null;
> protected AuthorityID authorityParentID = null;
> protected String authorityDescription = null;
> + protected Collection<String> authorityKeyHosts = null;
> protected boolean authorityEnabled = true;
> private boolean hasKeys = false;
> private ECAException signingUnitException = null;
> @@ -340,6 +344,7 @@ public class CertificateAuthority
> AuthorityID aid,
> AuthorityID parentAID,
> String signingKeyNickname,
> + Collection<String> authorityKeyHosts,
> String authorityDescription,
> boolean authorityEnabled
> ) throws EBaseException {
> @@ -355,6 +360,7 @@ public class CertificateAuthority
> this.authorityDescription = authorityDescription;
> this.authorityEnabled = authorityEnabled;
> mNickname = signingKeyNickname;
> + this.authorityKeyHosts = authorityKeyHosts;
> init(hostCA.mOwner, hostCA.mConfig);
> }
>
> @@ -504,7 +510,7 @@ public class CertificateAuthority
>
> // init signing unit & CA cert.
> try {
> - initSigUnit();
> + initSigUnit(/* retrieveKeys */ true);
> // init default CA attributes like cert version,
> validity.
> initDefCaAttrs();
> } catch (EBaseException e) {
> @@ -1446,7 +1452,7 @@ public class CertificateAuthority
> /**
> * init CA signing unit & cert chain.
> */
> - private void initSigUnit()
> + private boolean initSigUnit(boolean retrieveKeys)
> throws EBaseException {
> try {
> // init signing unit
> @@ -1476,7 +1482,14 @@ public class CertificateAuthority
> } catch (CAMissingCertException |
> CAMissingKeyException e) {
> CMS.debug("CA signing key and cert not (yet)
> present in NSSDB");
> signingUnitException = e;
> - return;
> + if (retrieveKeys == true) {
> + CMS.debug("Starting KeyRetrieverRunner
> thread");
> + new Thread(
> + new KeyRetrieverRunner(this),
> + "KeyRetrieverRunner-" + authorityID
> + ).start();
> + }
> + return false;
> }
> CMS.debug("CA signing unit inited");
>
> @@ -1601,6 +1614,8 @@ public class CertificateAuthority
> mNickname = mSigningUnit.getNickname();
> CMS.debug("in init - got CA name " + mName);
>
> + return true;
> +
> } catch (CryptoManager.NotInitializedException e) {
> log(ILogger.LL_FAILURE,
> CMS.getLogMessage("CMSCORE_CA_CA_OCSP_SIGNING", e.toString()));
> throw new
> ECAException(CMS.getUserMessage("CMS_CA_CRYPTO_NOT_INITIALIZED"));
> @@ -2527,11 +2542,14 @@ public class CertificateAuthority
> throw new EBaseException("Failed to convert issuer DN
> to string: " + e);
> }
>
> + String thisClone = CMS.getEEHost() + ":" +
> CMS.getEESSLPort();
> +
> LDAPAttribute[] attrs = {
> new LDAPAttribute("objectclass", "authority"),
> new LDAPAttribute("cn", aidString),
> new LDAPAttribute("authorityID", aidString),
> new LDAPAttribute("authorityKeyNickname", nickname),
> + new LDAPAttribute("authorityKeyHost", thisClone),
> new LDAPAttribute("authorityEnabled", "TRUE"),
> new LDAPAttribute("authorityDN", subjectDN),
> new LDAPAttribute("authorityParentDN", parentDNString)
> @@ -2612,7 +2630,9 @@ public class CertificateAuthority
>
> return new CertificateAuthority(
> hostCA, subjectX500Name,
> - aid, this.authorityID, nickname, description, true);
> + aid, this.authorityID,
> + nickname, Collections.singleton(thisClone),
> + description, true);
> }
>
> /**
> @@ -2785,6 +2805,23 @@ public class CertificateAuthority
> }
> }
>
> + /**
> + * Add this instance to the authorityKeyHosts
> + */
> + private void addInstanceToAuthorityKeyHosts() throws
> ELdapException {
> + String hostname = CMS.getEEHost();
> + if (authorityKeyHosts.contains(hostname)) {
> + // already there; nothing to do
> + return;
> + }
> + LDAPModificationSet mods = new LDAPModificationSet();
> + mods.add(
> + LDAPModification.ADD,
> + new LDAPAttribute("authorityKeyHost", hostname));
> + modifyAuthorityEntry(mods);
> + authorityKeyHosts.add(hostname);
> + }
> +
> public synchronized void deleteAuthority() throws
> EBaseException {
> if (isHostAuthority())
> throw new CATypeException("Cannot delete the host
> CA");
> @@ -2933,7 +2970,6 @@ public class CertificateAuthority
> case LDAPPersistSearchControl.ADD:
> CMS.debug("authorityMonitor: ADD");
> readAuthority(entry);
> - // TODO kick off signing key
> replication via custodia
> break;
> case LDAPPersistSearchControl.DELETE:
> CMS.debug("authorityMonitor: DELETE");
> @@ -2990,6 +3026,7 @@ public class CertificateAuthority
>
> LDAPAttribute aidAttr = entry.getAttribute("authorityID");
> LDAPAttribute nickAttr =
> entry.getAttribute("authorityKeyNickname");
> + LDAPAttribute keyHostsAttr =
> entry.getAttribute("authorityKeyHost");
> LDAPAttribute dnAttr = entry.getAttribute("authorityDN");
> LDAPAttribute parentAIDAttr =
> entry.getAttribute("authorityParentID");
> LDAPAttribute parentDNAttr =
> entry.getAttribute("authorityParentDN");
> @@ -3052,6 +3089,16 @@ public class CertificateAuthority
> }
>
> String keyNick = (String)
> nickAttr.getStringValues().nextElement();
> +
> + Collection<String> keyHosts;
> + if (keyHostsAttr == null) {
> + keyHosts = Collections.emptyList();
> + } else {
> + @SuppressWarnings("unchecked")
> + Enumeration<String> keyHostsEnum =
> keyHostsAttr.getStringValues();
> + keyHosts = Collections.list(keyHostsEnum);
> + }
> +
> AuthorityID parentAID = null;
> if (parentAIDAttr != null)
> parentAID = new AuthorityID((String)
> @@ -3067,7 +3114,7 @@ public class CertificateAuthority
>
> try {
> CertificateAuthority ca = new CertificateAuthority(
> - hostCA, dn, aid, parentAID, keyNick, desc,
> enabled);
> + hostCA, dn, aid, parentAID, keyNick, keyHosts,
> desc, enabled);
> caMap.put(aid, ca);
> entryUSNs.put(aid, newEntryUSN);
> nsUniqueIds.put(aid, nsUniqueId);
> @@ -3117,4 +3164,107 @@ public class CertificateAuthority
> }
> }
>
> + private class KeyRetrieverRunner implements Runnable {
> + private CertificateAuthority ca;
> +
> + public KeyRetrieverRunner(CertificateAuthority ca) {
> + this.ca = ca;
> + }
> +
> + public void run() {
> + String KR_CLASS_KEY =
> "features.authority.keyRetrieverClass";
> + String className = null;
> + try {
> + className =
> CMS.getConfigStore().getString(KR_CLASS_KEY);
> + } catch (EBaseException e) {
> + CMS.debug("Unable to read key retriever class from
> CS.cfg: " + e);
> + return;
> + }
> +
> + KeyRetriever kr = null;
> + try {
> + kr = Class.forName(className)
> + .asSubclass(KeyRetriever.class)
> + .newInstance();
> + } catch (ClassNotFoundException e) {
> + CMS.debug("Could not find class: " + className);
> + CMS.debug(e);
> + return;
> + } catch (ClassCastException e) {
> + CMS.debug("Class is not an instance of
> KeyRetriever: " + className);
> + CMS.debug(e);
> + return;
> + } catch (InstantiationException |
> IllegalAccessException e) {
> + CMS.debug("Could not instantiate class: " +
> className);
> + CMS.debug(e);
> + return;
> + }
> +
> + KeyRetriever.Result krr = null;
> + try {
> + krr = kr.retrieveKey(ca.mNickname,
> ca.authorityKeyHosts);
> + } catch (Throwable e) {
> + CMS.debug("Caught exception during execution of
> KeyRetriever.retrieveKey");
> + CMS.debug(e);
> + return;
> + }
> +
> + if (krr == null) {
> + CMS.debug("KeyRetriever did not return a
> result.");
> + return;
> + }
> +
> + CMS.debug("Importing key and cert");
> + byte[] certBytes = krr.getCertificate();
> + byte[] paoData = krr.getPKIArchiveOptions();
> + try {
> + CryptoManager manager =
> CryptoManager.getInstance();
> + CryptoToken token =
> manager.getInternalKeyStorageToken();
> +
> + X509Certificate cert =
> manager.importCACertPackage(certBytes);
> + PublicKey pubkey = cert.getPublicKey();
> + token.getCryptoStore().deleteCert(cert);
> +
> + PrivateKey unwrappingKey =
> hostCA.mSigningUnit.getPrivateKey();
> +
> + CryptoUtil.importPKIArchiveOptions(
> + token, unwrappingKey, pubkey, paoData);
> +
> + cert = manager.importUserCACertPackage(certBytes,
> ca.mNickname);
> + } catch (Throwable e) {
> + CMS.debug("Caught exception during cert/key
> import");
> + CMS.debug(e);
> + return;
> + }
> +
> + boolean initSigUnitSucceeded = false;
> + try {
> + CMS.debug("Reinitialising SigningUnit");
> + // re-init signing unit, but avoid triggering
> + // key replication if initialisation fails again
> + // for some reason
> + //
> + initSigUnitSucceeded = ca.initSigUnit(/*
> retrieveKeys */ false);
> + } catch (Throwable e) {
> + CMS.debug("Caught exception during SigningUnit re
> -init");
> + CMS.debug(e);
> + return;
> + }
> +
> + if (!initSigUnitSucceeded) {
> + CMS.debug("Failed to re-init SigningUnit");
> + return;
> + }
> +
> + CMS.debug("Adding self to authorityKeyHosts
> attribute");
> + try {
> + ca.addInstanceToAuthorityKeyHosts();
> + } catch (Throwable e) {
> + CMS.debug("Failed to add self to
> authorityKeyHosts");
> + CMS.debug(e);
> + return;
> + }
> + }
> + }
> +
> }
> diff --git a/base/ca/src/com/netscape/ca/KeyRetriever.java
> b/base/ca/src/com/netscape/ca/KeyRetriever.java
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..7c0df0bf56578b81062de77de
> 47aa516b5c9d949
> --- /dev/null
> +++ b/base/ca/src/com/netscape/ca/KeyRetriever.java
> @@ -0,0 +1,56 @@
> +// --- BEGIN COPYRIGHT BLOCK ---
> +// This program is free software; you can redistribute it and/or
> modify
> +// it under the terms of the GNU General Public License as
> published by
> +// the Free Software Foundation; version 2 of the License.
> +//
> +// This program is distributed in the hope that it will be useful,
> +// but WITHOUT ANY WARRANTY; without even the implied warranty of
> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +// GNU General Public License for more details.
> +//
> +// You should have received a copy of the GNU General Public
> License along
> +// with this program; if not, write to the Free Software
> Foundation, Inc.,
> +// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> +//
> +// (C) 2016 Red Hat, Inc.
> +// All rights reserved.
> +// --- END COPYRIGHT BLOCK ---
> +
> +package com.netscape.ca;
> +
> +import java.util.Collection;
> +
> +public interface KeyRetriever {
> + /**
> + * Retrieve the specified signing key from specified clone and
> + * return to the KeyRetrieverRunner.
> + *
> + * A KeyRetriever MUST NOT import the cert and key to the
> NSSDB
> + * itself. It SHALL, if successful in retrieving the key and
> + * certificate, return a Result which contains a PEM-encoded
> + * X.509 certificate and a DER-encoded PKIArchiveOptions
> object
> + * containing an EncryptedValue of the target private key
> + * wrapped by the host authority's public key.
> + *
> + * Upon failure the KeyRetriever SHALL return null.
> + */
> + Result retrieveKey(String nickname, Collection<String>
> hostPorts);
> +
> + class Result {
> + private byte[] certificate;
> + private byte[] pkiArchiveOptions;
> +
> + public Result(byte[] certificate, byte[]
> pkiArchiveOptions) {
> + this.certificate = certificate;
> + this.pkiArchiveOptions = pkiArchiveOptions;
> + }
> +
> + public byte[] getCertificate() {
> + return certificate;
> + }
> +
> + public byte[] getPKIArchiveOptions() {
> + return pkiArchiveOptions;
> + }
> + }
> +}
> diff --git a/base/util/src/netscape/security/pkcs/PKCS12Util.java
> b/base/util/src/netscape/security/pkcs/PKCS12Util.java
> index
> 43435c822c9400248fe556bf066cd2659e18ae17..9931027dacfe88e636b694b7c
> 490ffc6804068dd 100644
> --- a/base/util/src/netscape/security/pkcs/PKCS12Util.java
> +++ b/base/util/src/netscape/security/pkcs/PKCS12Util.java
> @@ -536,7 +536,10 @@ public class PKCS12Util {
>
> Path path = Paths.get(filename);
> byte[] b = Files.readAllBytes(path);
> + return loadFromByteArray(b, password);
> + }
>
> + public PKCS12 loadFromByteArray(byte[] b, Password password)
> throws Exception {
> ByteArrayInputStream bis = new ByteArrayInputStream(b);
>
> PFX pfx = (PFX) (new PFX.Template()).decode(bis);
> --
> 2.5.5
>
> From d2e57290b2d3392082335955be72dfaf3111afd0 Mon Sep 17 00:00:00
> 2001
> From: Fraser Tweedale <ftweedal(a)redhat.com>
> Date: Fri, 8 Apr 2016 22:23:42 +1000
> Subject: [PATCH] Lightweight CAs: add IPACustodiaKeyRetriever
>
> Add 'IPACustodiaKeyRetriever', a 'KeyRetriever' implementation for
> use when Dogtag is deployed as a FreeIPA CA. The Java class
> invokes
> 'pki-ipa-key-retriever', a Python script that retrieves lightweight
> CA keys from the Custodia server on a replica that possesses the
> keys. 'pki-ipa-key-retriever' depends on FreeIPA libraries,
> FreeIPA
> server configuration, and Kerberos and Custodia keys owned by
> 'pkiuser'.
>
> Part of: https://fedorahosted.org/pki/ticket/1625
> ---
> base/ca/src/CMakeLists.txt | 9 ++-
> .../com/netscape/ca/IPACustodiaKeyRetriever.java | 75
> ++++++++++++++++++++++
> base/server/CMakeLists.txt | 11 ++++
> base/server/libexec/pki-ipa-retrieve-key | 42
> ++++++++++++
> specs/pki-core.spec | 1 +
> 5 files changed, 137 insertions(+), 1 deletion(-)
> create mode 100644
> base/ca/src/com/netscape/ca/IPACustodiaKeyRetriever.java
> create mode 100755 base/server/libexec/pki-ipa-retrieve-key
>
> diff --git a/base/ca/src/CMakeLists.txt
> b/base/ca/src/CMakeLists.txt
> index
> 5b805e1b3a5eddb46d17178ca2ac204c36ae5680..1817dacfbacaeb2635db2550e
> 32ff62c26d628ef 100644
> --- a/base/ca/src/CMakeLists.txt
> +++ b/base/ca/src/CMakeLists.txt
> @@ -24,6 +24,13 @@ find_file(COMMONS_CODEC_JAR
> /usr/share/java
> )
>
> +find_file(COMMONS_IO_JAR
> + NAMES
> + commons-io.jar
> + PATHS
> + /usr/share/java
> +)
> +
> find_file(COMMONS_LANG_JAR
> NAMES
> commons-lang.jar
> @@ -73,7 +80,7 @@ javac(pki-ca-classes
> com/netscape/ca/*.java
> org/dogtagpki/server/ca/*.java
> CLASSPATH
> - ${COMMONS_CODEC_JAR} ${COMMONS_LANG_JAR}
> + ${COMMONS_CODEC_JAR} ${COMMONS_IO_JAR} ${COMMONS_LANG_JAR}
> ${JSS_JAR} ${SYMKEY_JAR}
> ${LDAPJDK_JAR}
> ${SERVLET_JAR} ${TOMCAT_CATALINA_JAR}
> diff --git
> a/base/ca/src/com/netscape/ca/IPACustodiaKeyRetriever.java
> b/base/ca/src/com/netscape/ca/IPACustodiaKeyRetriever.java
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..4a162d3702fccc19dfef792a5
> 213c653286930f3
> --- /dev/null
> +++ b/base/ca/src/com/netscape/ca/IPACustodiaKeyRetriever.java
> @@ -0,0 +1,75 @@
> +// --- BEGIN COPYRIGHT BLOCK ---
> +// This program is free software; you can redistribute it and/or
> modify
> +// it under the terms of the GNU General Public License as
> published by
> +// the Free Software Foundation; version 2 of the License.
> +//
> +// This program is distributed in the hope that it will be useful,
> +// but WITHOUT ANY WARRANTY; without even the implied warranty of
> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +// GNU General Public License for more details.
> +//
> +// You should have received a copy of the GNU General Public
> License along
> +// with this program; if not, write to the Free Software
> Foundation, Inc.,
> +// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> +//
> +// (C) 2016 Red Hat, Inc.
> +// All rights reserved.
> +// --- END COPYRIGHT BLOCK ---
> +
> +package com.netscape.ca;
> +
> +import java.lang.Process;
> +import java.lang.ProcessBuilder;
> +import java.util.Collection;
> +import java.util.Stack;
> +
> +import org.apache.commons.io.IOUtils;
> +import org.apache.commons.lang.ArrayUtils;
> +
> +import com.netscape.certsrv.apps.CMS;
> +
> +public class IPACustodiaKeyRetriever implements KeyRetriever {
> + public Result retrieveKey(String nickname, Collection<String>
> hostPorts) {
> + CMS.debug("Running IPACustodiaKeyRetriever");
> +
> + Stack<String> command = new Stack<>();
> + command.push("/usr/libexec/pki-ipa-retrieve-key");
> + command.push(nickname);
> +
> + for (String hostPort : hostPorts) {
> + String host = hostPort.split(":")[0];
> + command.push(host);
> + CMS.debug("About to execute command: " + command);
> + ProcessBuilder pb = new ProcessBuilder(command);
> + try {
> + Process p = pb.start();
> + int exitValue = p.waitFor();
> + if (exitValue != 0)
> + continue;
> +
> + /* Custodia returns a PEM-encoded certificate and
> a
> + * base64-encoded PKIArchiveOptions containing the
> + * wrapped private key. These values are output
> by
> + * the Python 'pki-ipa-retrieve-key' program,
> + * separated by a null byte (password first)
> + */
> + byte[] output =
> IOUtils.toByteArray(p.getInputStream());
> + int splitIndex = ArrayUtils.indexOf(output, (byte)
> 0);
> + if (splitIndex == ArrayUtils.INDEX_NOT_FOUND) {
> + CMS.debug("Invalid output: null byte not
> found");
> + continue;
> + }
> + return new Result(
> + ArrayUtils.subarray(output, 0, splitIndex),
> + ArrayUtils.subarray(output, splitIndex + 1,
> output.length)
> + );
> + } catch (Throwable e) {
> + CMS.debug("Caught exception while executing
> command: " + e);
> + } finally {
> + command.pop();
> + }
> + }
> + CMS.debug("Failed to retrieve key from any host.");
> + return null;
> + }
> +}
> diff --git a/base/server/CMakeLists.txt
> b/base/server/CMakeLists.txt
> index
> 5a6aea96a2317655fb454967f9f218020443bcb8..9e5b27833c8d023e63320c43d
> 64ad64b0055c254 100644
> --- a/base/server/CMakeLists.txt
> +++ b/base/server/CMakeLists.txt
> @@ -81,6 +81,17 @@ install(
>
> install(
> DIRECTORY
> + libexec/
> + DESTINATION
> + ${LIBEXEC_INSTALL_DIR}
> + FILE_PERMISSIONS
> + OWNER_EXECUTE OWNER_WRITE OWNER_READ
> + GROUP_EXECUTE GROUP_READ
> + WORLD_EXECUTE WORLD_READ
> +)
> +
> +install(
> + DIRECTORY
> upgrade
> DESTINATION
> ${DATA_INSTALL_DIR}/server/
> diff --git a/base/server/libexec/pki-ipa-retrieve-key
> b/base/server/libexec/pki-ipa-retrieve-key
> new file mode 100755
> index
> 0000000000000000000000000000000000000000..9305150a446bb4deed339b89d
> aad821a19b9e7df
> --- /dev/null
> +++ b/base/server/libexec/pki-ipa-retrieve-key
> @@ -0,0 +1,42 @@
> +#!/usr/bin/python
> +
> +from __future__ import print_function
> +
> +import ConfigParser
> +import base64
> +import sys
> +
> +from jwcrypto.common import json_decode
> +
> +from ipaplatform.paths import paths
> +from ipapython.secrets.client import CustodiaClient
> +
> +conf = ConfigParser.ConfigParser()
> +conf.read(paths.IPA_DEFAULT_CONF)
> +hostname = conf.get('global', 'host')
> +realm = conf.get('global', 'realm')
> +
> +keyname = "ca_wrapped/" + sys.argv[1]
> +servername = sys.argv[2]
> +
> +client_keyfile = "/etc/pki/pki-tomcat/dogtag-ipa-custodia.keys"
> +client_keytab = "/etc/pki/pki-tomcat/dogtag-ipa-custodia.keytab"
> +
> +client = CustodiaClient(
> + client=hostname, server=servername, realm=realm,
> + ldap_uri="ldaps://" + hostname,
> + client_servicename='dogtag-ipa-custodia',
> + keyfile=client_keyfile, keytab=client_keytab,
> + )
> +
> +result_json = client.fetch_key(keyname, store=False)
> +result = json_decode(result_json)
> +certificate = result["certificate"]
> +wrapped_key = base64.b64decode(result["wrapped_key"])
> +
> +# Custodia returns a PEM-encoded certificate and a base64-encoded
> +# DER PKIArchiveOptions object. Output these values, separated by
> a
> +# null byte (certificate first), to be read by the Java
> +# IPACustodiaKeyRetriever that invoked this program.
> +
> +print(certificate, wrapped_key, sep='\0', end='')
> diff --git a/specs/pki-core.spec b/specs/pki-core.spec
> index
> bce6bd2d298bc8e1ad5cb40618f982b5ba23b27d..509ecdafa4e5d0e651ce22497
> db06420abcdf259 100644
> --- a/specs/pki-core.spec
> +++ b/specs/pki-core.spec
> @@ -1007,6 +1007,7 @@ systemctl daemon-reload
> %{_sbindir}/pki-server
> %{_sbindir}/pki-server-nuxwdog
> %{_sbindir}/pki-server-upgrade
> +%{_libexecdir}/pki-ipa-retrieve-key
> %{python2_sitelib}/pki/server/
> %dir %{_datadir}/pki/deployment
> %{_datadir}/pki/deployment/config/
11:58 p.m.
Thanks Ade. Updated patch 0096 attached. Comments inline.
On Wed, Apr 20, 2016 at 11:30:52AM -0400, Ade Lee wrote:
Comments:
95 - ack
96 -
1. You have made the return type of initSigUnit() to be boolean.
Should you be checking the return value in init()?
It is not needed to check it here; only when re-entering init from
the KeyReplicatorRunner thread.
2. In addInstanceToAuthorityKeyHosts(), you are still using only the
hostname. Should be host:port
Good pickup. Fixed in latest patch.
3. The logic in the KeyRetrieverRunner class looks OK to me, but
I'd
like cfu and/or jmagne to check it and make sure we are calling the
right primitives to wrap/unwrap inside the cryptographic token.
Also I'd like them to confirm that this would wor for an HSM.
Statements like the following make me question that:
CryptoToken token = manager.getInternalKeyStorageToken()
It won't work on HSM. Can I get an HSM to test with? ;) I've filed
a ticket for HSM support[1]. FreeIPA does not yet support HSM[2] so
I think we can put it in 10.4 milestone (I've put it there for now).
[1] https://fedorahosted.org/pki/ticket/2292
[2] https://fedorahosted.org/freeipa/ticket/5608
4. Can you explain what happens if for some reason the script fails
to
retrieve the key? Do we end up retrying later and if so, when?
If the script fails to retrieve the key, it does not retry
automatically. I filed a ticket[3] to implement retry with
backoff (this patchset is big enough already!) and put it in
10.3.1 milestone (that's up for discussion).
[3] https://fedorahosted.org/pki/ticket/2293
Right now, the following events cause authority reinitialisation,
entailing key retrieval if necessary:
- Dogtag is restarted
- LDAP disconnect-reconnect
- LDAP modification of authority replicated from another clone
97- ACK
98 - ACK
Thanks. Any feedback on patch 0099?
9:29 a.m.
ACK on latest 96 and 99.
I will ask cfu or jmagne to look at the KeyRetrieveRunner logic today.
Ade
On Thu, 2016-04-21 at 14:58 +1000, Fraser Tweedale wrote:
Thanks Ade. Updated patch 0096 attached. Comments inline.
On Wed, Apr 20, 2016 at 11:30:52AM -0400, Ade Lee wrote:
> Comments:
>
> 95 - ack
>
> 96 -
>
> 1. You have made the return type of initSigUnit() to be boolean.
> Should you be checking the return value in init()?
>
It is not needed to check it here; only when re-entering init from
the KeyReplicatorRunner thread.
> 2. In addInstanceToAuthorityKeyHosts(), you are still using only
> the
> hostname. Should be host:port
>
Good pickup. Fixed in latest patch.
> 3. The logic in the KeyRetrieverRunner class looks OK to me, but
> I'd
> like cfu and/or jmagne to check it and make sure we are calling the
> right primitives to wrap/unwrap inside the cryptographic token.
>
> Also I'd like them to confirm that this would wor for an HSM.
> Statements like the following make me question that:
> CryptoToken token = manager.getInternalKeyStorageToken()
>
It won't work on HSM. Can I get an HSM to test with? ;) I've filed
a ticket for HSM support[1]. FreeIPA does not yet support HSM[2] so
I think we can put it in 10.4 milestone (I've put it there for now).
[1] https://fedorahosted.org/pki/ticket/2292
[2] https://fedorahosted.org/freeipa/ticket/5608
> 4. Can you explain what happens if for some reason the script fails
> to
> retrieve the key? Do we end up retrying later and if so, when?
>
If the script fails to retrieve the key, it does not retry
automatically. I filed a ticket[3] to implement retry with
backoff (this patchset is big enough already!) and put it in
10.3.1 milestone (that's up for discussion).
[3] https://fedorahosted.org/pki/ticket/2293
Right now, the following events cause authority reinitialisation,
entailing key retrieval if necessary:
- Dogtag is restarted
- LDAP disconnect-reconnect
- LDAP modification of authority replicated from another clone
> 97- ACK
>
> 98 - ACK
>
Thanks. Any feedback on patch 0099?
7:03 p.m.
On Thu, Apr 21, 2016 at 10:29:32AM -0400, Ade Lee wrote:
ACK on latest 96 and 99.
I will ask cfu or jmagne to look at the KeyRetrieveRunner logic today.
Ade
Thanks Ade; I'll wait for their feedback before I merge it.
Cheers,
Fraser
On Thu, 2016-04-21 at 14:58 +1000, Fraser Tweedale wrote:
> Thanks Ade. Updated patch 0096 attached. Comments inline.
>
> On Wed, Apr 20, 2016 at 11:30:52AM -0400, Ade Lee wrote:
> > Comments:
> >
> > 95 - ack
> >
> > 96 -
> >
> > 1. You have made the return type of initSigUnit() to be boolean.
> > Should you be checking the return value in init()?
> >
> It is not needed to check it here; only when re-entering init from
> the KeyReplicatorRunner thread.
>
> > 2. In addInstanceToAuthorityKeyHosts(), you are still using only
> > the
> > hostname. Should be host:port
> >
> Good pickup. Fixed in latest patch.
>
> > 3. The logic in the KeyRetrieverRunner class looks OK to me, but
> > I'd
> > like cfu and/or jmagne to check it and make sure we are calling the
> > right primitives to wrap/unwrap inside the cryptographic token.
> >
> > Also I'd like them to confirm that this would wor for an HSM.
> > Statements like the following make me question that:
> > CryptoToken token = manager.getInternalKeyStorageToken()
> >
> It won't work on HSM. Can I get an HSM to test with? ;) I've filed
> a ticket for HSM support[1]. FreeIPA does not yet support HSM[2] so
> I think we can put it in 10.4 milestone (I've put it there for now).
>
> [1] https://fedorahosted.org/pki/ticket/2292
> [2] https://fedorahosted.org/freeipa/ticket/5608
>
> > 4. Can you explain what happens if for some reason the script fails
> > to
> > retrieve the key? Do we end up retrying later and if so, when?
> >
> If the script fails to retrieve the key, it does not retry
> automatically. I filed a ticket[3] to implement retry with
> backoff (this patchset is big enough already!) and put it in
> 10.3.1 milestone (that's up for discussion).
>
> [3] https://fedorahosted.org/pki/ticket/2293
>
> Right now, the following events cause authority reinitialisation,
> entailing key retrieval if necessary:
>
> - Dogtag is restarted
> - LDAP disconnect-reconnect
> - LDAP modification of authority replicated from another clone
>
> > 97- ACK
> >
> > 98 - ACK
> >
> Thanks. Any feedback on patch 0099?
6:50 p.m.
New subject: [PATCH] 0084..0086 Lightweight CA replication support
I took a look at the stuff alee asked for.
CFU even took a quick look when I asked her a couple of questions.
She was unsure of something (as was I) and she would like to be able
to take a closer look next week. I will give my quick thoughts.
1. I agree that HSM support is not in the patch, seems fine to move that
to a future ticket.
Here is one thing I was kind of worried about:
This is the code that imports the archive of the desired private key.
ublic static PrivateKey importPKIArchiveOptions(
+ CryptoToken token, PrivateKey unwrappingKey,
+ PublicKey pubkey, byte[] data)
+ throws InvalidBERException, Exception {
+ ByteArrayInputStream in = new ByteArrayInputStream(data);
+ PKIArchiveOptions options = (PKIArchiveOptions)
+ (new PKIArchiveOptions.Template()).decode(in);
+ EncryptedKey encKey = options.getEncryptedKey();
+ EncryptedValue encVal = encKey.getEncryptedValue();
+ AlgorithmIdentifier algId = encVal.getSymmAlg();
+ BIT_STRING encSymKey = encVal.getEncSymmKey();
+ BIT_STRING encPrivKey = encVal.getEncValue();
This the wrapper object that is build off of the caSigningUnit key gotten
in the other patch, the RetrieverThread like this:
PrivateKey unwrappingKey = hostCA.mSigningUnit.getPrivateKey();
The code below works fine if said key is RSA. I talked over with CFU and she said there
could be a chance this key is ECC for an ECC CA.
We both think the rest of the code in this routine is fine, except for possibly that.
She is also not even sure if JSS can support an ECC private key wrapper.
She requests you guys give her a day or two to look at it.
Except for the hsm issue, the code that calls this routine in the thread seems fine too.
+
+ KeyWrapper wrapper = token.getKeyWrapper(KeyWrapAlgorithm.RSA);
+ wrapper.initUnwrap(unwrappingKey, null);
+ SymmetricKey sk = wrapper.unwrapSymmetric(
+ encSymKey.getBits(), SymmetricKey.Type.DES3, 0);
+
+ ASN1Value v = algId.getParameters();
+ v = ((ANY) v).decodeWith(new OCTET_STRING.Template());
+ byte iv[] = ((OCTET_STRING) v).toByteArray();
+ IVParameterSpec ivps = new IVParameterSpec(iv);
+
+ wrapper = token.getKeyWrapper(KeyWrapAlgorithm.DES3_CBC_PAD);
+ wrapper.initUnwrap(sk, ivps);
+ PrivateKey.Type keyType = pubkey.getAlgorithm().equals("EC")
+ ? PrivateKey.Type.EC
+ : PrivateKey.Type.RSA;
+ return wrapper.unwrapPrivate(encPrivKey.getBits(), keyType, pubkey);
+ }
----- Original Message -----
From: "Fraser Tweedale" <ftweedal(a)redhat.com>
To: "Ade Lee" <alee(a)redhat.com>
Cc: pki-devel(a)redhat.com
Sent: Wednesday, April 20, 2016 9:58:54 PM
Subject: Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support
Thanks Ade. Updated patch 0096 attached. Comments inline.
On Wed, Apr 20, 2016 at 11:30:52AM -0400, Ade Lee wrote:
> Comments:
>
> 95 - ack
>
> 96 -
>
> 1. You have made the return type of initSigUnit() to be boolean.
> Should you be checking the return value in init()?
>
It is not needed to check it here; only when re-entering init from
the KeyReplicatorRunner thread.
> 2. In addInstanceToAuthorityKeyHosts(), you are still using only the
> hostname. Should be host:port
>
Good pickup. Fixed in latest patch.
> 3. The logic in the KeyRetrieverRunner class looks OK to me, but I'd
> like cfu and/or jmagne to check it and make sure we are calling the
> right primitives to wrap/unwrap inside the cryptographic token.
>
> Also I'd like them to confirm that this would wor for an HSM.
> Statements like the following make me question that:
> CryptoToken token = manager.getInternalKeyStorageToken()
>
It won't work on HSM. Can I get an HSM to test with? ;) I've filed
a ticket for HSM support[1]. FreeIPA does not yet support HSM[2] so
I think we can put it in 10.4 milestone (I've put it there for now).
[1] https://fedorahosted.org/pki/ticket/2292
[2] https://fedorahosted.org/freeipa/ticket/5608
> 4. Can you explain what happens if for some reason the script fails to
> retrieve the key? Do we end up retrying later and if so, when?
>
If the script fails to retrieve the key, it does not retry
automatically. I filed a ticket[3] to implement retry with
backoff (this patchset is big enough already!) and put it in
10.3.1 milestone (that's up for discussion).
[3] https://fedorahosted.org/pki/ticket/2293
Right now, the following events cause authority reinitialisation,
entailing key retrieval if necessary:
- Dogtag is restarted
- LDAP disconnect-reconnect
- LDAP modification of authority replicated from another clone
> 97- ACK
>
> 98 - ACK
>
Thanks. Any feedback on patch 0099?
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
4:21 a.m.
On Fri, Apr 22, 2016 at 07:50:06PM -0400, John Magne wrote:
I took a look at the stuff alee asked for.
CFU even took a quick look when I asked her a couple of questions.
She was unsure of something (as was I) and she would like to be able
to take a closer look next week. I will give my quick thoughts.
1. I agree that HSM support is not in the patch, seems fine to move that
to a future ticket.
Thanks. I filed ticket: https://fedorahosted.org/pki/ticket/2292
Here is one thing I was kind of worried about:
This is the code that imports the archive of the desired private key.
ublic static PrivateKey importPKIArchiveOptions(
+ CryptoToken token, PrivateKey unwrappingKey,
+ PublicKey pubkey, byte[] data)
+ throws InvalidBERException, Exception {
+ ByteArrayInputStream in = new ByteArrayInputStream(data);
+ PKIArchiveOptions options = (PKIArchiveOptions)
+ (new PKIArchiveOptions.Template()).decode(in);
+ EncryptedKey encKey = options.getEncryptedKey();
+ EncryptedValue encVal = encKey.getEncryptedValue();
+ AlgorithmIdentifier algId = encVal.getSymmAlg();
+ BIT_STRING encSymKey = encVal.getEncSymmKey();
+ BIT_STRING encPrivKey = encVal.getEncValue();
This the wrapper object that is build off of the caSigningUnit key gotten
in the other patch, the RetrieverThread like this:
PrivateKey unwrappingKey = hostCA.mSigningUnit.getPrivateKey();
The code below works fine if said key is RSA. I talked over with CFU and she said there
could be a chance this key is ECC for an ECC CA.
We both think the rest of the code in this routine is fine, except for possibly that.
She is also not even sure if JSS can support an ECC private key wrapper.
Yes, it is currently not supported in JSS (I'm unsure if NSS
supports it). However, because the first release of Lightweight CAs
is to support FreeIPA sub-CAs feature, and FreeIPA does not yet
support EC CA, I don't think it's a show-stopper. I filed a ticket
for key replication with non-RSA CA:
https://fedorahosted.org/pki/ticket/2291
She requests you guys give her a day or two to look at it.
No problem. Thank you (and Christina) for the review.
Cheers,
Fraser
Except for the hsm issue, the code that calls this routine in the
thread seems fine too.
+
+ KeyWrapper wrapper = token.getKeyWrapper(KeyWrapAlgorithm.RSA);
+ wrapper.initUnwrap(unwrappingKey, null);
+ SymmetricKey sk = wrapper.unwrapSymmetric(
+ encSymKey.getBits(), SymmetricKey.Type.DES3, 0);
+
+ ASN1Value v = algId.getParameters();
+ v = ((ANY) v).decodeWith(new OCTET_STRING.Template());
+ byte iv[] = ((OCTET_STRING) v).toByteArray();
+ IVParameterSpec ivps = new IVParameterSpec(iv);
+
+ wrapper = token.getKeyWrapper(KeyWrapAlgorithm.DES3_CBC_PAD);
+ wrapper.initUnwrap(sk, ivps);
+ PrivateKey.Type keyType = pubkey.getAlgorithm().equals("EC")
+ ? PrivateKey.Type.EC
+ : PrivateKey.Type.RSA;
+ return wrapper.unwrapPrivate(encPrivKey.getBits(), keyType, pubkey);
+ }
----- Original Message -----
> From: "Fraser Tweedale" <ftweedal(a)redhat.com>
> To: "Ade Lee" <alee(a)redhat.com>
> Cc: pki-devel(a)redhat.com
> Sent: Wednesday, April 20, 2016 9:58:54 PM
> Subject: Re: [Pki-devel] [PATCH] 0084..0086 Lightweight CA replication support
>
> Thanks Ade. Updated patch 0096 attached. Comments inline.
>
> On Wed, Apr 20, 2016 at 11:30:52AM -0400, Ade Lee wrote:
> > Comments:
> >
> > 95 - ack
> >
> > 96 -
> >
> > 1. You have made the return type of initSigUnit() to be boolean.
> > Should you be checking the return value in init()?
> >
> It is not needed to check it here; only when re-entering init from
> the KeyReplicatorRunner thread.
>
> > 2. In addInstanceToAuthorityKeyHosts(), you are still using only the
> > hostname. Should be host:port
> >
> Good pickup. Fixed in latest patch.
>
> > 3. The logic in the KeyRetrieverRunner class looks OK to me, but I'd
> > like cfu and/or jmagne to check it and make sure we are calling the
> > right primitives to wrap/unwrap inside the cryptographic token.
> >
> > Also I'd like them to confirm that this would wor for an HSM.
> > Statements like the following make me question that:
> > CryptoToken token = manager.getInternalKeyStorageToken()
> >
> It won't work on HSM. Can I get an HSM to test with? ;) I've filed
> a ticket for HSM support[1]. FreeIPA does not yet support HSM[2] so
> I think we can put it in 10.4 milestone (I've put it there for now).
>
> [1] https://fedorahosted.org/pki/ticket/2292
> [2] https://fedorahosted.org/freeipa/ticket/5608
>
> > 4. Can you explain what happens if for some reason the script fails to
> > retrieve the key? Do we end up retrying later and if so, when?
> >
> If the script fails to retrieve the key, it does not retry
> automatically. I filed a ticket[3] to implement retry with
> backoff (this patchset is big enough already!) and put it in
> 10.3.1 milestone (that's up for discussion).
>
> [3] https://fedorahosted.org/pki/ticket/2293
>
> Right now, the following events cause authority reinitialisation,
> entailing key retrieval if necessary:
>
> - Dogtag is restarted
> - LDAP disconnect-reconnect
> - LDAP modification of authority replicated from another clone
>
> > 97- ACK
> >
> > 98 - ACK
> >
> Thanks. Any feedback on patch 0099?
>
> _______________________________________________
> Pki-devel mailing list
> Pki-devel(a)redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel
9:59 p.m.
On Fri, Apr 22, 2016 at 07:50:06PM -0400, John Magne wrote:
I took a look at the stuff alee asked for.
CFU even took a quick look when I asked her a couple of questions.
She was unsure of something (as was I) and she would like to be able
to take a closer look next week. I will give my quick thoughts.
1. I agree that HSM support is not in the patch, seems fine to move that
to a future ticket.
Here is one thing I was kind of worried about:
This is the code that imports the archive of the desired private key.
ublic static PrivateKey importPKIArchiveOptions(
+ CryptoToken token, PrivateKey unwrappingKey,
+ PublicKey pubkey, byte[] data)
+ throws InvalidBERException, Exception {
+ ByteArrayInputStream in = new ByteArrayInputStream(data);
+ PKIArchiveOptions options = (PKIArchiveOptions)
+ (new PKIArchiveOptions.Template()).decode(in);
+ EncryptedKey encKey = options.getEncryptedKey();
+ EncryptedValue encVal = encKey.getEncryptedValue();
+ AlgorithmIdentifier algId = encVal.getSymmAlg();
+ BIT_STRING encSymKey = encVal.getEncSymmKey();
+ BIT_STRING encPrivKey = encVal.getEncValue();
This the wrapper object that is build off of the caSigningUnit key gotten
in the other patch, the RetrieverThread like this:
PrivateKey unwrappingKey = hostCA.mSigningUnit.getPrivateKey();
The code below works fine if said key is RSA. I talked over with CFU and she said there
could be a chance this key is ECC for an ECC CA.
We both think the rest of the code in this routine is fine, except for possibly that.
She is also not even sure if JSS can support an ECC private key wrapper.
She requests you guys give her a day or two to look at it.
Except for the hsm issue, the code that calls this routine in the thread seems fine too.
+
+ KeyWrapper wrapper = token.getKeyWrapper(KeyWrapAlgorithm.RSA);
+ wrapper.initUnwrap(unwrappingKey, null);
+ SymmetricKey sk = wrapper.unwrapSymmetric(
+ encSymKey.getBits(), SymmetricKey.Type.DES3, 0);
+
+ ASN1Value v = algId.getParameters();
+ v = ((ANY) v).decodeWith(new OCTET_STRING.Template());
+ byte iv[] = ((OCTET_STRING) v).toByteArray();
+ IVParameterSpec ivps = new IVParameterSpec(iv);
+
+ wrapper = token.getKeyWrapper(KeyWrapAlgorithm.DES3_CBC_PAD);
+ wrapper.initUnwrap(sk, ivps);
+ PrivateKey.Type keyType = pubkey.getAlgorithm().equals("EC")
+ ? PrivateKey.Type.EC
+ : PrivateKey.Type.RSA;
+ return wrapper.unwrapPrivate(encPrivKey.getBits(), keyType, pubkey);
+ }
Pushed to master.
Christina, I know you were were/are very busy so thanks for spending
some time looking at these patches. If you have any other questions
or concerns let me know ASAP.
24992c089b9b5088f4481fda3d01a907565b5121 Lightweight CAs: authority schema changes
dc8c21cc9a68968a2b1db87f9b21cf3afbdb966a Add method CryptoUtil.importPKIArchiveOptions
e21aadd5e14dbcda73c20f20e67b1bcc8d5b5bfc Add ca-authority-key-export command
94ee373d053b34e534fbb61826e586693a38c934 Lightweight CAs: add key retrieval framework
a2a4117dbc7e489cbb1964d6ce5f95b786a03fde Lightweight CAs: add IPACustodiaKeyRetriever
Cheers,
Fraser
7:40 p.m.
On Thu, Apr 14, 2016 at 04:35:10PM -0400, Ade Lee wrote:
Still reviewing .. ACK on 87-95 (inclusive).
Thanks. I pushed 87..94 to master. I left 95 for now, in case of
further schema changes.
6d72a9c7fc067df42a3259fc5ea87b65e94f76ad Lightweight CAs: add exceptions for missing
signing key or cert
908c75dcefcb5030b2e3328835c506bf4c53704f Lightweight CAs: use static db connection
factory
536312af6798ca688556f559f8bdc76e2ba53e4d Lightweight CAs: avoid repeat definition of
authorities DN
18ed063edde8608f2ef30f62c118e24b835f1d83 Lightweight CAs: move host authority creation out
of load method
6a2195d6dab0dd4d20155177892af88aeb67d517 Lightweight CAs: extract LDAP commit/delete
methods
a39499f08966a517d52c97ef0cd54d8e6f098fb9 Lightweight CAs: monitor database for changes
28bc4ed903bc9e2618390ec412602d889e28354b Lightweight CAs: set DN based on data from LDAP
8f93e60e0057b0706c5d5ad762d7ff7ce20b7b39 Lightweight CAs: indicate when CA does not yet
have keys
Cheers,
Fraser
On Thu, 2016-04-14 at 16:18 +1000, Fraser Tweedale wrote:
> On Thu, Apr 14, 2016 at 09:04:31AM +1000, Fraser Tweedale wrote:
> > On Wed, Apr 13, 2016 at 05:26:44PM -0400, Ade Lee wrote:
> > > Still reviewing ..
> > >
> > > See comment on 87. ACK on 88,89,90,91,92,93, 94, 95.
> > >
> > > Ade
> > >
> > > On Mon, 2016-04-11 at 12:32 +1000, Fraser Tweedale wrote:
> > > > Thanks for review, Ade. Comments to specific feedback inline.
> > > > Rebased and updated patches attached. The substantive changes
> > > > are:
> > > >
> > > > - KeyRetriever implementations are now required NOT to import
> > > > the
> > > > key themselves. Instead the API is updated with
> > > > KeyRetriever.retrieveKey returning a Result, which contains
> > > > PKCS
> > > > #12 data and password for same.
> > > >
> > > > - KeyRetrieverRunner reads the Result and imports the PKCS #12
> > > > into
> > > > NSSDB.
> > > >
> > > > - Added new patch 0097 which provides the
> > > > IPACustodiaKeyRetriever
> > > > and assoicated Python helper script. It depends on an
> > > > unmerged
> > > > FreeIPA patch[1] as well as a particular principal and
> > > > associated
> > > > keytab and Custodia keys existing. I'm working on FreeIPA
> > > > updates
> > > > to satisfy these requirements automatically on install or
> > > > upgrade
> > > > but if you want to test this patch LMK and I'll provide
> > > > detailed
> > > > instructions.
> > > >
> > > > [1]
> > > > https://www.redhat.com/archives/freeipa-devel/2016-April/msg000
> > > > 55.html
> > > >
> > > > Other comments inline.
> > > >
> > > > Cheers,
> > > > Fraser
> > > >
> > > > On Fri, Apr 08, 2016 at 11:16:19AM -0400, Ade Lee wrote:
> > > > >
> > > > > 0087
> > > > >
> > > > > 1. In SigningUnit.java -- you catch an ObjectNotFound
> > > > > exception and
> > > > > rethrow that as a CAMissingKey exception. Is that the only
> > > > > way the
> > > > > ObjectNotFound exception can be thrown? What if the key is
> > > > > present
> > > > > but
> > > > > the cert is not? Can we refactor here to ensure that the
> > > > > correct
> > > > > exception is thrown?
> > > > >
> > > > One can't get additional info out of ObjectNotFound without
> > > > inspecting the String message, which I'm not comfortable doing.
> > > > The
> > > > key retrieval system should import key and cert at same time so
> > > > I've
> > > > renamed the exception to CAMissingKeyOrCert for clarity.
> > > >
> > >
> > > Well, you can always nest exceptions like so :
> > >
> > > mToken.login(cb); // ONE_TIME by default.
> > >
> > > try {
> > > mCert = mManager.findCertByNickname(mNickname);
> > > CMS.debug("Found cert by nickname: '" +
mNickname
> > > + "' with serial number: " + mCert.getSerialNumber());
> > >
> > > mCertImpl = new X509CertImpl(mCert.getEncoded());
> > > CMS.debug("converted to x509CertImpl");
> > > } catch (ObjectNotFoundException e) {
> > > throw new CAMissingCertException();
> > > }
> > >
> > > try {
> > > mPrivk = mManager.findPrivKeyByCert(mCert);
> > > CMS.debug("Got private key from cert");
> > > } catch (ObjectNotFoundException e) {
> > > throw new CAMissingKeyException();
> > > }
> > > ....
> > >
> > > The only reason that I suggest this is that I could imagine this
> > > kind
> > > of differentiation being useful in debugging failed custodia
> > > replications. If you think otherwise, I'm prepare to be
> > > convinced
> > > otherwise.
> > >
> > I think a scenario where we get key but not cert, or vice versa, is
> > unlikely (Custodia gives us a PKCS #12 file with both). However,
> > your suggestion should work and it is a relatively small change.
> > I'll cut a new patchset with this change today, along with the
> > rebase.
> >
> Updated and rebased patches attached.
>
> The suggested changes were made to 0087. This also resulted in
> changes to patch 0094 (indicate when CA does not yet have keys).
>
> No substantive changes to any other patches.
>
> Cheers,
> Fraser
4:30 p.m.
96-2 looks like it does not apply. Please rebase .
On Mon, 2016-04-11 at 12:32 +1000, Fraser Tweedale wrote:
Thanks for review, Ade. Comments to specific feedback inline.
Rebased and updated patches attached. The substantive changes are:
- KeyRetriever implementations are now required NOT to import the
key themselves. Instead the API is updated with
KeyRetriever.retrieveKey returning a Result, which contains PKCS
#12 data and password for same.
- KeyRetrieverRunner reads the Result and imports the PKCS #12 into
NSSDB.
- Added new patch 0097 which provides the IPACustodiaKeyRetriever
and assoicated Python helper script. It depends on an unmerged
FreeIPA patch[1] as well as a particular principal and associated
keytab and Custodia keys existing. I'm working on FreeIPA updates
to satisfy these requirements automatically on install or upgrade
but if you want to test this patch LMK and I'll provide detailed
instructions.
[1] https://www.redhat.com/archives/freeipa-devel/2016-April/msg000
55.html
Other comments inline.
Cheers,
Fraser
On Fri, Apr 08, 2016 at 11:16:19AM -0400, Ade Lee wrote:
>
> 0087
>
> 1. In SigningUnit.java -- you catch an ObjectNotFound exception and
> rethrow that as a CAMissingKey exception. Is that the only way the
> ObjectNotFound exception can be thrown? What if the key is present
> but
> the cert is not? Can we refactor here to ensure that the correct
> exception is thrown?
>
One can't get additional info out of ObjectNotFound without
inspecting the String message, which I'm not comfortable doing. The
key retrieval system should import key and cert at same time so I've
renamed the exception to CAMissingKeyOrCert for clarity.
> 0088:
>
> 2. What does dbFactory.reset() do and does it need to be called in
> a
> cleanup routine somewhere? Are we leaking resources?
>
> Answered I think on IRC. It just terminates any current
> connections -
> but do we need to call it on CA shutdown?
>
dbFactory.reset() is already called in the shutdown() method. (Only
the host authority calls it).
> 0089: ACK
>
> 0090: ACK
>
> 0091: ACK (with proviso below)
>
> 3. Not super-crazy about the names of the methods
> commitAuthority(),
> commitModifyAuthority and deleteAuthorityEntry(). They are not
> very
> consistent. I would suggest addAuthorityEntry(),
> modifyAuthorityEntry() and deleteAuthorityEntry() instead.
>
Done.
> 0092: ACK (with following proviso)
>
> 4. Talking with Nathan about this, he suggested that syncrepl is
> then
> more modern and preferred method to perform persistent searches.
> In
> fact, I see IPA tickets to replace persistent searches with
> syncrepl
> instead.
>
> We could replace the persistent search with a separate follow-on
> patch
> if you prefer, or just do it now.
>
Syncrepl is not supported by ldapjdk (iirc). If/when it is, and if
syncrepl provides a tangible advantage over persistent search in our
use case (where it can be assumed that disconnections from DS are
infrequent and brief, and full refresh of local view is tolerable),
then I am happy to change it - in a separate commit (because
LDAPProfileSubsystem is also affected).
> 0093 : ACK
>
> 0094: ACK
>
> 0095: ACK
>
> 0096: Looks good in general.
>
> 5. One thing to keep in mind though. It is perfectly possible to
> have
> more than one dogtag instance on a host. What determines the
> uniqueness of the instance is the host:port.
>
Noted. KeyRetriever implementations can access instance info via
the `CMS' class.
Cheers,
Fraser
2914
days inactive
2960
days old
22 comments
3 participants
participants (3)
-
Ade Lee -
Fraser Tweedale -
John Magne