[Freeipa-devel] CSR autogeneration next steps

Fri Dec 16 04:11:05 UTC 2016

On 12/12/2016 03:52 AM, Jan Cholasta wrote:
> On 5.12.2016 16:48, Ben Lipton wrote:
>> Hi Jan, thanks for the comments.
>>
>>
>> On 12/05/2016 04:25 AM, Jan Cholasta wrote:
>>> Hi Ben,
>>>
>>> On 3.11.2016 00:12, Ben Lipton wrote:
>>>> Hi everybody,
>>>>
>>>> Soon I'm going to have to reduce the amount of time I spend on new
>>>> development work for the CSR autogeneration project, and I want to 
>>>> leave
>>>> the project in as organized a state as possible. So, I'm taking
>>>> inventory of the work I've done in order to make sure that what's 
>>>> ready
>>>> for review can get reviewed and the ideas that have been discussed get
>>>> prototyped or at least recorded so they won't be forgotten.
>>>
>>> Thanks, I have some questions and comments, see below.
>>>
>>>>
>>>> Code that's ready for review (I will continue to put in as much 
>>>> time as
>>>> needed to help get these ready for submission):
>>>>
>>>> - Current PR: https://github.com/freeipa/freeipa/pull/10
>>>
>>> How hard would it be to update the PR to use the "new" interface from
>>> the design thread? By this I mean that currently there is a command
>>> (cert_get_requestdata), which creates a CSR from profile id +
>>> principal + helper, but in the design we discussed a command which
>>> creates a CertificationRequestInfo from profile id + principal +
>>> public key.
>>>
>>> Internally it could use the OpenSSL helper, no need to implement the
>>> full "new" design. With your build_requestinfo.c code below it looks
>>> like it should be pretty straightforward.
>>
>> This is probably doable with the cffi, but I'm concerned about
>> usability. A user can run the current command to get a (reusable)
>> script, and run the script to get a CSR. It works with keys in both PEM
>> files and NSS databases already. If we change to outputting a
>> CertificationRequestInfo, in order to make this usable on the command
>> line, we'll need:
>> - An additional tool to sign a CSR given a CertificationRequestInfo (for
>> both types of key storage).
>> - A way to extract a SubjectPublicKeyInfo structure from a key within
>> the ipa command (like [1] but we need it for both types of key storage)
>> Since as far as I know there's no standard encoding for files containing
>> only a CertificationRequestInfo or a SubjectPublicKeyInfo, we'll be
>> writing and distributing these ourselves. I think that's where most of
>> the extra work will come in.
>
> For PEM files, this is easily doable using python-cryptography (to 
> extract SubjectPublicKeyInfo and sign CertificationRequestInfo) and 
> PyASN1 (to create a CSR from the CertificationRequestInfo and the 
> signature).

I didn't realize that python-cryptography knew about 
SubjectPublicKeyInfo structures, but indeed this seems to be pretty 
straightforward:

key = load_pem_private_key(key_bytes, None, default_backend())
pubkey_info = key.public_key().public_bytes(Encoding.DER, 
PublicFormat.SubjectPublicKeyInfo)

Thanks for letting me know this functionality already existed.
>
> For NSS databases, this will be trickier and will require calling C 
> functions, as neither certutil nor python-nss provide a way to a) 
> address existing keys in the database by key ID b) get 
> SubjectPublicKeyInfo for a given key.
>
> As for encoding, the obvious choice is DER. It does not really matter 
> there is no standard file format, as we won't be transferring these as 
> files anyway.

Agreed. I just meant there aren't tools already because this isn't a 
type of file one often needs to process.
>
>>
>> Would it be ok to stick with the current design in this PR? I'd feel
>> much better if we could get the basic functionality into the repo and
>> then iterate on it rather than changing the plan at this point. I can
>> create a separate PR to change cert_get_requestdata to this new
>> interface and at the same time add the necessary adapters (bullet points
>> above) to make it user-friendly.
>
> Works for me.

Updated the PR to fix conflicts with master. Had some trouble with CI 
but creating a new PR with the same commits fixed it 
(https://github.com/freeipa/freeipa/pull/337). Not sure if it's fixed 
permanently, so I guess I'll just keep the two PRs synchronized now, or 
we could close the old one.
>
>>
>> I would probably just implement the adapters within the
>> cert_build/cert_request client code unless you think having standalone
>> tools is valuable. I suppose certmonger is going to need these features
>> too, but I don't know how well sharing code between them is going to 
>> work.
>
> cert-request is exactly the place where it should be :-) I wouldn't 
> bother with certmonger until we have a server-side csrgen.
>
>>>
>>>>
>>>> - Allow some fields to be specified by the user at creation time:
>>>> https://github.com/LiptonB/freeipa/commits/local-user-data
>>>
>>> Good idea :-)
>>>
>>>>
>>>> - Automation for the full process from getting CSR data to requesting
>>>> cert: https://github.com/LiptonB/freeipa/commits/local-cert-build
>>>
>>> LGTM, although I would prefer if this was a client-side extension of
>>> cert-request rather than a completely new command.
>>
>> I did try that at first, but I struggled to figure out the interface for
>> the modified cert-request. (Not that the current solution is so great,
>> what with the copying of options from cert_request and certreq.) If I
>> remember correctly, I was uncertain how to implement parameters that are
>> required/invalid based on other parameters: the current cert-request
>> takes a signed CSR (required), a principal (required), and a profile ID;
>> the new cert-request (what I implemented as cert-build) takes a
>> principal (required), a profile ID (required), and a key location
>> (required). I can't remember if that was the only problem, but I'll try
>> again to merge the commands and get back to you.
>
> To make the CSR argument optional on the client, you can do this:
>
>     def get_options(self):
>         for option in super(cert_request, self).get_options():
>             if option.name == 'csr':
>                 option = option.clone(required=False)
>             yield
>
> IMO profile ID should default to caIPAserviceCert on the client as well.

