date:20120726

[Freeipa-devel] Freeipa wiki editing

2012-07-26 Thread Javier Ramirez

Hi,

As per the instructions found at http://freeipa.com/page/Contribute , I send 
this email to request for a freeipa wiki account . I have some amends to make 
to http://freeipa.com/page/ConfiguringAixClients .

Thanks.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] slow response

2012-07-26 Thread Stephen Ingram

On Wed, Jul 25, 2012 at 4:31 AM, John Dennis jden...@redhat.com wrote:
 On 07/25/2012 02:59 AM, Stephen Ingram wrote:

 Seeing python2.7, I'm guessing these patches were against Fedora.
 Since I couldn't get them to apply against RH 6.3 I applied them by
 hand. I couldn't get the WebUI to load after applying the patches. I'm
 not sure of the code that caused the problem, but I did see mention of
 global name datetime not being defined. You wouldn't happen to have
 patches for RH 6.3?


 Sorry, I thought you had a F17 install. It should be easy to fix the
 datetime issue, just add this add the top of the file:

 import time, datetime

 If that simple fix doesn't work then let me know the version you've got and
 I'll make up a new patch against that version.

Yes, please send a patch for 2.20 with RH 6.3. The import time,
datetime did not address all of the errors.

Steve

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0041] Cleanup in logging code

2012-07-26 Thread Adam Tkac

On Wed, Jul 25, 2012 at 03:31:34PM +0200, Petr Spacek wrote:
 Hello,
 
 this patch clears logging code a bit. Adding functions like
 log_info() and similar will be trivial from now.
 
 It will be necessary for ticket #71: Log successful reconnect
 https://fedorahosted.org/bind-dyndb-ldap/ticket/71

