PR - WIP: Support CLI tooling for x509 management

[389-ds-bot]

Cloned from Pagure Pull-Request: https://pagure.io/389-ds-base/pull-request/50648

• Created at 2019-10-15 01:00:42 by firstyear (@Firstyear)
• Merged at 2019-11-07 01:48:18

---

NSSDB's are difficult and hard to use. They make the certificate experience with 389 poor. This is really highlighted with systems like lets encrypt and such where people are so used to PEM file management, the idea of using certificate databases goes against this.

This adds support tooling to make the certificate DB easier to interact with. It's not intended to replace certutil - but to wrap and make common operations simpler to approach and use.

Resolves: #3066

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-10-21 13:22:33

Wouldn't it be better to use log.info() here? We consistently use it everywhere and it allows to set verbose, debug modes

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-10-21 13:23:54

I think the message is not verbose enough...

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-10-21 13:28:27

Probably, should take care of the lower case here

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-10-21 13:34:49

I think it will be nice to add a man file with detailed descriptions of commands that we run. So the user is aware of the decisions that we make for certutil commands. Like: the file names/paths, ciphers, etc.

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-10-21 13:36:36

I haven't tested it yet, but the rest of the code looks simple and easy to follow. Maybe @kenoh will have something to add...

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-10-21 14:59:43

We already have a security subcommand that does most of this. See /lib389/cli_conf/security.py

Perhaps you can extend the existing security CLI instead of a duplicate subcommand?

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-10-21 23:24:39

We already have a security subcommand that does most of this. See /lib389/cli_conf/security.py
Perhaps you can extend the existing security CLI instead of a duplicate subcommand?

dsconf vs dsctl - this needs local FS access to manipulate the nssdb, which is why it's not part of the security command ....

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-10-21 23:25:31

I think it will be nice to add a man file with detailed descriptions of commands that we run. So the user is aware of the decisions that we make for certutil commands. Like: the file names/paths, ciphers, etc.

What do you mean here? Like improving the help section? Or by explaining what options we choose?

Generally the idea here is we encapsulate best practices and then over time we can always improve and upgrade these decisiosns so users don't have to.

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-10-21 23:30:27

We already have a security subcommand that does most of this. See /lib389/cli_conf/security.py
Perhaps you can extend the existing security CLI instead of a duplicate subcommand?

dsconf vs dsctl - this needs local FS access to manipulate the nssdb, which is why it's not part of the security command ....

Thought it was dsconf, ugh. I'll shut up now :-p

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-10-21 23:32:34

@droideck So part of the question for you was about how we should structure and display the certificate data - that's what I'd really like some feedback on.

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-10-21 23:38:37

We already have a security subcommand that does most of this. See /lib389/cli_conf/security.py
Perhaps you can extend the existing security CLI instead of a duplicate subcommand?
dsconf vs dsctl - this needs local FS access to manipulate the nssdb, which is why it's not part of the security command ....

Thought it was dsconf, ugh. I'll shut up now :-p

All good, easy mistake to make. Please don't shut up, I need the comments!

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-10-22 22:52:52

I think it will be nice to add a man file with detailed descriptions of commands that we run. So the user is aware of the decisions that we make for certutil commands. Like: the file names/paths, ciphers, etc.

What do you mean here? Like improving the help section? Or by explaining what options we choose?
Generally the idea here is we encapsulate best practices and then over time we can always improve and upgrade these decisiosns so users don't have to.

Yeah, basically, what I meant is to describe in more detail and explicitly these decisions in the help section.
So in the result, they are displayed in man dsctl info too.

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-10-22 22:57:21

Probably, more of a nitpick but still. All over the CLI we use 'dash' approach. So maybe we can rename the commands to remove-cert in this tool too?

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-10-22 23:06:39

Probably, more of a nitpick but still. All over the CLI we use 'dash' approach. So maybe we can rename the commands to remove-cert in this tool too?

Yes please make this conversion, nice catch @droideck !

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-10-22 23:10:54

@droideck So part of the question for you was about how we should structure and display the certificate data - that's what I'd really like some feedback on.

I think the main thing we should keep in mind - is that it should be simple (we already have 'certutil' for complex and detailed operations). And I think your structure is good in that term. :)

Another point to remember - is that the user should be able to pass the displayed certificates through pipes. So it should be straight forward there without any not needed strings.

The rest, I think, looks good and fine-grained enough to cover most of the operations.
But I've always done only basic certutil stuff (generate cert, sign it, import, etc.)...

And to check if we don't break any other, more rare cases that we can encounter while managing DS - I think, we need @kenoh here :)

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-10-23 03:37:31

So I've added thumbs as I checked off each comment as I worked on it. I agree with your comment that complex and detailed things are for certutil. This is just for some simple transforms an "basic" day to day. It's mainly because I'm actually too lazy to use certutil, and if IM too lazy, imagine how our customers feel .....

Anyway, I think the piping may be an advanced case - I'll leave that to certutil. But you are right, we should keep this simple, so I think I'll make the display either just:

• for CA listing show nick names + expiry date + subject
• For details on a specific cert we effectively just wrap certutil -L -d path -n nickname, to make it a bit easier to display details without reinventing this ourselves.

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-10-23 04:24:29

rebased onto 5c1e973a53cf385581a89491ba63b866315d99c8

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-10-23 04:24:45

As an aside, the positional args can NOT be renamed from say cert_path to cert-path as cert-ptah is not a valid python variable ....

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-10-23 08:41:01

As an aside, the positional args can NOT be renamed from say cert_path to cert-path as cert-ptah is not a valid python variable ....

Yep, what I meant is just the parsers, not arguments, of course.
Maybe I misread and the parsers were okay... I can't tell now because of the rebase.
And sorry for the noise if it's on me. :D

The rest looks good but I still haven't tested it.
Also, I'll leave for others to check too because it's important to agree about the structure and the made decisions of such tool together.

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-10-25 01:29:33

Yep. It would be good for you to test this I think before giving the yay/nay. @mreynolds389 do you have thoughts here too?

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-10-29 23:59:00

poke @droideck and @mreynolds389 :)

Everyone just seems so busy lately :)

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-11-04 01:21:59

Review reminder :)

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-11-06 20:30:06

Yeah works nicely, ack

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-11-07 01:13:06

Awesome! Thanks I'll merge this soon then.

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-11-07 01:45:16

rebased onto 83d4143

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2019-11-07 01:48:18

Pull-Request has been merged by Firstyear

[389-ds-bot]

Patch
50648.patch