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

Fri Jul 29 15:13:16 UTC 2016

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.
>
> 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:

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-0010-2-Add-a-new-cert-profile-for-users.patch
Type: text/x-patch
Size: 9454 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160729/8ae3d9eb/attachment.bin>