I originally had it doing so, but changed it to a required option based 
on feedback in this email: 
https://www.redhat.com/archives/freeipa-devel/2016-August/msg00021.html: 
"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?" I guess since we're talking about cert-request now, the 
compatibility issues are back.

https://github.com/LiptonB/freeipa/commits/local-cert-build has now been 
updated to change the cert_request command rather than adding a new 
command. It seems to work now (thanks for the advice on making the 
argument optional), the only thing I'm having trouble with is the 
default for the profile_id argument. Previously, the default was applied 
by this code in cert_request.execute:

profile_id = kw.get('profile_id', self.Backend.ra.DEFAULT_PROFILE)

But now, in the client, I need the default to pass to 
cert_get_requestdata if no profile is specified. I'm not sure I can 
access backends from the client to get it the same way the server code 
does. Should I just import ipapython/dogtag.py and use the 
DEFAULT_PROFILE set in there? Is there a way I can give the option a 
default that will be seen in both the server and the client?
>
>>>
>>>>
>>>> Other prototypes and design ideas that aren't ready for submission 
>>>> yet:
>>>>
>>>> - Utility written in C to build a CertificationRequestInfo from a
>>>> SubjectPublicKeyInfo and an openssl-style config file. The purpose of
>>>> this is to take a config that my code already knows how to 
>>>> generate, and
>>>> put it in a form that certmonger can use. This is nearly done and
>>>> available at:
>>>> https://github.com/LiptonB/freeipa-prototypes/blob/master/build_requestinfo.c 
>>>>
>>>>
>>>
>>> Nice! As I said above, this could really make implementing the "new"
>>> csrgen interface simple.
>>>
>>>>
>>>>
>>>> - Ideally it should be possible to use this tool to reimplement the 
>>>> full
>>>> cert-request automation (local-cert-build branch) without a dependency
>>>> on the certutil/openssl tools. However, I don't think any of the 
>>>> python
>>>> crypto libraries have bindings for the functions that deal with
>>>> CertificationRequestInfo objects, so I don't think I can do this in 
>>>> the
>>>> short term.
>>>
>>> You can use python-cffi to write your own minimal bindings. It's
>>> fairly straightforward, take a look at FreeIPA commit 500ee7e2 for an
>>> example of how to port C code to Python with python-cffi.
>>
>> Thank you for the example. I will take a look.
>>>
>>>>
>>>> - Certmonger "helper" program that takes in the 
>>>> CertificationRequestInfo
>>>> that certmonger generates, calls out to IPA for profile-specific data,
>>>> and returns an updated CertificationRequestInfo built from the data.
>>>> Certmonger doesn't currently support this type of helper, but (if I
>>>> understood correctly) this is the architecture Nalin believed would be
>>>> simplest to fit in. This is not done yet, but I intend to complete it
>>>> soon - it shouldn't require much code beyond what's in
>>>> build_requestinfo.c.
>>>
>>> To me this sounds like it should be a new operation of the current
>>> helper rather than a completely new helper.
>>
>> Maybe so. I certainly wouldn't call this a finished design, I just
>> wanted to have some kind of proof of concept for how the certmonger
>> integration could work. For what it's worth, that prototype is now
>> available at [2].
>
> OK.
>
>>>
>>> Anyway, the ultimate goal is to move the csrgen code to the server,
>>> which means everything the helper will have to do is call a command
>>> over RPC.
>>>
>>>>
>>>> - Tool to convert an XER-encoded cert extension to DER, given the 
>>>> ASN.1
>>>> description of the extension. This would unblock Jan Cholasta's 
>>>> idea of
>>>> using XSLT for templates rather than text-based formatting. I 
>>>> should be
>>>> able to implement the conversion tool, but it may be a while before I
>>>> have time to demo the full XSLT idea.
>>>
>>> Was there any progress on this?
>>
>> I have started working on implementing it with asn1c, and I'm already
>> seeing some of the inconvenience (security issues aside) of building on
>> the server. Libtasn1 seems like a much better model, but doesn't seem to
>> have XER support. Anyway, don't quite have results here yet but I think
>> I should have the XER->DER demo with asn1c ready in a week or two.
>
> Implementing XER codec on top of libtasn1 shouldn't be too hard; I 
> have a WIP which I will post soon.
>
>>>
>>>>
>>>> So: currently on my to do list are the certmonger helper and the
>>>> XER->DER conversion tool. Do you have any comments about these plans,
>>>> and is there anything else I can do to wrap up the project neatly?
>>>>
>>>> Thanks,
>>>> Ben
>>>>
>>>
>>> Honza
>>>
>> [1] https://github.com/LiptonB/freeipa-prototypes/blob/master/key2spki.c
>> [2]
>> https://github.com/LiptonB/freeipa-prototypes/blob/master/cm_ipa_csrgen.c 
>>
>