Re: [Pki-devel] [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"
> To: "Ade Lee"
> Cc: pki-devel@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@redhat.com
> https://www.redhat.com/mailman/li
Re: [Pki-devel] [PATCH] 297, 298 add validity check for external CA
On 4/22/2016 2:37 PM, Ade Lee wrote:
commit 0fe7bf5ff989bbc24875dce30cec8f32e89c0a8f
Author: Ade Lee
Date: Fri Apr 22 15:31:43 2016 -0400
Add validity check for the signing certificate in pkispawn
When either an existing CA or external CA installation is
performed, use the pki-server cert validation tool to check
the signing certiticate and chain.
Ticket #2043
commit 9104fdda145c4f2bbbedec7256c73922e8bffcef
Author: Ade Lee
Date: Wed Apr 20 17:26:23 2016 -0400
Add CLI to check system certificate status
We add two different calls:
1. pki client-cert-validate - which checks a certificate in the client
certdb and calls the System cert verification call performed by JSS
in the system self test. This does some basic extensions and trust
tests, and also validates cert validity and cert trust chain.
2. pki-server subsystem-cert-validate
This calls pki client-cert-validate using the nssdb for the subsystem
on all of the system certificates by default (or just one if the
nickname is defined).
This is a great thing to call when healthchecking an instance,
and also will be used by pkispawn to verify the signing cert in the
externally signed CA case.
Trac Ticket 2043
In general it's ACKed. I have some minor comments/questions:
1. The SubsystemCertificateVerifier probably should be renamed to
SystemCertificateVerifier since "system certificate" refers to a cert in
the subsystem/instance's NSS database and "subsystem certificate" could
be confused with the "subsystemCert cert-pki-tomcat".
2. Instead of storing a shared SubsystemCertificateVerifier object in
the PKIDeployer object it might be better to create a factory method, so
the verifier can be used like this:
verifier = deployer.create_system_cert_verifier()
verifier.verify_certificate('signing')
That way the life-cycle of the verifier object will be short.
3. The .classpath got changed to point to a local path on your machine.
4. Is the "hardward-" name used consistently in our code?
passwd = instance.get_password("hardware-%s" % token)
--
Endi S. Dewata
___
Pki-devel mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
[Pki-devel] [PATCH] 297, 298 add validity check for external CA
commit 0fe7bf5ff989bbc24875dce30cec8f32e89c0a8f Author: Ade Lee Date: Fri Apr 22 15:31:43 2016 -0400 Add validity check for the signing certificate in pkispawn When either an existing CA or external CA installation is performed, use the pki-server cert validation tool to check the signing certiticate and chain. Ticket #2043 commit 9104fdda145c4f2bbbedec7256c73922e8bffcef Author: Ade Lee Date: Wed Apr 20 17:26:23 2016 -0400 Add CLI to check system certificate status We add two different calls: 1. pki client-cert-validate - which checks a certificate in the client certdb and calls the System cert verification call performed by JSS in the system self test. This does some basic extensions and trust tests, and also validates cert validity and cert trust chain. 2. pki-server subsystem-cert-validate This calls pki client-cert-validate using the nssdb for the subsystem on all of the system certificates by default (or just one if the nickname is defined). This is a great thing to call when healthchecking an instance, and also will be used by pkispawn to verify the signing cert in the externally signed CA case. Trac Ticket 2043From 0fe7bf5ff989bbc24875dce30cec8f32e89c0a8f Mon Sep 17 00:00:00 2001 From: Ade Lee Date: Fri, 22 Apr 2016 15:31:43 -0400 Subject: [PATCH 298/298] Add validity check for the signing certificate in pkispawn When either an existing CA or external CA installation is performed, use the pki-server cert validation tool to check the signing certiticate and chain. Ticket #2043 --- .../python/pki/server/deployment/pkihelper.py | 29 + .../server/deployment/scriptlets/configuration.py | 50 +++--- 2 files changed, 64 insertions(+), 15 deletions(-) diff --git a/base/server/python/pki/server/deployment/pkihelper.py b/base/server/python/pki/server/deployment/pkihelper.py index f01f6f69ff66d3687875c8f3d88840daf2115e3f..2c7ab0ef143839159887f595e9e9577f3fe0647d 100644 --- a/base/server/python/pki/server/deployment/pkihelper.py +++ b/base/server/python/pki/server/deployment/pkihelper.py @@ -4592,6 +4592,34 @@ class ConfigClient: return cert +class SubsystemCertificateVerifier: +""" Verifies system certificates for a subsystem""" + +def __init__(self, instance=None, subsystem=None): +self.instance = instance +self.subsystem = subsystem + +def verify_certificate(self, cert_id=None): +cmd = ['pki-server', 'subsystem-cert-validate', + '-i', self.instance.name, + self.subsystem] +if cert_id is not None: +cmd.append(cert_id) +try: +subprocess.check_output( +cmd, +stderr=subprocess.STDOUT) +except subprocess.CalledProcessError as e: +config.pki_log.error( +"pki subsystem-cert-validate return code: " + str(e.returncode), +extra=config.PKI_INDENTATION_LEVEL_2 +) +config.pki_log.error( +e.output, +extra=config.PKI_INDENTATION_LEVEL_2) +raise + + class PKIDeployer: """Holds the global dictionaries and the utility objects""" @@ -4621,6 +4649,7 @@ class PKIDeployer: self.systemd = Systemd(self) self.tps_connector = TPSConnector(self) self.config_client = ConfigClient(self) +self.subsystem_cert_verifier = SubsystemCertificateVerifier(self) def deploy_webapp(self, name, doc_base, descriptor): """ diff --git a/base/server/python/pki/server/deployment/scriptlets/configuration.py b/base/server/python/pki/server/deployment/scriptlets/configuration.py index 5f77ac52379be0ca08b1a5dff9f71626c731bd3f..42d6b401224a76c036307b33b2d1febd5400f452 100644 --- a/base/server/python/pki/server/deployment/scriptlets/configuration.py +++ b/base/server/python/pki/server/deployment/scriptlets/configuration.py @@ -88,7 +88,8 @@ class PkiScriptlet(pkiscriptlet.AbstractBasePkiScriptlet): instance = pki.server.PKIInstance(deployer.mdict['pki_instance_name']) instance.load() -subsystem = instance.get_subsystem(deployer.mdict['pki_subsystem'].lower()) +subsystem = instance.get_subsystem( +deployer.mdict['pki_subsystem'].lower()) token = deployer.mdict['pki_token_name'] nssdb = instance.open_nssdb(token) @@ -146,7 +147,8 @@ class PkiScriptlet(pkiscriptlet.AbstractBasePkiScriptlet): with open(external_csr_path) as f: signing_csr = f.read() -signing_csr = pki.nssdb.convert_csr(signing_csr, 'pem', 'base64') +signing_csr = pki.nssdb.convert_csr( +signing_csr, 'pem', 'base64') subsystem.config['ca.signing.certreq'] = signing_csr # This is n
[Pki-devel] [PATCH] 296 - fix cert requests problem ..
Fix problem in creating certificate requests
Some incorrect code was added to request processing
in the realm patches. In the request LDAP modification code,
if the realm was not present, we added a modification to remove the
realm attribute.
Unfortunately, if the realm was not present to begin with, this resulted
in LDAP returning a "No Such Attribute (16)" error, causing all kinds
of requests - including certificate requests to fail to be submitted.
At this point, we do not permit users to change the realm of a request.
Therefore, there is no reason to remove the realm. If we ever need
to do this in future, we'll have to be smarter about it.From 63032b005bf88313b08bb2c55fe1b6d114be708f Mon Sep 17 00:00:00 2001
From: Ade Lee
Date: Fri, 22 Apr 2016 14:22:16 -0400
Subject: [PATCH] Fix problem in creating certificate requests
Some incorrect code was added to request processing
in the realm patches. In the request LDAP modification code,
if the realm was not present, we added a modification to remove the
realm attribute.
Unfortunately, if the realm was not present to begin with, this resulted
in LDAP returning a "No Such Attribute (16)" error, causing all kinds
of requests - including certificate requests to fail to be submitted.
At this point, we do not permit users to change the realm of a request.
Therefore, there is no reason to remove the realm. If we ever need
to do this in future, we'll have to be smarter about it.
---
base/server/cmscore/src/com/netscape/cmscore/request/RequestRecord.java | 2 --
1 file changed, 2 deletions(-)
diff --git a/base/server/cmscore/src/com/netscape/cmscore/request/RequestRecord.java b/base/server/cmscore/src/com/netscape/cmscore/request/RequestRecord.java
index 074bff41c8090f6d998e3c879b06d3518550ce70..f617e26ae5dfe3b06fde2f52dffc29f83ba71e9c 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/request/RequestRecord.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/request/RequestRecord.java
@@ -200,8 +200,6 @@ public class RequestRecord
if (r.getRealm() != null) {
mods.add(IRequestRecord.ATTR_REALM, Modification.MOD_REPLACE, r.getRealm());
-} else {
-mods.add(IRequestRecord.ATTR_REALM, Modification.MOD_DELETE, null);
}
for (int i = 0; i < mRequestA.length; i++) {
--
2.4.3
___
Pki-devel mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
