[Freeipa-devel] [freeipa PR#764][comment] Basic uninstaller for the CA

2017-05-09 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/764 Title: #764: Basic uninstaller for the CA rcritten commented: """ As far as I can tell it is always recoverable using this. I wasn't able to force a failure of replication, that could be a potential show-stopper. The P

[Freeipa-devel] [freeipa PR#764][comment] Basic uninstaller for the CA

2017-05-05 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/764 Title: #764: Basic uninstaller for the CA rcritten commented: """ What do you mean half-effective? Did you try it? There already IS a unified framework for component uninstallation: each service provides it. There are edge cases

[Freeipa-devel] [freeipa PR#764][opened] Basic uninstaller for the CA

2017-05-04 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/764 Author: rcritten Title: #764: Basic uninstaller for the CA Action: opened PR body: """ This in response to watching users flounder with repeated failed replica installations and ipa-ca-install attempts that require a com

[Freeipa-devel] [freeipa PR#735][comment] automount install: do not wait for sssd restart on uninstallation

2017-04-26 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/735 Title: #735: automount install: do not wait for sssd restart on uninstallation rcritten commented: """ I guess I have more issues with the commit message than the patch content. What would you suggest ensure that sssd is up?

[Freeipa-devel] [freeipa PR#590][comment] Validate user input for cert-get-requestdata

2017-03-15 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/590 Title: #590: Validate user input for cert-get-requestdata rcritten commented: """ You are duplicating the list of helpers. It would have been better to have helper defined as a StrEnum. If it isn't too late to change (

[Freeipa-devel] [freeipa PR#546][comment] Store session cookie in a ccache option

2017-03-07 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/546 Title: #546: Store session cookie in a ccache option rcritten commented: """ Should this patch not also remove the keyring code? Unit tests should be provided. """ See the full comment at https://githu

[Freeipa-devel] [freeipa PR#531][comment] httpinstance: disable system trust module in /etc/httpd/alias

2017-03-06 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/531 Title: #531: httpinstance: disable system trust module in /etc/httpd/alias rcritten commented: """ Just FYI I'm opening an upstream discussion with the NSS team on this. It is very strange that there is a conflict like this,

[Freeipa-devel] [freeipa PR#531][comment] httpinstance: disable system trust module in /etc/httpd/alias

2017-03-06 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/531 Title: #531: httpinstance: disable system trust module in /etc/httpd/alias rcritten commented: """ IIRC on install all three existing db's get copied to .orig, or something like that right? So uninstall would move t

[Freeipa-devel] [freeipa PR#482][comment] Don't count service/host/user cert md5 fprints in FIPS

2017-02-20 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/482 Title: #482: Don't count service/host/user cert md5 fprints in FIPS rcritten commented: """ In service.py the error isn't wrapped in _(). You should use the same message in both. Given the different messages I'm s

[Freeipa-devel] [freeipa PR#468][comment] Remove non-sensical kdestroy on https stop

2017-02-15 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/468 Title: #468: Remove non-sensical kdestroy on https stop rcritten commented: """ Rudeness is not necessary. You said: "As to why a) we backup Kerberos keys, and b) support restoring into running IPA server that is beyond me

[Freeipa-devel] [freeipa PR#468][comment] Remove non-sensical kdestroy on https stop

2017-02-15 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/468 Title: #468: Remove non-sensical kdestroy on https stop rcritten commented: """ If you don't backup the keytab then how do you expect to bring the server back up? Fetch new keys for all services? Full restore is very cle

[Freeipa-devel] [freeipa PR#450][comment] Add FIPS-token password of HTTPD NSS database

2017-02-10 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/450 Title: #450: Add FIPS-token password of HTTPD NSS database rcritten commented: """ I guess this is one approach to fix the problem. Would it be cleaner to pass in, or detect, FIPS mode, and only write out the token that will ac

[Freeipa-devel] [freeipa PR#453][comment] Cleanup certdb

2017-02-09 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/453 Title: #453: Cleanup certdb rcritten commented: """ I'm pretty sure the chdir() hack was due to SELinux issues, be sure to test in enforcing mode. It may no longer be required. """ See the full comment at

[Freeipa-devel] [freeipa PR#407][comment] New lite-server implementation

2017-01-24 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/407 Title: #407: New lite-server implementation rcritten commented: """ Right, and IMHO that development process is inefficient and prone to error. Rather than copying bits around and doing full installs over and over you can run the

[Freeipa-devel] [freeipa PR#407][comment] New lite-server implementation

2017-01-23 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/407 Title: #407: New lite-server implementation rcritten commented: """ I always found the lite-server to be incredibly helpful for server-side plugin development. If it isn't being used any more then I'd wonder what is bein

[Freeipa-devel] [freeipa PR#394][comment] Add fix for ipa plugins command

2017-01-16 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/394 Title: #394: Add fix for ipa plugins command rcritten commented: """ How about a test to prevent future regressions? """ See the full comment at https://github.com/freeipa/freeipa/pull/394#issuecomment-272962038 --

[Freeipa-devel] [freeipa PR#367][comment] Remove nsslib from IPA

2017-01-12 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/367 Title: #367: Remove nsslib from IPA rcritten commented: """ SSLv2 should not be supported, period. Not that it would work anyway because most SSL libs have completely removed this support, but it is just a terrible idea to even t

[Freeipa-devel] [freeipa PR#367][comment] Remove nsslib from IPA

2017-01-12 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/367 Title: #367: Remove nsslib from IPA rcritten commented: """ Wait, you added support for SSLv2? Please remove it, it isn't needed even for backwards compatibility and would not be considered a regression. ""&qu

[Freeipa-devel] [freeipa PR#367][comment] Remove nsslib from IPA

2017-01-04 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/367 Title: #367: Remove nsslib from IPA rcritten commented: """ Did you open a bug against NSS or python-nss regarding the PIN requirement? """ See the full comment at https://github.com/freeipa/freeipa/pull/367#iss

[Freeipa-devel] [freeipa PR#314][comment] RFC: privilege separation for ipa framework code

2017-01-03 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/314 Title: #314: RFC: privilege separation for ipa framework code rcritten commented: """ You can specify the nickname using -n/--nickname. You'll probably also want to set --cafile=/etc/ipa/ca.crt, --dbdir=/etc/httpd/alias

[Freeipa-devel] [freeipa PR#182][comment] Use env var IPA_CONFDIR to get confdir for 'cli' context

2016-11-28 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/182 Title: #182: Use env var IPA_CONFDIR to get confdir for 'cli' context rcritten commented: """ I don't see this as a convenience method. I'd find it less likely to use directly with the ipa tool (though having

[Freeipa-devel] [freeipa PR#215][comment] Add script to setup krb5 NFS exports

2016-11-10 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/215 Title: #215: Add script to setup krb5 NFS exports rcritten commented: """ Quite a lot of this code can be eliminated if you use ipalib instead of manually reading configuration files, forking out to ipa, doing a kinit, etc or

[Freeipa-devel] [freeipa PR#195][comment] [WIP] Make ipaclient pip install-able

2016-11-08 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/195 Title: #195: [WIP] Make ipaclient pip install-able rcritten commented: """ This is an important feature for integration. OpenStack uses pip to install dependencies into virtual environments for doing multi-version python test

[Freeipa-devel] [freeipa PR#204][comment] ipautil.run: Remove hardcoded environ PATH value

2016-11-02 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/204 Title: #204: ipautil.run: Remove hardcoded environ PATH value rcritten commented: """ +1 on using absolute paths. I don't recall any cases where KRB5_TRACE was needed so is this a theoretical use case or an actual one?

[Freeipa-devel] [freeipa PR#204][comment] ipautil.run: Remove hardcoded environ PATH value

2016-11-01 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/204 Title: #204: ipautil.run: Remove hardcoded environ PATH value rcritten commented: """ This isn't about replacing existing binaries, it's about putting binaries into unexpected places that are in the default PATH (e.g

[Freeipa-devel] [freeipa PR#204][comment] ipautil.run: Remove hardcoded environ PATH value

2016-11-01 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/204 Title: #204: ipautil.run: Remove hardcoded environ PATH value rcritten commented: """ PATH is untrustworthy because there is no knowing what is in it, or the order. It could easily have /usr/local/bin first and some rogue versi

[Freeipa-devel] [freeipa PR#204][comment] ipautil.run: Remove hardcoded environ PATH value

2016-11-01 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/204 Title: #204: ipautil.run: Remove hardcoded environ PATH value rcritten commented: """ NACK. I'd be fine with changing the PATH to remove cruft but the primary purpose is to prevent an attacker from providing their o

[Freeipa-devel] [freeipa PR#189][comment] Create relative symbol links

2016-10-26 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/189 Title: #189: Create relative symbol links rcritten commented: """ I think you should add the reasoning for switching the link type to the commit message and if this is related to some higher-level ticket that should be i

[Freeipa-devel] [freeipa PR#145][comment] Refactoring: LDAP Connection Management

2016-10-20 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/145 Title: #145: Refactoring: LDAP Connection Management rcritten commented: """ Removing the connection timeout makes me a bit edgy. It is quite unclear why that's there, perhaps python-ldap doesn't provide a timeout and

[Freeipa-devel] [freeipa PR#84][comment] Fix update_from_dict function testing

2016-09-21 Thread rcritten
URL: https://github.com/freeipa/freeipa/pull/84 Title: #84: Fix update_from_dict function testing rcritten commented: """ The sort of pie-in-the-sky thinking about 3rd party plugins that Jason and I talked about eons ago was users would provide their own update files as

[Freeipa-devel] [freeipa PR#84] Removed update_from_dict function from ldapupdate (comment)

2016-09-15 Thread rcritten
rcritten commented on a pull request """ For the record this test used to pass. Don't blame the test when the code it is testing was changed. """ See the full comment at https://github.com/freeipa/freeipa/pull/84#issuecomment-247329152 -- Manage your subscri

[Freeipa-devel] [PATCH 1/1] After unininstall see if certmonger is still tracking any of our certs.

2012-10-23 Thread rcritten
From: Rob Crittenden Rather than providing a list of nicknames I'm going to look at the NSS databases directly. Anything in there is suspect and this will help future-proof us. certmonger may be tracking other certificates but we only care about a subset of them, so don't complain if there are o

[Freeipa-devel] [PATCH 0/1] Check for leftover certmonger certs

2012-10-23 Thread rcritten
From: Rob Crittenden Rob Crittenden (1): After unininstall see if certmonger is still tracking any of our certs. install/tools/ipa-server-install | 10 +- ipapython/certmonger.py | 33 + 2 files changed, 42 insertions(+), 1 deletion(-) --