[Freeipa-devel] CSR autogeneration next steps
Ben Lipton
blipton at redhat.com
Sat Feb 4 15:40:51 UTC 2017
On 01/12/2017 04:35 AM, Jan Cholasta wrote:
> On 11.1.2017 00:38, Ben Lipton wrote:
>>
>> On 01/10/2017 01:58 AM, Jan Cholasta wrote:
>>> On 19.12.2016 21:59, Ben Lipton wrote:
>>>>
>>>> On 12/15/2016 11:11 PM, Ben Lipton wrote:
>>>>>
>>>>> 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.
>>
>> I'm currently working on the step of signing the
>> CertificationRequestInfo and creating a CSR from it. I think I have it
>> working with pyasn1, but of course the "signature algorithm" for the CSR
>> needs to be specified and implemented within the code since I'm not
>> using a library that understands CSRs natively. The code I have
>> currently always produces CSRs with the sha256WithRSAEncryption
>> algorithm (DER-encode request info, SHA256, PKCS #1v1.5 padding, RSA
>> encryption), and the OID for that algorithm is hardcoded in the output
>> CSR. Is this ok or will we need more flexibility than that?
>
> IMO it's OK for starters.
>
>>>>>>
>>>>>> 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.
>>>
>>> This can be worked around by:
>>>
>>> 1. Generating a key + temporary certificate:
>>>
>>> n=$(head -c 40 /dev/urandom | base32)
>>> certutil -S -n $n -s CN=$n -x -t ,,
>>>
>>> 2. Extracting the public key from the certificate:
>>>
>>> certutil -L -n $n -a >temp.crt
>>> (extract the public key using python-cryptography)
>>>
>>> 3. Deleting the temporary certificate:
>>>
>>> certutil -D -n $n
>>>
>>> 4. Importing the newly issued certificate:
>>>
>>> certutil -A -n $n -t ,, -a <new.crt
>>>
>> Oof, thanks, I'm not sure I would have been able to come up with that.
>> Can you generate a key without a temporary certificate if you use the
>> NSS API, or does their model require every key to belong to a cert?
>
> I'm pretty sure it's possible, but it certainly won't be as simple as
> this. I gave up after a few hours of digging into NSS source code and
> not being able to figure out how.
>
>>>>>>
>>>>>> 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.
>>>
>>> You can close the old one.
>>>
>>> Just to make sure we are on the same page, you want this PR to be
>>> merged before submitting additional PRs built on top of it?
>>
>> Yes, I would like to merge this one to have as a starting point if
>> you're comfortable with it: https://github.com/freeipa/freeipa/pull/337.
>> I just did a force push to clean up the history, but the final diff
>> should be the same as it was before.
>
> OK.
>
>>>
>>>>>>
>>>>>>>
>>>>>>> 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?
>> Just wanted to call attention to this question. The code that's
>> currently problematic is here:
>> https://github.com/LiptonB/freeipa/blob/dda05b0b4dfa332569a8ca75632eaeceb95fbd6a/ipaclient/plugins/cert.py#L86
>>
>> (will pass None when in fact the argument default should be used).
>
> self.get_default_of('profile_id')
>
>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> 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.
>>>>
>>>> It took me some experimentation to get this to work, but the solution
>>>> with asn1c is actually quite simple because the tool automatically
>>>> provides a sample C file that converts between different formats. So,
>>>> this very basic shell script is able to do the conversion:
>>>> https://github.com/LiptonB/freeipa-prototypes/blob/master/xer2der.sh
>>>>
>>>> $ cat ExtKeyUsage.xer
>>>> <ExtKeyUsageSyntax>
>>>> <KeyPurposeId>1.3.6.1.5.5.7.3.2</KeyPurposeId>
>>>> <KeyPurposeId>1.3.6.1.5.5.7.3.4</KeyPurposeId>
>>>> </ExtKeyUsageSyntax>
>>>>
>>>> $ cat KeyUsage.asn1
>>>> KUModule DEFINITIONS ::=
>>>> BEGIN
>>>>
>>>> KeyUsage ::= BIT STRING {
>>>> digitalSignature (0),
>>>> nonRepudiation (1), -- recent editions of X.509 have
>>>> -- renamed this bit to
>>>> contentCommitment
>>>> keyEncipherment (2),
>>>> dataEncipherment (3),
>>>> keyAgreement (4),
>>>> keyCertSign (5),
>>>> cRLSign (6),
>>>> encipherOnly (7),
>>>> decipherOnly (8) }
>>>>
>>>> ExtKeyUsageSyntax ::= SEQUENCE SIZE (1..MAX) OF KeyPurposeId
>>>>
>>>> KeyPurposeId ::= OBJECT IDENTIFIER
>>>>
>>>> END
>>>>
>>>> $ ./xer2der.sh KeyUsage.asn1 ExtKeyUsageSyntax ExtKeyUsage.xer
>>>> 2>/dev/null | xxd
>>>> 00000000: 3014 0608 2b06 0105 0507 0302 0608 2b06 0...+.........+.
>>>> 00000010: 0105 0507 0304 ......
>>>
>>> So far I don't have a working example using libtasn1. I have something
>>> close to it, but it's hacky, as the libtasn1 API is pretty limited,
>>> and I didn't have time to work on it in the last few weeks.
>
> I got it working, needs just a little polishing. It's still ugly hacky
> though.
>
>>>
>>>>
>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> 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
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>
Thank you for the review! I just created
https://github.com/freeipa/freeipa/pull/433 and
https://github.com/freeipa/freeipa/pull/434 for the two follow-up
branches I had pending (and updated with ideas from this thread and the
previous PR's thread). I'm still working on converting the API to
consuming SubjectPublicKeyInfo structures and producing
CertificationRequestInfo ones - I have the OpenSSL flow working, but am
still missing a step for the NSS flow. Specifically, after step 2 of the
4 you suggested above, I need to use NSS to use the private key in the
db to sign the SubjectPublicKeyInfo before I can use python-cryptography
to make it into a CSR like I'm doing with OpenSSL. I'm sure this is not
very hard, but I haven't quite figured it out yet.
More information about the Freeipa-devel
mailing list