[Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs
Martin Babinsky
mbabinsk at redhat.com
Mon Jun 13 10:48:18 UTC 2016
On 06/13/2016 08:59 AM, Jan Cholasta wrote:
> On 13.6.2016 08:38, Fraser Tweedale wrote:
>> On Fri, Jun 10, 2016 at 12:48:00AM +1000, Fraser Tweedale wrote:
>>> On Thu, Jun 09, 2016 at 12:36:35PM +0200, Jan Cholasta wrote:
>>>> On 9.6.2016 11:10, Fraser Tweedale wrote:
>>>>> On Thu, Jun 09, 2016 at 10:12:40AM +0200, Jan Cholasta wrote:
>>>>>> On 9.6.2016 08:44, Fraser Tweedale wrote:
>>>>>>> On Thu, Jun 09, 2016 at 01:21:29AM +1000, Fraser Tweedale wrote:
>>>>>>>> On Wed, Jun 08, 2016 at 01:00:36PM +0200, Jan Cholasta wrote:
>>>>>>>>> On 8.6.2016 05:15, Fraser Tweedale wrote:
>>>>>>>>>> On Tue, Jun 07, 2016 at 03:42:22PM +1000, Fraser Tweedale wrote:
>>>>>>>>>>> On Wed, Jun 01, 2016 at 02:51:04PM +1000, Fraser Tweedale wrote:
>>>>>>>>>>>> Hi team,
>>>>>>>>>>>>
>>>>>>>>>>>> This patchset implements the 'ca' plugin for creating and
>>>>>>>>>>>> managing
>>>>>>>>>>>> lightweight sub-CAs, and updates the 'caacl' plugin and
>>>>>>>>>>>> 'cert-request' command to support multiple CAs.
>>>>>>>>>>>>
>>>>>>>>>>>> A brief overview of the patches:
>>>>>>>>>>>>
>>>>>>>>>>>> 0059
>>>>>>>>>>>> 'ca' plugin, associated schema changes and container objects,
>>>>>>>>>>>> Dogtag REST API wrapper
>>>>>>>>>>>> 0060
>>>>>>>>>>>> Add CA entry for the IPA CA on install/upgrade
>>>>>>>>>>>> 0061
>>>>>>>>>>>> Update 'caacl' plugin with CA support (including enforcement)
>>>>>>>>>>>> 0062
>>>>>>>>>>>> Update ra.request_certificate() to support specifying
>>>>>>>>>>>> target CA
>>>>>>>>>>>> 0063
>>>>>>>>>>>> Add '--ca' option to 'cert-request' command
>>>>>>>>>>>> 0064
>>>>>>>>>>>> Add '--issuer' option to 'cert-find' command
>>>>>>>>>>>>
>>>>>>>>>>>> These patches depend on other pending patches:
>>>>>>>>>>>>
>>>>>>>>>>>> 0051, 0052, 0053, 0054, 0055, 0056
>>>>>>>>>>>>
>>>>>>>>>>>> Signing key replication depends on unmerged Dogtag patches.
>>>>>>>>>>>> Builds
>>>>>>>>>>>> of Dogtag with the required patches, and of FreeIPA with all
>>>>>>>>>>>> completed sub-CAs work, should be available from my COPR soon:
>>>>>>>>>>>> https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/
>>>>>>>>>>>>
>>>>>>>>>>>> Some parts of the design are not implemented in the current
>>>>>>>>>>>> patchset, including:
>>>>>>>>>>>>
>>>>>>>>>>>> - local parent CA (ipaca object) references
>>>>>>>>>>>> - sub-CA certificate renewal
>>>>>>>>>>>> - 'cert-show' command '--ca=NAME' option
>>>>>>>>>>>> - certmonger support for specifying CA
>>>>>>>>>>>> - revocation of deleted CAs
>>>>>>>>>>>>
>>>>>>>>>>>> I look forward to your reviews!
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Fraser
>>>>>>>>>>>>
>>>>>>>>>>> Rebased and updated patches attached.
>>>>>>>>>>>
>>>>>>>>>>> Substantive changes:
>>>>>>>>>>>
>>>>>>>>>>> - add required attributes for issuer DN and subject DN
>>>>>>>>>>> - prevent rename of IPA CA
>>>>>>>>>>> - when adding IPA CA entry, contact Dogtag to learn authority
>>>>>>>>>>> id,
>>>>>>>>>>> issuer DN and subject DN
>>>>>>>>>>> - add 'read_ca' method to Dogtag interface
>>>>>>>>>>> - tighten ACIs to prevent modification of ipacaid attribute
>>>>>>>>>>>
>>>>>>>>>> Updated patch 0064-3; adds --issuer option to cert-show and --ca
>>>>>>>>>> option to cert-show and cert-find.
>>>>>>>>>
>>>>>>>>> Patch 0059:
>>>>>>>>>
>>>>>>>>> 1) On upgrade, why is the lightweight CA container created
>>>>>>>>> twice - once in
>>>>>>>>> 41-subca.update, once using ensure_entry() call? It should be
>>>>>>>>> done only
>>>>>>>>> once.
>>>>>>>>>
>>>>>>>> I'll remove 41-subca.update; the routine in cainstance is the one
>>>>>>>> that's needed.
>>>>>>>>
>>>>>>>>> 2) In ca_del, every CA specified in args[0] should be deleted,
>>>>>>>>> not just the
>>>>>>>>> first one.
>>>>>>>>>
>>>>>>>>> 3) Do not use NonFatalError, issue a warning instead:
>>>>>>>>>
>>>>>>>>> self.add_message(MyNewWarningClass(name=...))
>>>>>>>>>
>>>>>>>>> 4) Can it actually happen that ca_show does not return ipacaid?
>>>>>>>>> I guess not,
>>>>>>>>> so you should be able to remove the check altogether and don't
>>>>>>>>> bother with
>>>>>>>>> the warning.
>>>>>>>>>
>>>>>>>> ipacaid is mandatory now, so I'll remove the check.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Patch 0060-0062: LGTM
>>>>>>>>>
>>>>>>>> Yippee \o/
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Patch 0063:
>>>>>>>>>
>>>>>>>>> Could you please define the CA param as follows:
>>>>>>>>>
>>>>>>>>> Str('cacn?',
>>>>>>>>> cli_name='ca',
>>>>>>>>> query=True,
>>>>>>>>> label=_("CA"),
>>>>>>>>> doc=_("CA to use"),
>>>>>>>>> ),
>>>>>>>>>
>>>>>>>>> ?
>>>>>>>>>
>>>>>>>>> This is for consitency with framework-generated parent key
>>>>>>>>> params, which
>>>>>>>>> unfortunately we cannot leverage in cert_request currently.
>>>>>>>>>
>>>>>>>> No problemo.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Patch 0064:
>>>>>>>>>
>>>>>>>>> 1) See my comment for patch 0063, it applies here as well.
>>>>>>>>>
>>>>>>>>> 2) The --issuer option should not be included in cert_show -
>>>>>>>>> show commands
>>>>>>>>> are supposed to retrieve an object given primary key(s), and
>>>>>>>>> the primary key
>>>>>>>>> of CA objects is just their cn.
>>>>>>>>>
>>>>>>>> The --issuer argument is because primary key for a cert is really
>>>>>>>> (issuer, serial). So it show the cert _with_ that issuer (and
>>>>>>>> serial), not the cert _for_ that issuer.
>>>>>>
>>>>>> Correct, but in IPA the issuer is represented by the CA object, so
>>>>>> in IPA
>>>>>> the primary key for a certificate is actually (CA name, serial).
>>>>>>
>>>>>> Certificate lookup by issuer name and serial is actually a search
>>>>>> operation,
>>>>>> analogical to how CA lookup by subject name is also a search
>>>>>> operation, so
>>>>>> it should be done by cert-find.
>>>>>>
>>>>> OK, I will remove the --issuer option for cert-find.
>>>>>
>>>>>>>>
>>>>>>>>> 3) In find commands, the options form a filter, so instead of
>>>>>>>>> raising
>>>>>>>>> MutuallyExclusiveError in cert-find, return an empty result, as
>>>>>>>>> with any
>>>>>>>>> other unmatched filter.
>>>>>>>>>
>>>>>>>> Here, --issuer and --ca are two different ways to specify the
>>>>>>>> issuer. --issuer lets you give the issuer DN straight up; --ca
>>>>>>>> takes the name of an IPA CA object and looks up its issuer DN.
>>>>>>>> (Thus it makes no sense to give both options at once).
>>>>>>
>>>>>> That's one way to look at it, but it's true only if you assume that
>>>>>> cert-find can only search certificates in Dogtag. This will very
>>>>>> soon became
>>>>>> untrue, as we will allow cert-find to also search certificates
>>>>>> anywhere in
>>>>>> LDAP (the server part of ticket #5381). There, the difference
>>>>>> between the
>>>>>> options would be that with --ca you search for certificates issued
>>>>>> by the
>>>>>> specified managed CA, but with --issuer you search for
>>>>>> certificates with the
>>>>>> given issuer name, be it managed CA or not.
>>>>>>
>>>>> --ca is just a "shorthand" for --issuer - it merely looks up subject
>>>>> DN of the specified CA, and uses that as the issuer option.
>>>>>
>>>>>> For now, IMO the correct behavior should be that if both are
>>>>>> specified and
>>>>>> the issuer name of the specified CA does not match the specified
>>>>>> issuer
>>>>>> name, empty result is returned, otherwise carry on with the search in
>>>>>> Dogtag.
>>>>>>
>>>>> If I allow both, the behaviour will then be:
>>>>>
>>>>> specify issuer DN only)
>>>>> search using given issuer DN
>>>>> specify CA only)
>>>>> search using subject DN of specified CA. If no such CA, error.
>>>>> specify issuer DN and CA)
>>>>> search using given issuer DN, and ensure that result (if any)
>>>>> matches subject DN of specified CA. If no such CA, error.
>>>>>
>>>>> I'm happy to implement this if you confirm that you think it's the
>>>>> correct behaviour.
>>>>
>>>> Looks correct to me.
>>>>
>>> Thanks; updated patches attached. Martin, this patch should also
>>> fix the upgrade issue.
>>>
>>> Cheers,
>>> Fraser
>>>
>> Another rebase to fix conflicts in VERSION file. No other changes.
>
> Thanks.
>
> The remaining cert commands (status, revoke, remove-hold) should also
> have the cacn option consistent with cert-show. This can be added in
> further patch, though.
>
> I think it would make sense if the cn argument in ca commands was
> optional, with the default being 'ipa'. This can also be done later.
>
> So LGTM. If Martin agrees, we can push this patch set.
>
Hi Fraser,
during functional review I found the following issues:
1.)
If I create a CAACL rule tied to a specific sub-CA let's say for user
certificate issuance:
"""
ipa caacl-show user_cert_issuance
Enabled: TRUE
User category: all
CAs: user_sub_ca
Profiles: caIPAuserCert
ACL name: user_cert_issuance
"""
I can still happily request certificate for a user using root-CA:
"""
ipa cert-request cert.csr --principal jdoe --ca ipa
Certificate: MIID9j.../Ov8mkjFA==
Subject: CN=jdoe,O=IPA.TEST
Issuer: CN=Certificate Authority,O=IPA.TEST
...
"""
should not this be denied by CA-ACL rule?
The default IPA CAACL rule is like this:
"""
ipa caacl-show hosts_services_caIPAserviceCert
Enabled: TRUE
Host category: all
Service category: all
Profiles: caIPAserviceCert
ACL name: hosts_services_caIPAserviceCert
"""
so the default rule should not allow users to request certs at all.
2.)
The '--chain' option in cert-show/cert-request is not implemented, but
this was probably a deliberate decision. We should however at least open
a ticket about this and add it later.
3.)
I cannot issue a user certificate on a replica with configured CA:
"""
[root at replica1 ~]# ipa cert-request cert.csr --ca user_sub_ca
--principal jrogan
ipa: ERROR: Certificate operation cannot be completed: FAILURE (CA not
found: bc953851-1f24-4e16-9847-9ee5bbe6dd0e)
[root at replica1 ~]# ipa ca-find user_sub_ca
------------
1 CA matched
------------
Name: user_sub_ca
Description: CA for issuing user certificates
Authority ID: bc953851-1f24-4e16-9847-9ee5bbe6dd0e
Subject DN: CN=User Cert Sub-CA,O=IPA.TEST
Issuer DN: CN=Certificate Authority,O=IPA.TEST
----------------------------
Number of entries returned 1
----------------------------
[root at replica1 ~]# ipa cert-request cert.csr --ca user_sub_ca
--principal jrogan
ipa: ERROR: Certificate operation cannot be completed: FAILURE (CA not
found: bc953851-1f24-4e16-9847-9ee5bbe6dd0e)
"""
The LDAP entry is there, but it seems like the sub-CA was not replicated
at the dogtag backend. This occurs regardless of whether the master was
freshly installed or upgraded from 4.3.1 release.
--
Martin^3 Babinsky
More information about the Freeipa-devel
mailing list