[Freeipa-devel] [PATCH 0004-0012] Automatic CSR generation

Wed Aug 3 13:21:58 UTC 2016

On 08/01/2016 11:57 PM, Fraser Tweedale wrote:
> On Fri, Jul 29, 2016 at 11:13:16AM -0400, Ben Lipton wrote:
>> On 07/29/2016 09:39 AM, Petr Spacek wrote:
>>> On 27.7.2016 19:06, Ben Lipton wrote:
>>>> Hi all,
>>>>
>>>> I think the automatic CSR generation feature
>>>> (https://fedorahosted.org/freeipa/ticket/4899,
>>>> http://www.freeipa.org/page/V4/Automatic_Certificate_Request_Generation) is
>>>> stable enough to review now. The following are summaries of the attached patches:
>>>> 0004: LDAP schema changes for the new feature
>>>> 0005: Basic API for new objects and CSR generation
>>>> 0006: Update install automation to create some default mapping rules
>>>> 0007: Implement the lookups and text processing that generates the CSR config
>>>> 0008 and 0009: Implement some actual transformation rules so that the feature
>>>> is usable
>>>> 0010: Add a new cert profile for user certs, with mappings
>>>> 0011: Implement import/export of cert profiles with mappings
>>>> 0012: Tests for profile import/export
>>>>
>>>> Generally speaking, later patches depend on earlier ones, but I don't
>>>> anticipate any problems from committing earlier patches without later ones.
>>>>
>>>> If you prefer, you can also comment on the pull request version:
>>>> https://github.com/LiptonB/freeipa/pull/4. Note that I may force push on this
>>>> branch.
>>>>
>>>> Allocation of OIDs for schema change also needs review:
>>>> https://code.engineering.redhat.com/gerrit/#/c/80061/
>>>>
>>>> Known issues:
>>>> - When the requested principal does not have some of the requested data,
>>>> produces funny-looking configs with extra commas, empty sections, etc. They
>>>> are still accepted by my copies of openssl and certutil, but they look ugly.
>>>> - The new objects don't have any ACIs, so for the moment only admin can run
>>>> the new commands.
>>>> - Does not yet have support for prompting user for field values, so currently
>>>> all data must come from the database.
>>>> - All processing happens on the server side. As discussed in a previous
>>>> thread, it would be desirable to break this out into a library so it could be
>>>> used client-side.
>>>>
>>>> Very excited to hear your thoughts!
>>> Hi Ben,
>>>
>>> I wanted to give it a try from user's point of view first, before diving into
>>> implementation details. Here are some observations:
>> Thanks for giving it a try! This is great feedback.
>>> 0. Design pages are using term "helper" and it is used even as option in the
>>> example with smartcards. Please fix either design page or the code so they are
>>> consistent.
>> Good point. In a previous discussion, Alexander remarked that --helper
>> sounded too low-level, but I find that --use sounds very generic and
>> --format doesn't make a lot of sense for ipa cert-request, which never
>> actually gives you the config that's generated. So if they're all going to
>> be the same word, which is probably a good idea, I might be leaning towards
>> --helper, but I'm interested to hear opinions on this.
>>> 1. The "ipa cert-request" command is missing options --autofill and --use (aka
>>> helper aka format :-) which are mentioned in the design pages.
>> Yeah, I haven't managed to implement much of the UI niceness suggested by
>> the design page. I probably should have mentioned that in the email - all
>> that I expect to be working at this point is 'ipa cert-get-requestdata' and
>> CRUD for the mapping rules/profiles themselves.
>>> 2. "ipa cert_get_requestdata" command passes even without --profile-id and
>>> generates empty config. I think that this is not expected :-)
>> More expected than you might think :) I'm guessing what's happening is that
>> you're passing a user principal and it's defaulting to the caIPAserviceCert
>> profile, then failing to fill out any of the fields because the data it
>> needs is not there. I agree this isn't great. I was planning to address this
>> by having it throw an error if it can't generate at least the subject of the
>> request, and maybe suggest using a different profile.
>>
>> I chose to have it default to caIPAserviceCert because that's what ipa
>> cert-request does, but maybe that's not as predictable as I thought.
>>
> In general use I think that 'caIPAserviceCert' is unlikely to be
> used a majory of the time, and it is a new command so there are no
> compatibility issues; therefore why not make the profile option
> mandatory?
Fair enough. Ok, a modified patch that changes this (and fixes some 
label errors I noticed) is attached.
>
>>> 3. "ipa cert_get_requestdata --format=openssl" prints the text to stdout
>>> including label "Configuration file contents:". This is hard to use because
>>> simple redirection like "ipa cert_get_requestdata --format=openssl > config"
>>> will not give you valid OpenSSL config file - it needs hand-editing.
>>>
>>> It would be good to add --out parameter you envisioned in the design page.
>>> Please ask jcholast for proper name and semantics :-) With --out option the
>>> command can be used to generate valid config (or script if certutil was selected).
>> Agreed. Another example of the UI not being quite right yet. I've been
>> unsure how to handle this, because of certutil taking a command line and
>> openssl a config file. It actually gets even more complicated because, as
>> you point out in the next item, openssl also needs a command in addition to
>> the config file. I'm interested in thinking about how to handle this cleanly
>> from a user perspective. Generating a script, or providing the command lines
>> as hints, might be ways to get around these concerns.
>>> 4. "ipa cert_get_requestdata --format=openssl" could print hint what OpenSSL
>>> command should be executed on the generated config file. For testing I've used
>>> command "openssl req -new -out csr -text -config config" (stolen and modified
>>> from smart card example). Also, as a second hint, it could print the IPA
>>> command which needs to be used to sign the CSR generated by the helper.
>> Also agreed, the framework should be able to generate and (for purposes of
>> 'ipa cert-request --autofill') even execute the command required to make the
>> CSR.
>>
>>> 5. My naive attempt to get userCert for admin failed:
>>> $ ipa cert_get_requestdata --format=openssl --principal=admin
>>> --profile-id=userCert > usercert.conf
>>> # remove the trailing label
>>> $ openssl req -new -out csr -text -config usercert.conf
>>> $ ipa cert-request --principal=admin --profile-id=userCert usercert.csr
>>> ipa: ERROR: invalid 'csr': No Common Name was found in subject of request.
>>>
>>> I'm attaching files generated by the commands above. I did not modify anything
>>> in the templates or profiles, just tried to use the new profile added by
>>> freeipa-blipton-0010-Add-a-new-cert-profile-for-users.patch .
>> Hah! This is what I get for thinking I know what the output has to look
>> like, and not testing all the way through to requesting the cert. I'll
>> change the profile to generate a subject with CN= instead of UID=. Updated
>> patch is attached. Unfortunately these rules are only updated at
>> ipa-server-install time, so if you'd like to fix it without reinstalling:
>>
> (Tangential commentary...) Yeah, currently cert-request demands the
> CN.  There is a design to relax the requirement to handle empty
> subject names (look at SAN only).  IMO it would make sense to accept
> other "obvious" mappings in Subject DN like accepting UID instead of
> CN for user subjects, but that would be a separate RFE.  Noone has
> actually asked for it yet :)
>
> Cheers,
> Fraser
>
>> ipa certtransformationrule-mod dataUsernameCN dataUsernameCertutil
>> --template 'CN={{ipa.datafield(subject.uid.0)|quote}},{{ipa.datafield(config.ipacertificatesubjectbase.0)|quote}}'
>> ipa certtransformationrule-mod dataUsernameCN dataUsernameOpenssl --template
>> '{{ipa.datafield(config.ipacertificatesubjectbase.0)}}
>> CN={{ipa.datafield(subject.uid.0)}}'
>> Yes, that must be an actual linebreak between the subjectbase and the CN in
>> the openssl one. I know :-/
>>
>>> Hopefully I will get to other things soon (but not this week).
>>>
>>>
>>> Anyway, all this seems like (expected) initial problems. In general the first
>>> touch with user interface seems reasonable and needs only small improvements.
>>>
>>> Good work!
>>>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-blipton-0005-2-Add-plugin-for-CSR-generation.patch
Type: text/x-patch
Size: 20499 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160803/51deddf4/attachment.bin>