On 28/05/15 10:13, Fraser Tweedale wrote:
On Wed, May 27, 2015 at 06:12:50PM +0200, Martin Basti wrote:
On 27/05/15 15:53, Fraser Tweedale wrote:
This patch adds supports for multiple user / host certificates. No
schema change is needed ('usercertificate' attribute is already
multi-value). The revoke-previous-cert behaviour of host-mod and
user-mod has been removed but revocation behaviour of -del and
-disable is preserved.
The latest profiles/caacl patchset (0001..0013 v5) depends on this
patch for correct cert-request behaviour.
There is one design question (or maybe more, let me know): the
`--out=FILENAME' option to {host,service} show saves ONE certificate
to the named file. I propose to either:
a) write all certs, suffixing suggested filename with either a
sequential numerical index, e.g. "cert.pem" becomes
"cert.pem.1", "cert.pem.2", and so on; or
b) as above, but suffix with serial number and, if there are
different issues, some issuer-identifying information.
Let me know your thoughts.
Thanks,
Fraser
Is there a possible way how to store certificates into one file?
I read about possibilities to have multiple certs in one .pem file, but I'm
not cert guru :)
I personally vote for serial number in case there are multiple certificates,
if ^ is no possible.
Actually, yeah, we can just write the PEMs to a single file,
sequentially. I don't know why I didn't think of that... ¯\_(ツ)_/¯
1)
+ if len(certs) > 0:
please use only,
if certs:
I have strong feels about this. ``if len(certs) > 0:`` admits fewer
bugs than ``if certs:`` , e.g. if 'certs' were bound to a
non-length-able object by mistake. Even though we have already
iterated ``certs`` at this point in the function, this failure mode
is still possible, e.g. if ``certs`` is a generator. Using ``if
certs:`` will not fail for a generator, but it will be a silent bug!
PEP-8 is wrong on this one. It is better to use the construction
that admits fewer errors. Unless it causes lint failure (on f21 it
does not) I prefer not to change it.
I still think it should be in PEP8 style.
We should avoid these mistakes, not create the code which will detect it.
And, I forgot to write, please use
except Exception as e: instead except Exception, e
to be compatible with python3
2)
You need to re-generate API/ACI.txt in this patch
Good pickup, thanks.
3)
syntax error:
+ for dercert in certs_der
Geez... dunno how I let that one past -_- My bad.
4)
command
ipa user-mod ca_user --certificate=<ceritifcate>
removes the current certificate from the LDAP, by design.
Should be the old certificate(s) revoked? You removed that part in the code.
only the --addattr='usercertificate=<cert>' appends new value there
Yeah... I was a bit confused about how --addattr=... interacts with
options. I understand it now, and yes I think we should revoke
certificates that get removed this way.
cert-request will use addattr= so no existing certificates will get
removed (or revoked). New patch addressing this and other points
will arrive on list today.
Thanks for reviewing!
Fraser
--
Martin Basti
--
Martin Basti
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