Ack.

 From 26136d6fe5fce5ac4f3138063bcf4774f268bd3c Mon Sep 17 00:00:00 2001
 From: Petr Spacek pspa...@redhat.com
 Date: Thu, 19 Jul 2012 14:13:12 +0200
 Subject: [PATCH] Cleanup in logging code.
 
 Signed-off-by: Petr Spacek pspa...@redhat.com
 ---
  src/log.c |   22 ++
  src/log.h |   19 ---
  2 files changed, 18 insertions(+), 23 deletions(-)
 
 diff --git a/src/log.c b/src/log.c
 index 
 b23e4720a8dd484a65d8a7e6c58baf257fc9ce50..f731df706b58e1f894659811dae32d4148a8620c
  100644
 --- a/src/log.c
 +++ b/src/log.c
 @@ -28,31 +28,13 @@
  #include log.h
  
  void
 -log_debug(int level, const char *format, ...)
 +log_write(int level, const char *format, ...)
  {
   va_list args;
  
   va_start(args, format);
 -#ifdef LOG_AS_ERROR
 - UNUSED(level);
   isc_log_vwrite(dns_lctx, DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_DYNDB,
 -ISC_LOG_ERROR, format, args);
 -#else
 - isc_log_vwrite(dns_lctx, DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_DYNDB,
 -ISC_LOG_DEBUG(level), format, args);
 -#endif
 -
 - va_end(args);
 -}
 -
 -void
 -log_error(const char *format, ...)
 -{
 - va_list args;
 -
 - va_start(args, format);
 - isc_log_vwrite(dns_lctx, DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_DYNDB,
 -ISC_LOG_ERROR, format, args);
 +level, format, args);
   va_end(args);
  
  }
 diff --git a/src/log.h b/src/log.h
 index 
 0df4e25618fab932bdec97c276580d1b9d31bf08..898639be144dbf6049a1440493c3358e01a5c2dd
  100644
 --- a/src/log.h
 +++ b/src/log.h
 @@ -22,18 +22,31 @@
  #define _LD_LOG_H_
  
  #include isc/error.h
 +#include dns/log.h
 +
 +#ifdef LOG_AS_ERROR
 +#define GET_LOG_LEVEL(level) ISC_LOG_ERROR
 +#else
 +#define GET_LOG_LEVEL(level) (level)
 +#endif
  
  #define fatal_error(...) \
  isc_error_fatal(__FILE__, __LINE__, __VA_ARGS__)
  
  #define log_bug(fmt, ...) \
   log_error(bug in %s():  fmt, __func__,##__VA_ARGS__)
  
  #define log_error_r(fmt, ...) \
 - log_error(fmt : %s, ##__VA_ARGS__, isc_result_totext(result))
 + log_error(fmt : %s, ##__VA_ARGS__, dns_result_totext(result))
  
  /* Basic logging functions */
 -void log_debug(int level, const char *format, ...) ISC_FORMAT_PRINTF(2, 3);
 -void log_error(const char *format, ...) ISC_FORMAT_PRINTF(1, 2);
 +#define log_error(format, ...)   \
 + log_write(GET_LOG_LEVEL(ISC_LOG_ERROR), format, ##__VA_ARGS__)
 +
 +#define log_debug(level, format, ...)\
 + log_write(GET_LOG_LEVEL(level), format, ##__VA_ARGS__)
 +
 +void
 +log_write(int level, const char *format, ...) ISC_FORMAT_PRINTF(2, 3);
  
  #endif /* !_LD_LOG_H_ */
 -- 
 1.7.10.4
 


-- 
Adam Tkac, Red Hat, Inc.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0041] Cleanup in logging code

2012-07-26 Thread Petr Spacek


On 07/26/2012 10:06 AM, Adam Tkac wrote:

On Wed, Jul 25, 2012 at 03:31:34PM +0200, Petr Spacek wrote:

Hello,

this patch clears logging code a bit. Adding functions like
log_info() and similar will be trivial from now.

It will be necessary for ticket #71: Log successful reconnect
https://fedorahosted.org/bind-dyndb-ldap/ticket/71


Ack.



Pushed to the master:

http://git.fedorahosted.org/git?p=bind-dyndb-ldap.git;a=commitdiff;h=b04dfcbe328a8e713597921f7a43c9c8dd801e63

Petr^2 Spacek

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 0073 Update translations

2012-07-26 Thread Petr Viktorin

Update the pot to match current source, and pull translations from 
Transifex.



Warning: when this patch is pushed, the source strings on Transifex will 
update. The old versions will be lost from the site.



The patch is quite large (5MB), so I haven't attached it here (should 
I?). Please download it from 
https://github.com/encukou/freeipa/commit/fd638306ada102204494ec2e7f1b8d2bb7f6f8b1.patch




--
Petr³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 0065 Ensure ipa-adtrust-install is run with administrator privileges and Kerberos ticket

2012-07-26 Thread Alexander Bokovoy


Hi,

When setting up AD trusts support, ipa-adtrust-install utility
needs to be run as:
  - root, for performing Samba configuration and using LDAPI/autobind
  - kinit-ed IPA admin user, to ensure proper ACIs are granted to
fetch keytab

As result, we can get rid of Directory Manager credentials in
ipa-adtrust-install

https://fedorahosted.org/freeipa/ticket/2815

This ticket also simplifies a bit the way we handle admin connection in
Service class and particulary in Service._ldap_mod() by defaulting to
LDAPI/autobind in case of running as root and to GSSAPI otherwise.
Except few cases in remote replica management (not applicable in
_ldap_mod() case) we always run installation tools as root and can
benefit from using autobind feature. Unfortunately, it is not yet
possible to get away from using DM credentials for all cases as the same
class is used to perform initial directory server instance
configuration.

One side effect is explicit disconnect and reconnect in
Service.add_cert_to_service() due to way how SimpleLDAPObject class
handles stale connections (no handling at all). I've put some comments
in place so that others would not try to err out optimizing it in
future.

Finally, with next patch series which will introduce syncing ipaNTHash
attribute with RC4 key in existing kerberos credentials, we can remove
requirements to change passwords or re-kinit for majority of trust
cases. This should then conclude our trusts content for beta2 release.


--
/ Alexander Bokovoy
From 93a84dff75560e297e775b838fc12330ebd218e5 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy aboko...@redhat.com
Date: Fri, 13 Jul 2012 18:12:48 +0300
Subject: [PATCH 2/2] Ensure ipa-adtrust-install is run with Kerberos ticket
 for admin user

When setting up AD trusts support, ipa-adtrust-install utility
needs to be run as:
   - root, for performing Samba configuration and using LDAPI/autobind
   - kinit-ed IPA admin user, to ensure proper ACIs are granted to
 fetch keytab

As result, we can get rid of Directory Manager credentials in 
ipa-adtrust-install

https://fedorahosted.org/freeipa/ticket/2815
---
 install/tools/ipa-adtrust-install   |   42 +
 install/tools/man/ipa-adtrust-install.1 |3 ---
 ipaserver/install/adtrustinstance.py|   21 ---
 ipaserver/install/service.py|   44 ++-
 4 files changed, 74 insertions(+), 36 deletions(-)

diff --git a/install/tools/ipa-adtrust-install 
b/install/tools/ipa-adtrust-install
index 
6678018e6346d75d5042894cfb833d38079d3f21..f367d5b2b516bd411bce9275ff299eb3ffdf6bf9
 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -37,8 +37,6 @@ log_file_name = /var/log/ipaserver-install.log
 
 def parse_options():
 parser = IPAOptionParser(version=version.VERSION)
-parser.add_option(-p, --ds-password, dest=dm_password,
-  sensitive=True, help=directory manager password)
 parser.add_option(-d, --debug, dest=debug, action=store_true,
   default=False, help=print debugging information)
 parser.add_option(--ip-address, dest=ip_address,
@@ -86,6 +84,30 @@ def read_netbios_name(netbios_default):
 
 return netbios_name
 
+def ensure_kerberos_admin_rights(api):
+try:
+ctx = krbV.default_context()
+ccache = ctx.default_ccache()
+principal = ccache.principal()
+api.Backend.ldap2.connect(ccache.name)
+user = api.Command.user_show(unicode(principal[0]))['result']
+group = api.Command.group_show(u'admins')['result']
+api.Backend.ldap2.disconnect()
+if not (user['uid'][0] in group['member_user'] and
+group['cn'][0] in user['memberof_group']):
+raise errors.RequirementError(name='admins group membership')
+except Exception, e:
+error_messages = dict(
+   ACIError = Outdated Kerberos credentials. Use kdestroy and kinit 
to update your ticket,
+   Krb5Error = Must have Kerberos credentials to setup AD trusts on 
server,
+   RequirementError = Must have administrative privileges to setup AD 
trusts on server
+)
+name = type(e).__name__
+if name in error_messages:
+sys.exit(error_messages[name])
+else:
+sys.exit(Unrecognized error during check of admin rights: %s\n%s 
% (name, str(e)))
+
 def main():
 safe_options, options = parse_options()
 
@@ -128,6 +150,8 @@ def main():
 api.bootstrap(**cfg)
 api.finalize()
 
+ensure_kerberos_admin_rights(api)
+
 if adtrustinstance.ipa_smb_conf_exists():
 if not options.unattended:
 while True:
@@ -194,9 +218,8 @@ def main():
 if not options.unattended and ( not netbios_name or not 
options.netbios_name):
 netbios_name = read_netbios_name(netbios_name)
 
-dm_password = options.dm_password or read_password(Directory Manager,
-

Re: [Freeipa-devel] [PATCH] 0073 Update translations

2012-07-26 Thread Petr Viktorin


On 07/26/2012 03:49 PM, Petr Viktorin wrote:

On 07/26/2012 10:17 AM, Petr Viktorin wrote:

Update the pot to match current source, and pull translations from
Transifex.


Warning: when this patch is pushed, the source strings on Transifex will
update. The old versions will be lost from the site.


The patch is quite large (5MB), so I haven't attached it here (should
I?). Please download it from
https://github.com/encukou/freeipa/commit/fd638306ada102204494ec2e7f1b8d2bb7f6f8b1.patch





NACK. I forgot to validate the messages, so some messages might cause
exceptions when used.




Validation found a few bad messages in Spanish, and one in Tajik. I've 
corrected obvious errors; in two cases I wasn't sure how to fix so I've 
removed the translation.

Attaching my fixes for reference.

The new patch is here: 
https://github.com/encukou/freeipa/commit/4eb5b21fe3bcfec33e07c45050d5a56f4328206c.patch




We should also change these on Transifex.
I assume maintainers can edit all translations. John, could you give me 
maintainer rights? My username there is EnCuKou.


--
Petr³
From c1dbc7dab3a8dc3758a00e6500321cf1c1ce638a Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Thu, 26 Jul 2012 10:31:56 -0400
Subject: [PATCH] Fix or remove bad translations

---
 install/po/es.po | 10 +-
 install/po/tg.po |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/install/po/es.po b/install/po/es.po
index c9f66c9aaedaa7a04ef5604d815d43b37f9b75ca..ba6d9f7681b0d4fb125e50b570757cacce8bda0b 100644
--- a/install/po/es.po
+++ b/install/po/es.po
@@ -2121,7 +2121,7 @@ msgid Fingerprints
 msgstr Las huellas dactilares
 
 msgid Issue New Certificate for ${entity} ${primary_key}
-msgstr Nuevo nÃºmero de certificados de 
+msgstr 
 
 msgid Issued By
 msgstr Expedido por
@@ -2163,17 +2163,17 @@ msgid Remove from CRL
 msgstr Borrar de CRL
 
 msgid Restore Certificate for ${entity} ${primary_key}
-msgstr Restaurar Certificado para 
+msgstr 
 
 msgid 
 To confirm your intention to restore this certificate, click the \Restore\ 
 button.
 msgstr 
 Para confirmar su intenciÃ³n de restaurar este certificado, haga clic en el 
 botÃ³n \Restaurar\.
 
 msgid Revoke Certificate for ${entity} ${primary_key}
-msgstr Revocar certificado por ${entity} ${primary_key
+msgstr Revocar certificado por ${entity} ${primary_key}
 
 msgid 
 To confirm your intention to revoke this certificate, select a reason from 
@@ -2201,7 +2201,7 @@ msgid Validity
 msgstr Validez
 
 msgid Certificate for ${entity} ${primary_key}
-msgstr Certificado para ${entity} ${primary_key
+msgstr Certificado para ${entity} ${primary_key}
 
 msgid Data
 msgstr Datos
@@ -2285,7 +2285,7 @@ msgid Are you sure you want to unprovision this host?
 msgstr Â¿EstÃ¡ seguro que desea unprovision este equipo?
 
 msgid Unprovisioning ${entity}
-msgstr Unprovisioning 
+msgstr 
 
 msgid Host Group Settings
 msgstr Configuraciones del Grupo de Host
diff --git a/install/po/tg.po b/install/po/tg.po
index 362a80d59511c181fe4c10117dc0ffb33d0e0913..f00fd05b99455636dbd657afcdaaea1293ab48b3 100644
--- a/install/po/tg.po
+++ b/install/po/tg.po
@@ -72,7 +72,7 @@ msgstr ÐÐ°ÐºÐ¾Ð½
 
 #, python-format
 msgid File %(file)s not found
-msgstr Ð¤Ð°Ð¹Ð»Ð¸ %(file) Ð¿Ð°Ð¹Ð´Ð¾ Ð½Ð°ÑÑÐ´.
+msgstr Ð¤Ð°Ð¹Ð»Ð¸ %(file)s Ð¿Ð°Ð¹Ð´Ð¾ Ð½Ð°ÑÑÐ´.
 
 msgid Key
 msgstr Ð¢ÑÐ³Ð¼Ð°
-- 
1.7.11.2

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 1033 renew CA subsystem certificates

2012-07-26 Thread Jan Cholasta


Dne 25.7.2012 22:58, Rob Crittenden napsal(a):

Jan Cholasta wrote:

Dne 25.7.2012 16:01, Rob Crittenden napsal(a):

Petr Viktorin wrote:

On 07/23/2012 10:03 PM, Rob Crittenden wrote:

Rob Crittenden wrote:

Andrew Wnuk wrote:

On 07/16/2012 01:35 PM, Rob Crittenden wrote:

Nalin Dahyabhai wrote:

On Mon, Jul 16, 2012 at 09:23:24AM -0400, Rob Crittenden wrote:

Use the new certmonger capability to be able to renew the dogtag
subsystem certificates (audit, OCSP, etc).


Are the copies of the certificates in the pki-ca CS.cfg file being
updated elsewhere?  Or is it not turning out to be a problem if
they
aren't?


I didn't test validating OCSP signatures but the audit subsystem
seemed fine (it complained wildly when I had the wrong trust in the
NSS db).

Andrew, do I need to update CS.cfg as well?


Yes, you may need update CS.cfg too.


Ok, added a bit to update CS.cfg with the new certificate.


This should fix some SELinux issues preventing certmonger from
monitoring the dogtag certificate database in /var/lib/pki-ca/alias.

rob


I don't know enough about dogtag/certmonger to comment on the
functionality, but there are minor issues I can find. Attaching a patch
to fix them.


`make rpms` fails:

rpmbuild --define _topdir /rpmbuild -ba freeipa.spec
error: %changelog not in descending chronological order
make: *** [rpms] Error 1



`git am` complains:

Applying: Use certmonger to renew CA subsystem certificates
/home/pviktori/freeipa/.git/rebase-apply/patch:576: new blank line at
EOF.
+
/home/pviktori/freeipa/.git/rebase-apply/patch:645: new blank line at
EOF.
+
warning: 2 lines add whitespace errors.


Thanks, integrated this patch and added a missing script, renew_ipacert.

rob



NACK


First, a question: I haven't tested this (yet), but what happens when
someone uses the --{dirsrv,http,pkinit}_pkcs12 options of
ipa-server-install/ipa-replica-prepare? (There are also other options
which I suspect may cause trouble, namely --subject and --selfsign.)


CA certs aren't tracked if --selfsign is used.

subject doesn't matter, it is unrelated to renewal.

The provided PKCS#12 files are unrelated to this patch, but in general
we will still attempt to renew the dirsrv and http certs. We
automatically track all certs using the IPA CA. If they were not issued
by the IPA CA then they will fail to be renewed.


OK, thanks.



I'm thinking we need to deprecate ipa-server-certs and document that
using the PKCS#12 options is unsupported.



install/restart_scripts/renew_ra_cert doesn't seem to be used anywhere.


This replaces the ipaCert script. I fixed up the invocation.


ipa-replica-install --setup-ca fails with:

...
   [13/15]: configure clone certificate renewals

Your system may be partly configured.
Run /usr/sbin/ipa-server-install --uninstall to clean up.

Nickname ipaCert doesn't exist in NSS database /etc/httpd/alias


Fixed.


On clones, the CN=IPA RA,O=REALM certificate is tracked with post-save
command '/usr/lib64/ipa/certmonger/restart_httpd ipaCert', but
restart_httpd does not take any arguments (it does not break anything,
it's just weird).


That's true, suppressed.



Comments on individual files follow:


install/certmonger/Makefile.am:

Missing closing parenthesis:

+EXTRA_DIST =\
+$(app_SCRIPTS   \


Fixed, and replaced some spaces with tabs.



install/certmonger/dogtag-ipa-retrieve-agent-submit:

Typo (nicknamd):


Fixed.


Are these guaranteed to be upper-case? I'd put operation.upper() here,
just to be on the safe side:


Yes, guaranteed to be upper-case from certmonger.


This except block is not necessary, unhandled exceptions are caught in
the except block lower in the code:

+sys.exit(5)
+except Exception, e:
+# Unhandled error
+sys.exit(3)
+finally:


Sure, removed. I had it there for readability but it is such little code
it can still be followed.




install/restart_scripts/restart_dirsrv:

You import and initialize api, but then don't use it.


Ah, I did at one point, then ripped it all out (or almost all,
apparently). Gone.



install/restart_scripts/*:

All these scripts could use more exception handling, but I guess
potential bugs can be sorted out later.


Well, they all run in the background so even if they caught errors
nothing would see them unless we decide to syslog errors.



install/share/default-aci.ldif:

The ACIs are wrong (Kerberos principal instead of ldap URI in userdn, in
40-delegation.update it is done right).


Nice catch, not sure how I missed that. Fixed.


You forgot to fix the allow(add) one, it still has userdn = 
host/$FQDN@$REALM.






ipapython/certmonger.py:

This is ugly:

+if sys.maxsize  2**32:
+libpath = 'lib64'
+else:
+libpath = 'lib'


I'm open to suggestions, it's the best thing I could find.


OK, it is mentioned in http://docs.python.org/library/platform.html, 
so it is probably safe.


However, I think that in future it

[Freeipa-devel] [PATCH] Minor fix to sidgen plugin

2012-07-26 Thread Simo Sorce

Remove an unnecessary check that may give spurious failures on modified server 
where 999 is a valid ID.

-- 
Simo Sorce * Red Hat, Inc. * New York
From efbe4a5d21b8567f146c9170becd54e3dd671498 Mon Sep 17 00:00:00 2001
From: Simo Sorce sso...@redhat.com
Date: Thu, 26 Jul 2012 14:30:39 -0400
Subject: [PATCH] Do not check for DNA magic values

The DNA magic value can be arbitrarily changed by admins so we cannot use a
const value to check. And we relly do not need to check at all. If the DNA
plugin is broken and leaves magic values to reach the post-op stage we have
bigger problems. So just simply get rid of this check.
---
 daemons/ipa-slapi-plugins/ipa-sidgen/ipa_sidgen.h|2 --
 daemons/ipa-slapi-plugins/ipa-sidgen/ipa_sidgen_common.c |6 --
 2 files changed, 8 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-sidgen/ipa_sidgen.h b/daemons/ipa-slapi-plugins/ipa-sidgen/ipa_sidgen.h
index cfb624bde5750d406d631cb1c250c08d1a4366a2..dec2a652464ec451ca7d32b9a82dd958202298e5 100644
--- a/daemons/ipa-slapi-plugins/ipa-sidgen/ipa_sidgen.h
+++ b/daemons/ipa-slapi-plugins/ipa-sidgen/ipa_sidgen.h
@@ -54,8 +54,6 @@
 #define IPANT_USER_ATTRS ipantuserattrs
 #define IPANT_GROUP_ATTRS ipantgroupattrs
 
-#define IPA_DNA_MAGIC 999
-
 #define IPA_PLUGIN_NAME ipa-sidgen-postop
 #define IPA_SIDGEN_FEATURE_DESC IPA SIDGEN postop plugin
 #define IPA_SIDGEN_PLUGIN_DESC Add a SID to newly added or modified  \
diff --git a/daemons/ipa-slapi-plugins/ipa-sidgen/ipa_sidgen_common.c b/daemons/ipa-slapi-plugins/ipa-sidgen/ipa_sidgen_common.c
index cbbb2ef183f2d94826a9ead20ca1fc39daa09599..d7e6ac39a57ce26cf6ac7196a1797c44e5a65f77 100644
--- a/daemons/ipa-slapi-plugins/ipa-sidgen/ipa_sidgen_common.c
+++ b/daemons/ipa-slapi-plugins/ipa-sidgen/ipa_sidgen_common.c
@@ -479,12 +479,6 @@ int find_sid_for_ldap_entry(struct slapi_entry *entry,
 goto done;
 }
 
-if (uid_number == IPA_DNA_MAGIC || gid_number == IPA_DNA_MAGIC) {
-LOG_FATAL(Looks that DNA plugin was not run before.\n);
-ret = LDAP_OPERATIONS_ERROR;
-goto done;
-}
-
 if (uid_number = UINT32_MAX || gid_number = UINT32_MAX) {
 LOG_FATAL(ID value too large.\n);
 ret = LDAP_CONSTRAINT_VIOLATION;
-- 
1.7.10.4

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] DN patch and documentation

2012-07-26 Thread John Dennis


On 07/11/2012 03:59 AM, Petr Viktorin wrote:

I went over the changes to ipaserver/plugins/ldap2.py; now I can start
the testing.

I must ask about the wrapped _ext methods. I assume python-ldap exposes
them only to mimic the C interface, which has them because C doesn't
have default arguments. Would we lose anything if the class only defined
the _ext methods, and aliased the non-ext variants to them? e.g.

  def add_ext(self, dn, modlist, serverctrls=None, clientctrls=None):
  assert isinstance(dn, DN)
  dn = str(dn)
  modlist = self.encode(modlist)
  return self.conn.add_ext(dn, modlist, serverctrls, clientctrls)
  add = add_ext

The non-_ext variants are deprecated in the underlying C library, anyway.



With the current implementation you are correct. There would be no 
functional difference if we only utilized the _ext variants. However 
that means employing knowledge from the wrong side of an API boundary. 
From our perspective python-ldap exposes certain methods. In particular 
we use the simpler methods because that is sufficient to meet our needs. 
I think we should code to the defined API. If python-ldap ever changes 
or we use an alternate Python ldap library the porting effort would be 
much cleaner if we don't try to outsmart ourselves.



Why are some of the wrapper methods left out? (compare, delete_ext,
modify, etc.)


The set of wrapped methods was chosen based on what we actually use in 
our code. Originally I set out out wrap every method, but that was a 
fair amount of work most of which was completely unnecessary because it 
would never get invoked. Why make it more complicated than it needs to 
be? If in the future we decide we need to use another method(s) we can 
add the wrapper on an as-needed basis.





Line 1519:
  checkmembers = copy.deepcopy(members)
  for member_dn in checkmembers:
  ...
  if m not in checkmembers:
  checkmembers.append(m) # FIXME jrd; can't modify list
during traversal
NACK. You really can't modify lists during traversal, a FIXME won't cut it.


Absolutely, I didn't write the code but when I saw it I realized it had 
to get fixed, the FIXME was simply a reminder to go back and clean up 
previously incorrect code I had spotted but had existed for a long time. 
Your solution below is pretty close to what I was planning to add. Fixed.



A smaller issue is that the list `in` operator does alinear scan, so
calling it in a loop can very slow as the list grows. Sets are much
better for membership tests.
So I believe you want something like:

  checkmembers = set(DN(m, locked=True) for m in members)
  checked = set()
  while checkmembers:
  member_dn = checkmembers.pop()
  checked.add(member_dn)
  ...
  if m not in checked:
  checkmembers.add(m)



Lines 203, 825:
  os.environ['KRB5CCNAME'] = ccache_file
Should KRB5CCNAME be unset/restored once the code's done with it.


Agreed, once again I didn't write this but saw the problem when I moved 
the code block and made a note to fix it in the future. This particular 
place is fixed now. However I think we've got other places that do 
similar things. This is another example of the tendency to cut-n-paste 
blocks of duplicate code which drives me nuts. But how much unrelated 
code does one change in any patch? I did however add a FIXME comment to 
get that code cleaned up. In the past I would change any problem I saw 
when I touched an area of the code but I got a lot of complaints about 
changing things not directly related to the intent of the patch :-(


FWIW, the setting of KRB5CCNAME on line 825 is somewhat correct. It's 
the identity for the duration of the request. However it's redundant 
with what the session code sets up . ldap2 really shouldn't be doing 
this, ldap2 needs some clean up. FWIW the KRBCCNAME is removed by the 
session release_ipa_ccache() method.





Line 228:
  except IndexError:
The try clause is too long for such a general exception, this can hide
real errors.


Agreed, I didn't write the code, it needs clean up, how many non-dn 
related changes do I make?








And as usual I have a lot of nitpicks. Sometimes I just want to share my
ideas about how good code should look like, don't take them too
seriously if you think otherwise :)


Line 107  127:
  return utf8_codec.decode(val)[0]
  return utf8_codec.encode(unicode(val))[0]
I believe the following would be more readable:
  return val.decode('utf-8')
  return unicode(val).encode('utf-8')


Yes, your suggestion is easier to read, but it's more efficient to 
lookup the codec once rather than everytime you decode.





Line 134:
  class ServerSchema(object):

Don't use nested classes without reason



addressed previously




Line 148:
  def get_schema(self, url, conn=None, force_update=False):

Is the force_update  flush() useful? None of the code seems to use it.


We

Re: [Freeipa-devel] DN patch and documentation

2012-07-26 Thread John Dennis


On 07/17/2012 06:47 PM, John Dennis wrote:

ipapython/dn.py:
   in docstring:  DN(arg0, ..., locked=False, first_key_match=True)
   followed by:  def __init__(self, *args, **kwds):
   and:  kwds.get('first_key_match', True)

I don't see the reason for this. Just write `def __init__(self, *args,
locked=False, first_key_match=True)` and put a proper summary in the
first line of the docstring. Same in AVA  RDN.


A valid point. I think I was trying to be too flexible/extensible.


Sorry, I was unable to make the suggested change. I tried and it 
refreshed my memory as to why it was coded the way it was.


Python considers it a syntax error if keyword arguments follow an arg 
list reference. e.g.


 def foo(*args, bar=None):
  File stdin, line 1
def foo(*args, bar=None):
 ^
SyntaxError: invalid syntax

I'm not sure why, but that's why the methods are coded as

def foo(*args, **kwds):

If you have a suggestion as to how to use named parameters in this 
context let me know or maybe explain why it's an illegal construct (I'm 
sure it's in the language reference documentation somewhere, I just 
didn't take the time to research it).


--
John Dennis jden...@redhat.com

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] DN patch and documentation

2012-07-26 Thread John Dennis

I have applied the suggested fixes, rebased against master, run all the 
unit tests successfully, built RPM's, did a full install without errors, 
and brought up the web UI successfully.


The current code can be found here:

git clone git://fedorapeople.org/~jdennis/freeipa.dn.git
git checkout dn

I did not squash the individual commits (but they should be before we 
apply to master).


Please test (again).

I continue to believe the greatest lurking liability is the installer 
code and the individual command line utilities (e.g. replica-manage, 
etc.) Aside from the server install I have not exercised those components.


--
John Dennis jden...@redhat.com

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] Minor fix to sidgen plugin

2012-07-26 Thread Alexander Bokovoy


On Thu, 26 Jul 2012, Simo Sorce wrote:

Remove an unnecessary check that may give spurious failures on modified
server where 999 is a valid ID.

ACK.

--
/ Alexander Bokovoy

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0065 Ensure ipa-adtrust-install is run with administrator privileges and Kerberos ticket

2012-07-26 Thread Alexander Bokovoy


On Thu, 26 Jul 2012, Alexander Bokovoy wrote:

Hi,

When setting up AD trusts support, ipa-adtrust-install utility
needs to be run as:
  - root, for performing Samba configuration and using LDAPI/autobind
  - kinit-ed IPA admin user, to ensure proper ACIs are granted to
fetch keytab

As result, we can get rid of Directory Manager credentials in
ipa-adtrust-install

https://fedorahosted.org/freeipa/ticket/2815

This ticket also simplifies a bit the way we handle admin connection in
Service class and particulary in Service._ldap_mod() by defaulting to
LDAPI/autobind in case of running as root and to GSSAPI otherwise.
Except few cases in remote replica management (not applicable in
_ldap_mod() case) we always run installation tools as root and can
benefit from using autobind feature. Unfortunately, it is not yet
possible to get away from using DM credentials for all cases as the same
class is used to perform initial directory server instance
configuration.

One side effect is explicit disconnect and reconnect in
Service.add_cert_to_service() due to way how SimpleLDAPObject class
handles stale connections (no handling at all). I've put some comments
in place so that others would not try to err out optimizing it in
future.

Finally, with next patch series which will introduce syncing ipaNTHash
attribute with RC4 key in existing kerberos credentials, we can remove
requirements to change passwords or re-kinit for majority of trust
cases. This should then conclude our trusts content for beta2 release.


Patch updated, fixed small typo (auth_parms was initialized as
auth_params which led to non-existing auth_parms in ipa-adtrust-install
case).

--
/ Alexander Bokovoy
From 46391aa5953426d763c0c1b627c8abbf80d6fec2 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy aboko...@redhat.com
Date: Fri, 13 Jul 2012 18:12:48 +0300
Subject: [PATCH 2/6] Ensure ipa-adtrust-install is run with Kerberos ticket
 for admin user

When setting up AD trusts support, ipa-adtrust-install utility
needs to be run as:
   - root, for performing Samba configuration and using LDAPI/autobind
   - kinit-ed IPA admin user, to ensure proper ACIs are granted to
 fetch keytab

As result, we can get rid of Directory Manager credentials in 
ipa-adtrust-install

https://fedorahosted.org/freeipa/ticket/2815
---
 install/tools/ipa-adtrust-install   |   42 +
 install/tools/man/ipa-adtrust-install.1 |3 ---
 ipaserver/install/adtrustinstance.py|   21 ---
 ipaserver/install/service.py|   44 ++-
 4 files changed, 74 insertions(+), 36 deletions(-)

diff --git a/install/tools/ipa-adtrust-install 
b/install/tools/ipa-adtrust-install
index 
6678018e6346d75d5042894cfb833d38079d3f21..f367d5b2b516bd411bce9275ff299eb3ffdf6bf9
 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -37,8 +37,6 @@ log_file_name = /var/log/ipaserver-install.log
 
 def parse_options():
 parser = IPAOptionParser(version=version.VERSION)
-parser.add_option(-p, --ds-password, dest=dm_password,
-  sensitive=True, help=directory manager password)
 parser.add_option(-d, --debug, dest=debug, action=store_true,
   default=False, help=print debugging information)
 parser.add_option(--ip-address, dest=ip_address,
@@ -86,6 +84,30 @@ def read_netbios_name(netbios_default):
 
 return netbios_name
 
+def ensure_kerberos_admin_rights(api):
+try:
+ctx = krbV.default_context()
+ccache = ctx.default_ccache()
+principal = ccache.principal()
+api.Backend.ldap2.connect(ccache.name)
+user = api.Command.user_show(unicode(principal[0]))['result']
+group = api.Command.group_show(u'admins')['result']
+api.Backend.ldap2.disconnect()
+if not (user['uid'][0] in group['member_user'] and
+group['cn'][0] in user['memberof_group']):
+raise errors.RequirementError(name='admins group membership')
+except Exception, e:
+error_messages = dict(
+   ACIError = Outdated Kerberos credentials. Use kdestroy and kinit 
to update your ticket,
+   Krb5Error = Must have Kerberos credentials to setup AD trusts on 
server,
+   RequirementError = Must have administrative privileges to setup AD 
trusts on server
+)
+name = type(e).__name__
+if name in error_messages:
+sys.exit(error_messages[name])
+else:
+sys.exit(Unrecognized error during check of admin rights: %s\n%s 
% (name, str(e)))
+
 def main():
 safe_options, options = parse_options()
 
@@ -128,6 +150,8 @@ def main():
 api.bootstrap(**cfg)
 api.finalize()
 
+ensure_kerberos_admin_rights(api)
+
 if adtrustinstance.ipa_smb_conf_exists():
 if not options.unattended:
 while True:
@@ -194,9 +218,8 @@ def main():
 if not options.unattended and ( not netbios_name or

Re: [Freeipa-devel] [PATCHES][RFC] Implement special operation to revoer NT hash for a user

2012-07-26 Thread Alexander Bokovoy


On Thu, 12 Jul 2012, Alexander Bokovoy wrote:

On Thu, 12 Jul 2012, Simo Sorce wrote:

On Thu, 2012-07-12 at 10:48 +0300, Alexander Bokovoy wrote:

On Wed, 11 Jul 2012, Simo Sorce wrote:

From 84ef09a1193ff42fc301fb71354055c5039f51a5 Mon Sep 17 00:00:00 2001
From: Simo Sorce sso...@redhat.com
Date: Fri, 6 Jul 2012 16:18:29 -0400
Subject: [PATCH] Add special modify op to regen ipaNTHash

The NT Hash is the same thing as the RC4-HMAC key, so we add a function to
extract it from krb5 keys if they are available to avoid forcing a password
change when configuring trust relationships.
---
.../ipa-pwd-extop/ipapwd_prepost.c |  147 +++-
1 file changed, 144 insertions(+), 3 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_prepost.c 
b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_prepost.c
index 
deae642f82edcc4674a1c9580661c3dae94b..24fa52eb9ac92004576ccdba4f576162c358770d
 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_prepost.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_prepost.c
@@ -41,7 +41,12 @@
#  include config.h
#endif

-#define _XOPEN_SOURCE /* strptime needs this */
+/* strptime needs _XOPEN_SOURCE and endian.h needs __USE_BSD
+ * _GNU_SOURCE imply both, and we use it elsewhere, so use this */
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE 1
+#endif
+
#include stdio.h
#include string.h
#include strings.h
@@ -53,6 +58,7 @@
#include dirsrv/slapi-plugin.h
#include lber.h
#include time.h
+#include endian.h

#include ipapwd.h
#include util.h
@@ -379,6 +385,12 @@ done:
return 0;
}

+#define NTHASH_REGEN_VAL MagicRegen
+#define NTHASH_REGEN_LEN sizeof(NTHASH_REGEN_VAL)
+static int ipapwd_regen_nthash(Slapi_PBlock *pb, Slapi_Mods *smods,
+   char *dn, struct slapi_entry *entry,
+   struct ipapwd_krbcfg *krbcfg);
+
/* PRE MOD Operation:
 * Gets the clean text password (fail the operation if the password came
 * pre-hashed, unless this is a replicated operation).
@@ -407,6 +419,7 @@ static int ipapwd_pre_mod(Slapi_PBlock *pb)
int has_krb_keys = 0;
int has_history = 0;
int gen_krb_keys = 0;
+int is_magic_regen = 0;
int ret, rc;

LOG_TRACE( =\n);
@@ -447,6 +460,27 @@ static int ipapwd_pre_mod(Slapi_PBlock *pb)
default:
break;
}
+} else if (slapi_attr_types_equivalent(lmod-mod_type, ipaNTHash)) {
+/* check op filtering out LDAP_MOD_BVALUES */
+switch (lmod-mod_op  0x0f) {
+case LDAP_MOD_REPLACE:

This is still LDAP_MOD_REPLACE, not LDAP_MOD_ADD.


This is because I resent the old patch :(

Hopefully the correct patch is now attached.

Yes, now it is updated, thanks.

I'm going to experiment a bit with these patches, adding ipasam
responder to test them.

Here is ipasam part.



--
/ Alexander Bokovoy
From 0cd261dd74154efc9ef6b09ef283cc1fe3448d5e Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy aboko...@redhat.com
Date: Thu, 26 Jul 2012 22:05:25 +0300
Subject: [PATCH 6/6] When ipaNTHash is missing, ask IPA to generate it from
 kerberos keys

---
 daemons/ipa-sam/ipa_sam.c |   96 +++--
 1 file changed, 93 insertions(+), 3 deletions(-)

diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
index 
ab4b116c5f2b3b8dae6e8309403afba5fdf86708..aa54429b5bec4b26906b2a34e59ff95299a67f80
 100644
--- a/daemons/ipa-sam/ipa_sam.c
+++ b/daemons/ipa-sam/ipa_sam.c
@@ -2400,6 +2400,74 @@ static bool init_sam_from_td(struct samu *user, struct 
pdb_trusted_domain *td,
return true;
 }
 
+static bool ipasam_nthash_retrieve(struct ldapsam_privates *ldap_state,
+  TALLOC_CTX *mem_ctx,
+  char *entry_dn,
+  DATA_BLOB *nthash)
+{
+   int ret;
+   bool retval;
+   LDAPMessage *result;
+   LDAPMessage *entry = NULL;
+   int count;
+   struct smbldap_state *smbldap_state = ldap_state-smbldap_state;
+   const char *attr_list[] = {
+   LDAP_ATTRIBUTE_NTHASH,
+   NULL
+ };
+
+   ret = smbldap_search(smbldap_state, entry_dn,
+LDAP_SCOPE_BASE, , attr_list, 0,
+result);
+   if (ret != LDAP_SUCCESS) {
+   DEBUG(1, (Failed to get NT hash: %s\n,
+ ldap_err2string (ret)));
+   return false;
+   }
+
+   count = ldap_count_entries(smbldap_state-ldap_struct, result);
+
+   if (count != 1) {
+   DEBUG(1, (Unexpected number of results [%d] for NT hash 
+ of the single entry search.\n, count));
+   ldap_msgfree(result);
+   return false;
+   }
+
+   entry = ldap_first_entry(smbldap_state-ldap_struct, result);
+   if (entry ==

[Freeipa-devel] Freeipa wiki editing

Re: [Freeipa-devel] slow response

Re: [Freeipa-devel] [PATCH 0041] Cleanup in logging code

Re: [Freeipa-devel] [PATCH 0041] Cleanup in logging code

[Freeipa-devel] [PATCH] 0073 Update translations

[Freeipa-devel] [PATCH] 0065 Ensure ipa-adtrust-install is run with administrator privileges and Kerberos ticket

Re: [Freeipa-devel] [PATCH] 0073 Update translations

Re: [Freeipa-devel] [PATCH] 1033 renew CA subsystem certificates

[Freeipa-devel] [PATCH] Minor fix to sidgen plugin

Re: [Freeipa-devel] DN patch and documentation

Re: [Freeipa-devel] DN patch and documentation

Re: [Freeipa-devel] DN patch and documentation

Re: [Freeipa-devel] [PATCH] Minor fix to sidgen plugin

Re: [Freeipa-devel] [PATCH] 0065 Ensure ipa-adtrust-install is run with administrator privileges and Kerberos ticket

Re: [Freeipa-devel] [PATCHES][RFC] Implement special operation to revoer NT hash for a user

15 matches

Site Navigation

Mail list logo

Footer information