Re: [Freeipa-devel] [PATCH] 315 Convert external CA chain to PKCS#7 before passing it to pkispawn
On 08/08/2014 11:50 AM, Jan Cholasta wrote: Dne 8.8.2014 v 11:20 Martin Kosek napsal(a): On 08/08/2014 10:55 AM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4397. Honza Thanks! I did not test, just have couple questions/suggestions: 1) Are we testing that the certificate is in proper format, e.g. is not PKCS7 already? We need to error out properly then Yes, in ipa-server-install. 2) Are ipa-server-install --help options as informative as possible? --external-ca installation is tricky, we need to make sure that is no doubt about what the input is. I amended them a little bit. 3) We may want to add instructions how to convert PKCS#7 - PEM to man ipa-server-install too. Added. Martin Updated patch attached. Hello, This works for me, but I'm not sure if I'm correctly reproducing the specific scenario this patch fixes. So as always, can you please add tests for code you write? As far as other scenarios, it seems to me that when I do something wrong I get a very unhelpful error message late in the installation. I tried signing the request using xca but pkispawn choked on the result; I'll try to write a reproducer script using command-line tools. Attached is a script (based on the external ca integration test) that reproduces the same IndexError as mentioned in the ticket. (If necessary, adjust the IP addresses, hostnames, etc. to fit your environment.) The difference from a working script is that extensions aren't added to the IPA cert when it's signed. -- Petr³ index-error-reproducer.sh Description: application/shellscript ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 315 Convert external CA chain to PKCS#7 before passing it to pkispawn
On 08/13/2014 03:12 PM, Petr Viktorin wrote: On 08/08/2014 11:50 AM, Jan Cholasta wrote: Dne 8.8.2014 v 11:20 Martin Kosek napsal(a): On 08/08/2014 10:55 AM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4397. Honza Thanks! I did not test, just have couple questions/suggestions: 1) Are we testing that the certificate is in proper format, e.g. is not PKCS7 already? We need to error out properly then Yes, in ipa-server-install. 2) Are ipa-server-install --help options as informative as possible? --external-ca installation is tricky, we need to make sure that is no doubt about what the input is. I amended them a little bit. 3) We may want to add instructions how to convert PKCS#7 - PEM to man ipa-server-install too. Added. Martin Updated patch attached. Hello, This works for me, but I'm not sure if I'm correctly reproducing the specific scenario this patch fixes. So as always, can you please add tests for code you write? +1! As far as other scenarios, it seems to me that when I do something wrong I get a very unhelpful error message late in the installation. I tried signing the request using xca but pkispawn choked on the result; I'll try to write a reproducer script using command-line tools. Attached is a script (based on the external ca integration test) that reproduces the same IndexError as mentioned in the ticket. (If necessary, adjust the IP addresses, hostnames, etc. to fit your environment.) The difference from a working script is that extensions aren't added to the IPA cert when it's signed. This is a very good finding. If Jan's patch fixes the reported problem, let us push it. But the missing validation should be fixed too. Can you please extend https://fedorahosted.org/freeipa/ticket/4480 that is (will be) planned for 4.1 and attach your script as well so that we can improve the usability by both accepting more certificate types and validation? Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 733-735 webui: Better description for User authentication types
On 8/5/2014 6:38 AM, Petr Vobornik wrote: [PATCH] 733 webui: rename tooltip to title - use title for input's elements 'title' attribute - tooltip for Bootstrap's tooltip component https://fedorahosted.org/freeipa/ticket/4471 ACK. [PATCH] 734 webui: tooltip support Allow to set 'tooltip' attribute in spec. It displays info icon with Bootstrap's tooltip near field's label. https://fedorahosted.org/freeipa/ticket/4471 ACK. [PATCH] 735 webui: better authentication types description Tooltips were added to User authentication types and Default user objectclasses to describe their relationship and a meaning of not-setting a value. https://fedorahosted.org/freeipa/ticket/4471 Just one thing, in the patch comment you probably meant Default user authentication types. ACK. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0001 User Life Cycle: create containers and scoping DS plugins
On 08/08/2014 09:24 AM, thierry bordaz wrote: Hi, The attached patch is a first patch related to 'User Life Cycle' (https://fedorahosted.org/freeipa/ticket/3813) It creates 'Stage' and 'Delete' containers and configure DS plugin to scope only 'Active' container or exclude 'Stage'/'Delete' Hello, The .ldif files are copied only during initial installation. When upgrading to a version with this patch, changes in .ldif files are not applied. So all updates need to be in .update files. For example, for DNA plugin configuration you would need something like this in an .update file: dn: cn=Posix IDs,cn=Distributed Numeric Assignment Plugin,cn=plugins,cn=config remove:dnaScope: $SUFFIX add:dnaScope: cn=accounts,$SUFFIX .update files, on the other hand, are applied both on installation and on upgrade. To avoid duplication you can put whole entries in .update and delete them from the .ldif, provided the entries always end up being created in a correct order. Patch submission technicalities: Please don't add the Reviewed by tag to the commit message, it's added when pushing. The other tags are not used FreeIPA. (What's a Flag Day?) When you send more patches that depend on each other, either attach them all to one e-mail, or explicitly say what each patch depends on. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0001 User Life Cycle: create containers and scoping DS plugins
On 08/13/2014 08:48 AM, Petr Viktorin wrote: On 08/08/2014 09:24 AM, thierry bordaz wrote: Hi, The attached patch is a first patch related to 'User Life Cycle' (https://fedorahosted.org/freeipa/ticket/3813) It creates 'Stage' and 'Delete' containers and configure DS plugin to scope only 'Active' container or exclude 'Stage'/'Delete' Hello, The .ldif files are copied only during initial installation. When upgrading to a version with this patch, changes in .ldif files are not applied. So all updates need to be in .update files. For example, for DNA plugin configuration you would need something like this in an .update file: dn: cn=Posix IDs,cn=Distributed Numeric Assignment Plugin,cn=plugins,cn=config remove:dnaScope: $SUFFIX add:dnaScope: cn=accounts,$SUFFIX .update files, on the other hand, are applied both on installation and on upgrade. To avoid duplication you can put whole entries in .update and delete them from the .ldif, provided the entries always end up being created in a correct order. Patch submission technicalities: Please don't add the Reviewed by tag to the commit message, it's added when pushing. The other tags are not used FreeIPA. (What's a Flag Day?) Flag Day is a warning to other developers - Hey, this change will break something in your usual workflow, plan accordingly When you send more patches that depend on each other, either attach them all to one e-mail, or explicitly say what each patch depends on. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0001 User Life Cycle: create containers and scoping DS plugins
On 08/13/2014 04:48 PM, Petr Viktorin wrote: On 08/08/2014 09:24 AM, thierry bordaz wrote: Hi, The attached patch is a first patch related to 'User Life Cycle' (https://fedorahosted.org/freeipa/ticket/3813) It creates 'Stage' and 'Delete' containers and configure DS plugin to scope only 'Active' container or exclude 'Stage'/'Delete' Hello, The .ldif files are copied only during initial installation. When upgrading to a version with this patch, changes in .ldif files are not applied. So all updates need to be in .update files. For example, for DNA plugin configuration you would need something like this in an .update file: dn: cn=Posix IDs,cn=Distributed Numeric Assignment Plugin,cn=plugins,cn=config remove:dnaScope: $SUFFIX add:dnaScope: cn=accounts,$SUFFIX .update files, on the other hand, are applied both on installation and on upgrade. To avoid duplication you can put whole entries in .update and delete them from the .ldif, provided the entries always end up being created in a correct order. Hello Petr, Thanks you very much for your review. I understand that the fix does not work in upgrade and I will change it following your recommendation. I think that adding entries with the .update syntax should be similar to what is in 55-pbacmemberof.update for example. Patch submission technicalities: Please don't add the Reviewed by tag to the commit message, it's added when pushing. The other tags are not used FreeIPA. (What's a Flag Day?) When you send more patches that depend on each other, either attach them all to one e-mail, or explicitly say what each patch depends on. That is correct I used a review template that was for 389-ds and I will change it. 'Flag Day' was part of 389-DS template, it was a flag to inform if the fix had a wide impact (things needing to be ported/recompile). I split ULC fix into several logical sub fixes and you are right they are all related even if for example 0002 does not depend on 0001. Do you want I resend patch 0003 with the statement it relies on 0001 (and with the correct commit message ?). thanks thierry ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 736-740 webui: various minor fixes
On 8/5/2014 6:43 AM, Petr Vobornik wrote: [PATCH] 736 webui: convert widget.less indentation to spaces ACK. [PATCH] 737 webui: improve rule table css - category radio line has line-height large enough to contain undo button - content doesn't move several pixels on change - remove vertical padding from btns in table headers to maintain about the same height - remove invisible border from link buttons to have the same height for disabled and enabled button ACK. [PATCH] 738 webui: sshkey widget - usability fixes - save one click by opening edit dialog right after adding new row - add margin between fingerprint and show/edit button - fix honoring of writable/read-only flags upon row creation ACK. Possible improvements: 1. How about removing the row if the user cancels the addition or enters blank value? That way the rows will always have values, so we don't need the New: key set/not set labels anymore. 2. Can the UI parse the new key and display it the same way as other keys that are already saved? That will make it more seamless. 3. If we do #2, the Show/Set key button probably can be changed to Edit or Modify. [PATCH] 739 webui: disable batch action buttons by default action buttons associated with batch actions were enabled by default, but they were disabled right after facet creation and a load of data. It caused a visual flicker. UX is enhanced by making them disabled by default. ACK. [PATCH] 740 webui: fix group type padding ACK. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 741 webui: add link to OTP token app
On 8/5/2014 10:11 AM, Petr Vobornik wrote: - display info message which points user to FreeOTP project page - the link or the text can be easily changed by a plugin if needed https://fedorahosted.org/freeipa/ticket/4469 Notes: - the design can be a subject of discussion. - the FreeOTP project page [1] is not very end-user friendly but serves the purpose - it's not fully configurable, as decided at today's meeting, but a very simple plugin can hide or modify the message [1] https://fedorahosted.org/freeotp/ ACK. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0001 User Life Cycle: create containers and scoping DS plugins
On 08/13/2014 05:17 PM, thierry bordaz wrote: On 08/13/2014 04:48 PM, Petr Viktorin wrote: On 08/08/2014 09:24 AM, thierry bordaz wrote: Hi, The attached patch is a first patch related to 'User Life Cycle' (https://fedorahosted.org/freeipa/ticket/3813) It creates 'Stage' and 'Delete' containers and configure DS plugin to scope only 'Active' container or exclude 'Stage'/'Delete' Hello, The .ldif files are copied only during initial installation. When upgrading to a version with this patch, changes in .ldif files are not applied. So all updates need to be in .update files. For example, for DNA plugin configuration you would need something like this in an .update file: dn: cn=Posix IDs,cn=Distributed Numeric Assignment Plugin,cn=plugins,cn=config remove:dnaScope: $SUFFIX add:dnaScope: cn=accounts,$SUFFIX .update files, on the other hand, are applied both on installation and on upgrade. To avoid duplication you can put whole entries in .update and delete them from the .ldif, provided the entries always end up being created in a correct order. Hello Petr, Thanks you very much for your review. I understand that the fix does not work in upgrade and I will change it following your recommendation. I think that adding entries with the .update syntax should be similar to what is in 55-pbacmemberof.update for example. Yes. Patch submission technicalities: Please don't add the Reviewed by tag to the commit message, it's added when pushing. The other tags are not used FreeIPA. (What's a Flag Day?) When you send more patches that depend on each other, either attach them all to one e-mail, or explicitly say what each patch depends on. That is correct I used a review template that was for 389-ds and I will change it. 'Flag Day' was part of 389-DS template, it was a flag to inform if the fix had a wide impact (things needing to be ported/recompile). I split ULC fix into several logical sub fixes and you are right they are all related even if for example 0002 does not depend on 0001. Do you want I resend patch 0003 with the statement it relies on 0001 (and with the correct commit message ?). These guidelines just make it easier for us to handle the large numbers of patches that land on the list. Try to follow them next time you send a patch (or revision), but there's no need to resubmit things just to comply. We can change the message when pushing if the patch contents are acked. Thanks for the dependency information. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 715 webui: add bounce url to reset_password.html
On 7/29/2014 5:53 AM, Petr Vobornik wrote: Just one thing, there is no pause between clicking the Reset button and the redirection, so the Password reset was successful. confirmation message might only appear very briefly. A possible alternative is to show a confirmation page/message, but the user will have to click to continue to the next page. I don't believe there is a universal solution. I would say that it depends on personal preferences and a use case. I.e., if it's part of a login procedure I would prefer immediate redirection back to login page. If it's invoked from a user action - just to change the password, some delay might be good. We might add a URL param(s) to configure the delay/link. How about 2 URL params? 1. the link to the next page 2. an option whether to a) redirect to the link immediately b) show a confirmation page with the link Just my preference, but I don't really like a 'delay' on a web page. It's either too short or too long, and we can't put important info during the delay because there's no guarantee people will see it. Yes, how about this: 1. redirection=url_to_the_page 2. in=x, where x is number of seconds or a string - redirect immediately if only `redirection` is supplied or `in=0` - show Continue to anext page/a link if `in` is present - show count-down timer if `in` is 1 with You will be redirected in `x`s text. - don't redirect if `in` is negative or NaN (your scenario), e.g.: `in=no` up to user - ignore `in` if `redirection` is not present Then we will support all described scenarios and the decision will be on web-site admin. Feel free to propose better name for `in`. How about 'url/link' or 'next_url' for the first parameter and 'delay' for the second one? I think in this case 'delay' would describe its function better than 'in'. If delay=0 that means we're redirecting immediately. If delay parameter is not specified that means we're waiting for the user to click the link. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] - Add DRM to IPA
In Dogtag, we have decided to revert the name of the DRM to the old name KRA.
DRM was really only used in docs/marketing, whereas KRA is all over the code.
Soon, the code and the marketing/docs will match.
The following patch changes all references to the DRM to KRA.
so for example, you need to run ipa-kra-install etc.
Please apply this on top of the previous patch. I'll go ahead and squash them
before commit.
Thanks,
Ade
- Original Message -
From: Ade Lee a...@redhat.com
To: Petr Viktorin pvikt...@redhat.com
Cc: freeipa-devel@redhat.com
Sent: Wednesday, August 13, 2014 2:05:51 PM
Subject: Re: [Freeipa-devel] [PATCH] - Add DRM to IPA
New patch attached which all the issues noted below. Rebased to master.
Please review,
Thanks,
Ade
On Mon, 2014-08-11 at 16:54 +0200, Petr Viktorin wrote:
On 08/09/2014 01:36 AM, Rob Crittenden wrote:
Ade Lee wrote:
Attached is a new patch. I believe I have addressed all the issues
raided by pviktori, edewata and rcrit.
Ar!
Please let me know if I missed something!
Incidentally, to get all this to work, you should use the latest Dogtag
10.2 build, which also contains a fix for pkidestroy that is not yet
merged in. A COPR build is currently underway at:
http://copr.fedoraproject.org/coprs/vakwetu/dogtag/build/24804/
Some whitespace issues:
Applying: Add a DRM to IPA
/home/rcrit/redhat/freeipa/.git/rebase-apply/patch:3774: trailing
whitespace.
This relies on the DRM client to generate a wrapping key
/home/rcrit/redhat/freeipa/.git/rebase-apply/patch:3292: new blank line
at EOF.
+
warning: 2 lines add whitespace errors.
lying: Add a DRM to IPA
/home/rcrit/redhat/freeipa/.git/rebase-apply/patch:3774: trailing
whitespace.
This relies on the DRM client to generate a wrapping key
/home/rcrit/redhat/freeipa/.git/rebase-apply/patch:3292: new blank line
at EOF.
+
warning: 2 lines add whitespace errors.
fixed
I do hope you're planning on adding a minimum build dep at some point?
yes - as soon as we have an official-ish dogtag build.
Still seeing AVCs during install:
time-Fri Aug 8 19:13:35 2014
type=SYSCALL msg=audit(1407539615.743:1503): arch=c03e syscall=1
success=no exit=-13 a0=3 a1=210cb30 a2=2d a3=7fff1caa83f0 items=0
ppid=12121 pid=12307 auid=4294967295 uid=994 gid=993 euid=994 suid=994
fsuid=994 egid=993 sgid=993 fsgid=993 tty=(none) ses=4294967295
comm=cp exe=/usr/bin/cp subj=system_u:system_r:pki_tomcat_t:s0
key=(null)
type=AVC msg=audit(1407539615.743:1503): avc: denied { setfscreate }
for pid=12307 comm=cp scontext=system_u:system_r:pki_tomcat_t:s0
tcontext=system_u:system_r:pki_tomcat_t:s0 tclass=process
time-Fri Aug 8 19:13:35 2014
type=SYSCALL msg=audit(1407539615.743:1504): arch=c03e syscall=190
success=no exit=-13 a0=4 a1=7fff1caa8590 a2=210c8f0 a3=2d items=0
ppid=12121 pid=12307 auid=4294967295 uid=994 gid=993 euid=994 suid=994
fsuid=994 egid=993 sgid=993 fsgid=993 tty=(none) ses=4294967295
comm=cp exe=/usr/bin/cp subj=system_u:system_r:pki_tomcat_t:s0
key=(null)
type=AVC msg=audit(1407539615.743:1504): avc: denied { relabelfrom }
for pid=12307 comm=cp name=CS.cfg.bak.20140808191335 dev=dm-0
ino=430828 scontext=system_u:system_r:pki_tomcat_t:s0
tcontext=system_u:object_r:pki_tomcat_etc_rw_t:s0 tclass=file
time-Fri Aug 8 19:13:35 2014
type=SYSCALL msg=audit(1407539615.744:1505): arch=c03e syscall=88
success=no exit=-13 a0=7fffd3c0daa7 a1=7fffd3c0daea a2=0 a3=7fffd3c0b9b0
items=0 ppid=12121 pid=12308 auid=4294967295 uid=994 gid=993 euid=994
suid=994 fsuid=994 egid=993 sgid=993 fsgid=993 tty=(none) ses=4294967295
comm=ln exe=/usr/bin/ln subj=system_u:system_r:pki_tomcat_t:s0
key=(null)
type=AVC msg=audit(1407539615.744:1505): avc: denied { create } for
pid=12308 comm=ln name=CS.cfg.bak
scontext=system_u:system_r:pki_tomcat_t:s0
tcontext=system_u:object_r:pki_tomcat_etc_rw_t:s0 tclass=lnk_file
Rob, please open BZ against selinux-policy for these. Interestingly,
audit2allow doesn't provide me with any output for these AVC's.
The new estimated time was dead on :-)
There was a fairly long wait after Done configuring DRM server
(pki-tomcatd). and the install was done. I thought we always displayed
text when restarting (e.g. handled by the service wrapper) but I guess
not. It would be nice to know what is going on.
done
Re-running ipa-drm-install results in a scary error:
]# ipa-drm-install
Usage: ipa-drm-install [options] [replica_file]
ipa-drm-install: error: DRM is already installed.
Your system may be partly configured.
Run /usr/sbin/ipa_drm_install.py --uninstall to clean up.
Right, you don't want to override handle_error here. Instead, wrap the
body of run() in
try:
except:
self.log.error(self.FAIL_MESSAGE)
raise
(Yes, bare `except` and bare `raise`)
I
