On 06/30/2015 12:04 PM, Jan Cholasta wrote:
Dne 29.6.2015 v 10:36 Martin Babinsky napsal(a):
On 06/23/2015 01:49 PM, Martin Babinsky wrote:
This patchset implements new API commands for manipulating
user/host/service userCertificate attribute alongside some underlying
plumbing.

PATCH 0045 is a small test suite that I slapped together since manual
testing of this stuff is very cumbersome. It requires my PATCH 0040 to
apply and work which was pushed to master recently
(commit 74883bbc959058c8bfafd9f63e8fad0e3792ac28).

The work is related to http://www.freeipa.org/page/V4/User_Certificates
and https://fedorahosted.org/freeipa/ticket/4238



Attaching updated patches.

Here are some notes for Jan because I did some things differently than
we agreed on during review:


1.) I chose not to rename 'usercertificate' to 'usercertificate;binary'
and back in pre/post callbacks. Despite the fact that the correct way to
name the certificate attribute is 'usercertificate;binary', I feel that
suddenly renaming it in the new code is asking for trouble.

New code is new, there is no renaming, there is naming, and that naming
should follow standards, and the standard is userCertificate;binary.

(For the record I did not ask for any renaming in *old* host and service
code.)

OK I will then use 'usercertificate;binary' and try to not break things.

I'm all for changing the mapping between CLI options and actual
attribute names but it should be done in a systematic fashion.

+1, shall I post a patch?

That would be great, but I'm not sure if there is time for it. Maybe we can create a ticket for tracking?

2.) I have kept the `normalize_certs` function. It has the potential to
catch incorrectly formatted/encoded certificates and in a way
circumvents the slightly demented way the framework deals with
supposedly binary data.

One sentence above you asked for doing things in systematic fashion.
This is exactly what it isn't. A systematic solution would be a new
parameter type for certificates.

Ha I didn't notice that incorrect encoding is caught by validator.

But I think that we still need to catch malformed certificates that can not be decoded to DER and AFAIK we don't do that anywhere (failing tests when adding a random Base64-encoded string confirm this).

All this probably stems from my confusion about the way IPA framework guesses binary data. For example, if I call `api.Command.user_add_cert` and fill 'certificate' option with Base64 blob reencoded to Unicode, everything works as expected.

However, filling this option with 'str' leads to another round of Base64 encoding in the framework, leading to 'userCertificate;binary' which is filled by original Base64 blob instead of DER encoded cert.


I have also added two negative test cases which deal with incorrectly
encoded and formatted certificates.





--
Martin^3 Babinsky

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

Reply via email to