Re: [SSSD] [PATCH] IPA: Don't hang onto memory longer than necessary

2012-07-02 Thread Simo Sorce
On Mon, 2012-07-02 at 11:24 -0400, Stephen Gallagher wrote: > This request and attached memory would be freed at the end of > access-check processing, but it's a waste to keep it around. ACK Simo. -- Simo Sorce * Red Hat, Inc * New York _

Re: [SSSD] [PATCHES] Add Active Directory identity, auth and chpass providers to SSSD

2012-07-02 Thread Simo Sorce
On Mon, 2012-07-02 at 18:18 +0200, Stef Walter wrote: > On 07/02/2012 06:02 PM, Simo Sorce wrote: > > 1. > > You should never allow to set a domain that differs from the realm name > > in the AD provider, it is always assumed realm = domain in AD. > > > > In AD

Re: [SSSD] [PATCHES] Add Active Directory identity, auth and chpass providers to SSSD

2012-07-02 Thread Simo Sorce
ontrib spec (forgotten earlier). > Manual code review looks fine now. Although I haven't actually tested the code myself I would give an ACK here. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fed

Re: [SSSD] [PATCH] LDAP: Print extended failure message for SASL bind

2012-07-02 Thread Simo Sorce
tracking down issues while working on the Active Directory ID provider. > > > > Nack, > > you are comparing optret to LDAP_SUCCESS while you should be comparing it > > to > > EOK. > > > Thanks for catching that. Fixed in the atta

Re: [SSSD] No negative cache with sssd_nss

2012-07-02 Thread Simo Sorce
ated file). Ah I see, you are paying the penalty for a full roundtrip to sssd_nss for each file created. I think we can work on adding some negative caching to the memory cache so to reduce from one call every file to one call every 5 seconds or so. Would you mind opening a ticket for it ? Simo

Re: [SSSD] No negative cache with sssd_nss

2012-07-02 Thread Simo Sorce
On Tue, 2012-07-03 at 04:02 +0200, Jan Engelhardt wrote: > On Tuesday 2012-07-03 03:44, Simo Sorce wrote: > > > >The memory cache does not do negative caching indeed, but sssd_nss does, > >however it caches negative entries for a very short period of time. > &g

Re: [SSSD] [PATCH] pac responder: limit access by checking UIDs

2012-07-05 Thread Simo Sorce
der > uses it. > > I took a quite strict default here, i.e. only root is allowed to > access > the PAC responder by default. Is this too restrictive? > Patch looks good, but I wonder why you do not allow specifying user names, a getpwnam() is not too expensive. Simo. -

Re: [SSSD] [PATCH] pac responder: limit access by checking UIDs

2012-07-05 Thread Simo Sorce
On Thu, 2012-07-05 at 18:51 +0200, Sumit Bose wrote: > On Thu, Jul 05, 2012 at 09:12:16AM -0400, Simo Sorce wrote: > > On Thu, 2012-07-05 at 14:06 +0200, Sumit Bose wrote: > > > > > > > > > Hi, > > > > > > this patch added the checks req

Re: [SSSD] [PATCH] pac responder: limit access by checking UIDs

2012-07-05 Thread Simo Sorce
On Thu, 2012-07-05 at 21:01 +0200, Sumit Bose wrote: > On Thu, Jul 05, 2012 at 01:30:02PM -0400, Simo Sorce wrote: > > On Thu, 2012-07-05 at 18:51 +0200, Sumit Bose wrote: > > > On Thu, Jul 05, 2012 at 09:12:16AM -0400, Simo Sorce wrote: > > > > On Thu, 2012-07-05 at

Re: [SSSD] [PATCHES] Additional AD provider enhancements

2012-07-06 Thread Simo Sorce
ool() please use dom->case_sensitive that you just set, and not 'false' directly, this way if we ever need a change (or someone copy/paste the code elsewhere) we are consistent and only one line needs to be changed to change behavior. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] MAN: Unify "SEE ALSO" sections

2012-07-06 Thread Simo Sorce
the complete list in every manpage, > but I think this is better than having manpages no one knows about. > > This patch will be a prerequisite of my AD provider manpages. ACK. I wish we could simply autogenerate the list of pages. Simo. -- Simo So

Re: [SSSD] [PATCHES] Additional AD provider enhancements

2012-07-06 Thread Simo Sorce
On Fri, 2012-07-06 at 10:59 -0400, Stephen Gallagher wrote: > On Fri, 2012-07-06 at 08:09 -0400, Simo Sorce wrote: > > On Thu, 2012-07-05 at 20:53 -0400, Stephen Gallagher wrote: > > > > > > > > > These patches should finish off the AD provider functionality

Re: [SSSD] [PATCH] pac responder: limit access by checking UIDs

2012-07-10 Thread Simo Sorce
On Mon, 2012-07-09 at 18:00 +0200, Sumit Bose wrote: > On Thu, Jul 05, 2012 at 03:05:37PM -0400, Simo Sorce wrote: > > On Thu, 2012-07-05 at 21:01 +0200, Sumit Bose wrote: > > > On Thu, Jul 05, 2012 at 01:30:02PM -0400, Simo Sorce wrote: > > > > On Thu, 2012-07-05 at

Re: [SSSD] [PATCHES] Add support for multiple servers in the Kerberos locator plugin

2012-07-17 Thread Simo Sorce
nd > repeated calls to the cbfunc() callback, providing all of the > kdcinfo-specified servers as values. Clients using libkrb5 will > transparently try all of the provided servers until finding one that > works. Haven't really looked at the rest as the first nack unfort

Re: [SSSD] [PATCHES] Add support for multiple servers in the Kerberos locator plugin

2012-07-18 Thread Simo Sorce
On Wed, 2012-07-18 at 08:05 -0400, Stephen Gallagher wrote: > On Tue, 2012-07-17 at 17:46 -0400, Simo Sorce wrote: > > On Tue, 2012-07-17 at 13:36 -0400, Stephen Gallagher wrote: > > > This is the first half of the work necessary to resolve > > > https://fedor

Re: [SSSD] [PATCH] sdap_sudo.c: add missing end of line in few debug messages

2012-07-23 Thread Simo Sorce
On Mon, 2012-07-23 at 12:47 +0200, Pavel Březina wrote: > SSIA ack -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] sudo: provide automatic configuration of machine hostnames

2012-07-24 Thread Simo Sorce
On Tue, 2012-07-24 at 12:21 +0200, Pavel Březina wrote: > +#include > +#include // HOST_NAME_MAX > +#include I guess you attached the old patch. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list s

Re: [SSSD] [PATCHES][PRELIMINARY] Move SELinux processing to account PAM stack

2012-07-25 Thread Simo Sorce
ticket/1439 > > I realize that there might be some rough edges to sand off but right now the > important thing for me is to know whether the approach implemented in patch > #162 and described in the comment #1 in the ticket is valid. > NACK, we discussed a better approach on IRC.

Re: [SSSD] [PATCHES][PRELIMINARY] Move SELinux processing to account PAM stack

2012-07-26 Thread Simo Sorce
- Original Message - > Dne středa 25 července 2012 10:19:04, Simo Sorce napsal(a): > > On Wed, 2012-07-25 at 08:54 +0200, Jan Zelený wrote: > > > #161 - Rename session provider to selinux provider > > > #162 - Move SELinux provider processing right after PAM_A

Re: [SSSD] Legacy Systems Gateway Scenario

2012-07-27 Thread Simo Sorce
e general problem what we need is a bit more complex interface into SSSD. Finally, changing the client protocol is generally frowned on as it can easily break installation on upgrade (some processes may have the old plugin in memory and sssd starts requiring the new format).

Re: [SSSD] [PATCH][SET] Cleanups and subdomain refactoring

2012-07-31 Thread Simo Sorce
On Tue, 2012-07-31 at 13:25 +0200, Jakub Hrozek wrote: > On Mon, Jul 23, 2012 at 05:38:50PM -0400, Simo Sorce wrote: > > As aprt of the work for ticket #1380 I have created a long patchset that > > cleans up and refactors some of the ipa subdomain work. > > > > Thi

Re: [SSSD] [PATCH] Create a domain-realm mapping for krb5.conf to be included

2012-08-01 Thread Simo Sorce
ucting the contents and then writing > * the file is first created with mkstemp's default restrictive > permissions and then chmod-ed to be readable > * failure to write the file is not fatal anymore NACK, ret is overwritten in erro conditi

Re: [SSSD] Passing the torch

2012-08-01 Thread Simo Sorce
inds his feet in this new > responsibility. Congratulations, and good luck! > ___ > sssd-devel mailing list > sssd-devel@lists.fedorahosted.org > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel -- Simo Sorce * Red Hat, Inc * Ne

Re: [SSSD] [PATCH] Create a domain-realm mapping for krb5.conf to be included

2012-08-01 Thread Simo Sorce
On Wed, 2012-08-01 at 21:38 +0200, Jakub Hrozek wrote: > On Wed, Aug 01, 2012 at 03:21:33PM -0400, Simo Sorce wrote: > > On Wed, 2012-08-01 at 21:09 +0200, Jakub Hrozek wrote: > > > On Wed, Aug 01, 2012 at 07:13:22PM +0200, Jakub Hrozek wrote: > > > > When new subdo

Re: [SSSD] Only create the SELinux login file if there are mappings on the server

2012-08-06 Thread Simo Sorce
, file_content); > +ret = write_selinux_login_file(pd->user, file_content); > +} else { > +ret = remove_selinux_login_file(pd->user); > } See above comment, shoudln;t this be done after the 'done' label ? > done: > @@ -796,7 +826,7 @@ static

Re: [SSSD] [PATCH] SYSDB: Use ldb_msg_add_string for simple string additions

2012-08-06 Thread Simo Sorce
On Mon, 2012-08-06 at 12:44 +0200, Jakub Hrozek wrote: > > > Simo did the same change in the subdomains sysdb module. The attached > patch performs the changes in other sysdb modules, too. > ACK Simo. -- Simo Sorce * Red Hat,

Re: [SSSD] [PATCH] IPA: Do not attempt to close the same file twice

2012-08-06 Thread Simo Sorce
On Mon, 2012-08-06 at 09:38 -0400, Stephen Gallagher wrote: > > > Fixes https://fedorahosted.org/sssd/ticket/1456 > ACK. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahost

Re: [SSSD] [PATCH] IPA: Securely set umask for mkstemp in subdomain provider

2012-08-06 Thread Simo Sorce
is already ok (strict), I asked Jakub to not set/unset the umask in his last patch exactly because we are going to do a chmod later anyway. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.

Re: [SSSD] [PATCH] When ldap_group_nesting_level was reached, the LDAP provider, tried to link group members with groups outside nesting, limit.

2012-08-06 Thread Simo Sorce
num_added == 0) { continue; } else { add[num_added] = NULL; } The major question I have is that this new code introduces O(N^2) behavior, if there are more then a handful of groups it will be quite costly. Can we find a way that is not so expensive ? Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] Backward GOTOs rewritten into do-while loops.

2012-08-07 Thread Simo Sorce
) { > /* buffer too small ? */ here please use a do/whiel loop to make code more readable: do { status = ctx->ops.initgrou ... } while (status == NSS_STATUS_TRYAGAIN); Simo. -- Simo Sorce * Red Hat, Inc * New York

Re: [SSSD] [PATCH] delete entries with entryUSN > lastUSN when lastUSN changes

2012-08-07 Thread Simo Sorce
d)entryUSN > (new)lastUSN you amy end up simply removing *all* entries for no good reason, withy a lot of churn in the ldb files due to memebrship removals etc, and lost of cached password for users. I guess this is a NACK on the approach unless I grossly misunderstood something. Simo. -- Simo

Re: [SSSD] [PATCH] When ldap_group_nesting_level was reached, the LDAP provider, tried to link group members with groups outside nesting, limit.

2012-08-07 Thread Simo Sorce
On Tue, 2012-08-07 at 15:40 +0200, Michal Zidek wrote: > On 08/06/2012 08:57 PM, Simo Sorce wrote: > > The major question I have is that this new code introduces O(N^2) > > behavior, if there are more then a handful of groups it will be quite > > costly. Can we find

Re: [SSSD] [PATCH] When ldap_group_nesting_level was reached, the LDAP provider, tried to link group members with groups outside nesting, limit.

2012-08-07 Thread Simo Sorce
On Tue, 2012-08-07 at 13:15 -0400, Stephen Gallagher wrote: > On Tue, 2012-08-07 at 19:08 +0200, Michal Zidek wrote: > > On 08/07/2012 04:11 PM, Simo Sorce wrote: > > > On Tue, 2012-08-07 at 15:40 +0200, Michal Zidek wrote: > > >> On 08/06/2012 08:57 PM, Simo

Re: [SSSD] [PATCH] Backward GOTOs rewritten into do-while loops.

2012-08-08 Thread Simo Sorce
solution really. Simo. > Ondrej > > On 08/07/2012 03:58 PM, Simo Sorce wrote: > > One comment and one nitpick. > > > > Comment: polease use --patience flag to git format-patch so that the > > patches are more readable. > > > > Nitpick: > > &g

Re: [SSSD] [PATCH] Backward GOTOs rewritten into do-while loops.

2012-08-08 Thread Simo Sorce
d like to know what other think about this. A very slow nss module may end up having it's call killed midair ... Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedor

Re: [SSSD] Files 'group' and 'passwd' in /var/lib/sss/mc/ still present even after sssd quits

2012-08-16 Thread Simo Sorce
- Original Message - > On Thu, Aug 16, 2012 at 05:32:08PM +0200, Michal Židek wrote: > > Turning off sssd does not remove files in /var/lib/sss/mc. This > > allows to respond on requests like > > 'id username' with sssd turned off. Is this behavior OK or is it a > > bug? I think it is not

Re: [SSSD] Files 'group' and 'passwd' in /var/lib/sss/mc/ still present even after sssd quits

2012-08-17 Thread Simo Sorce
- Original Message - > On 08/16/2012 11:18 PM, Simo Sorce wrote: > > > > - Original Message - > >> On Thu, Aug 16, 2012 at 05:32:08PM +0200, Michal Židek wrote: > >>> Turning off sssd does not remove files in /var/lib/sss/mc. This > >>&

Re: [SSSD] [PATCH] Fix alignment issues reported by clang

2012-08-21 Thread Simo Sorce
NACK, casting to void does not resolve alignment issues, it just conceals them from the compiler. For example in krb5_child_handler.c use SAFEALIGN_SET_UINT32() to set expiration. or msg_subtype. Simo. ___ sssd-devel mailing list sssd-devel@lists.fedo

Re: [SSSD] [PATCH] Fix alignment issues reported by clang

2012-08-21 Thread Simo Sorce
- Original Message - > On 21/08/12 05:51, Simo Sorce wrote: > > NACK, > > casting to void does not resolve alignment issues, it just conceals > > them from the compiler. > > > > For example in krb5_child_handler.c use SAFEALIGN_SET_UINT32() to &

Re: [SSSD] [PATCH] Out-of-bounds read fix in hmac-sha-1

2012-09-07 Thread Simo Sorce
starting right after the key itself - > out-of-bounds. I haven't looked at the rest of the code, but a more defensive way of doing the test is checking key_len < HMAC_SHA1_BLOCKSIZE, and not just checking it is different. Simo. -- Simo

Re: [SSSD] [PATCH] Out-of-bounds read fix in hmac-sha-1

2012-09-07 Thread Simo Sorce
On Fri, 2012-09-07 at 14:25 +0200, Jakub Hrozek wrote: > On Fri, Sep 07, 2012 at 08:20:29AM -0400, Simo Sorce wrote: > > On Fri, 2012-09-07 at 10:56 +0200, Ondrej Kos wrote: > > > fixes https://fedorahosted.org/sssd/ticket/1331 > > > > > > Although the giv

[SSSD] [PATCH] Remove obsolete comment

2012-09-11 Thread Simo Sorce
okos on IRC pointed out this comment does not reflect the code anymore. The comment is old and need to be removed as the logic it refers to has been removed. Simo. -- Simo Sorce * Red Hat, Inc * New York >From b9164bfe32760d9f1f68cdb587836cef0b6fa30d Mon Sep 17 00:00:00 2001 From: Simo So

Re: [SSSD] [PATCHES] sss_cache tool invalidates records in memory cache

2012-09-13 Thread Simo Sorce
errno_t sss_mmap_cache_gr_store(struct sss_mc_ctx > *mcc, > gid_t gid, size_t memnum, > char *membuf, size_t memsize); > > +errno_t sss_mmap_cache_invalidate(struct sss_mc_ctx*); > + > #endif /* _NSSSRV_MMAP_CAC

Re: [SSSD] [PATCHES] sss_cache tool invalidates records in memory cache

2012-09-14 Thread Simo Sorce
On Fri, 2012-09-14 at 17:16 +0200, Michal Židek wrote: > On 09/13/2012 07:59 PM, Simo Sorce wrote: > > Main questions: > > the invalidation functions seem to be called unconditionally, what > > prevent cache invalidation durint a normal log rotation ? > > Shouldn&

Re: [SSSD] [PATCHES] sss_cache tool invalidates records in memory cache

2012-09-17 Thread Simo Sorce
id above) and the failure should only be logged, not be fatal (of course this is not a problem if we move back checks into sssd_nss). Simo. -- Simo Sorce * Red Hat, Inc. * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] do not check for POLLERR, POLLHUP, POLLNVAL when reading data

2012-10-04 Thread Simo Sorce
ssfully completed the read ? Or is there something else I am missing here ? Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] do not check for POLLERR, POLLHUP, POLLNVAL when reading data

2012-10-04 Thread Simo Sorce
n, but from the way you wrote the comment to me it reads like you mention 'output' as meaning 'write event' (ie POLLOUT) ? Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] do not check for POLLERR, POLLHUP, POLLNVAL when reading data

2012-10-08 Thread Simo Sorce
On Fri, 2012-10-05 at 10:48 +0200, Pavel Březina wrote: > On 10/04/2012 04:22 PM, Simo Sorce wrote: > > On Thu, 2012-10-04 at 13:03 +0200, Pavel Březina wrote: > >> From 7c49fe46a3fbce1433ad1680a2e12335542706ed Mon Sep 17 00:00:00 2001 > >> From: =?UTF-8?q?Pavel=20B=C

Re: [SSSD] [PATCH] PAM: close socket fd with pam_set_data

2012-10-11 Thread Simo Sorce
which means it will still point to a random file descriptor. Also I do not understand why this whole dance in passing pointers and the sss_pam_get_socket() function, seem all very unnecessary. . sss_cli_sd is a global variable which means: a) you can access it directly b) you need mutexes to access it (see sss_pam_make_request()). Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] Split the providers into separate subpackages

2012-10-11 Thread Simo Sorce
and authenticate against an Active Directory > server. > + > %package -n libsss_idmap > Summary: FreeIPA Idmap library > Group: Development/Libraries > @@ -205,7 +271,7 @@ used by Python applications. > Summary: A library to allow communication between SUDO and SSSD > Group: Development/Libraries > License: LGPLv3+ > -Requires: sssd = %{version}-%{release} > +Requires: sssd-ldap = %{version}-%{release} > Requires(post): /sbin/ldconfig > Requires(postun): /sbin/ldconfig why libsss_idmap would require the sssd-ldap subpakage ? Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] Create ghost users when a user DN is encountered in IPA

2012-10-11 Thread Simo Sorce
DIT. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] Alignment issues reported by clang.

2012-10-11 Thread Simo Sorce
> Comment: Same as number 4, but we also use addresses that are multiples > of the sizeof(data_type_used) (relative to the beginning of the buffer). > Which is OK. > > > > The rest of the warnings are suppr

Re: [SSSD] [PATCH] Create ghost users when a user DN is encountered in IPA

2012-10-11 Thread Simo Sorce
On Thu, 2012-10-11 at 19:47 +0200, Jakub Hrozek wrote: > On Thu, Oct 11, 2012 at 09:44:46AM -0400, Simo Sorce wrote: > > On Thu, 2012-10-11 at 10:52 +0200, Jakub Hrozek wrote: > > > The IPA has a defined directory tree structure that allows us to guess > > > the userna

Re: [SSSD] [PATCH] PAM: close socket fd with pam_set_data

2012-10-11 Thread Simo Sorce
On Thu, 2012-10-11 at 20:23 +0200, Jakub Hrozek wrote: > On Thu, Oct 11, 2012 at 09:30:29AM -0400, Simo Sorce wrote: > > On Thu, 2012-10-11 at 10:00 +0200, Jakub Hrozek wrote: > > > From 98c8a6b92db2872083473b4ce0761bffc919e847 Mon Sep 17 00:00:00 2001 > > > From: Jakub

Re: [SSSD] [PATCH] Create ghost users when a user DN is encountered in IPA

2012-10-11 Thread Simo Sorce
On Thu, 2012-10-11 at 20:25 +0200, Jakub Hrozek wrote: > On Thu, Oct 11, 2012 at 02:06:22PM -0400, Simo Sorce wrote: > > On Thu, 2012-10-11 at 19:47 +0200, Jakub Hrozek wrote: > > > On Thu, Oct 11, 2012 at 09:44:46AM -0400, Simo Sorce wrote: > > > > On Thu, 2012-10-1

Re: [SSSD] [PATCH] Alignment issues reported by clang.

2012-10-11 Thread Simo Sorce
es to make it clearer what buffer are we referring to. Thanks for double checking with me, this code is sometimes tricky and alignment issues make it trickier. All the other alignment fixes look good to me, but the patch is quite bit, I wouldn't mind if a second pair of eyes could check and

Re: [SSSD] [PATCH] Create ghost users when a user DN is encountered in IPA

2012-10-12 Thread Simo Sorce
On Thu, 2012-10-11 at 15:24 -0400, Simo Sorce wrote: > On Thu, 2012-10-11 at 20:25 +0200, Jakub Hrozek wrote: > > On Thu, Oct 11, 2012 at 02:06:22PM -0400, Simo Sorce wrote: > > > On Thu, 2012-10-11 at 19:47 +0200, Jakub Hrozek wrote: > > > > On Thu, Oct 11, 2012 a

Re: [SSSD] [PATCH] Only call krb5_set_trace_callback on platforms that support it

2012-10-12 Thread Simo Sorce
that #ifdefs the > > tracing if the API is not available. > > Simo nacked the original patch on IRC. A new patch is attached that > keeps the #ifdefs at one place. Excellent! ACK. Simo. -- Simo Sorce * Red Hat, Inc * New York ___

Re: [SSSD] [PATCH] MAN: improve wording of default_domain parameter

2012-10-12 Thread Simo Sorce
On Sun, 2012-10-07 at 17:25 +0200, Jakub Hrozek wrote: > attached.. Ack, Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] LDAP: Check validity of naming_context

2012-10-15 Thread Simo Sorce
On Mon, 2012-10-15 at 17:42 +0200, Jakub Hrozek wrote: > https://fedorahosted.org/sssd/ticket/1581 > > If the namingContext attribute had no values or multiple values, then > our code would dereference a NULL pointer. ACK Simo. -- Simo Sorce * Red Hat, In

Re: [SSSD] [PATCH] use systemd by default on fedora15+

2012-10-18 Thread Simo Sorce
md after GA. Did you mean to put F16 there ? Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] exit original process after sssd is initialized

2012-10-18 Thread Simo Sorce
ched. > > I haven't really had time to read the patch too carefully yet, but my > first thought was to always try to use tevent signals if you need > signals at all. It is generally mandatory to use tevent for signals, however if it is in the monitor it might be a special case, sorr

Re: [SSSD] [PATCHES] Allow password authentication for sub-domains

2012-10-18 Thread Simo Sorce
ncipal does >not belong to our realm Sumit you have nice comments here, and none in the git commits, Can you please rebase the patchset and add nice comments in the actual commits. They will come handy much after this review has been done and buried in the archives when someone is looking fo

Re: [SSSD] [PATCHES] Allow password authentication for sub-domains

2012-10-18 Thread Simo Sorce
On Thu, 2012-10-18 at 17:12 -0400, Simo Sorce wrote: > On Thu, 2012-10-18 at 22:36 +0200, Sumit Bose wrote: > > Hi, > > > > I'm sorry but this patch series is a bit longer than I originally > > expected. While testing password authentication I came across some >

Re: [SSSD] [PATCH] exit original process after sssd is initialized

2012-10-19 Thread Simo Sorce
gt; Using tevent for this case seems like using a sledgehammer to crack a > nut. Pavel if this is before we create the tevent event context than it is ok to directly handle signals, however put a comment there taht says so. example: /* We use signals directly here because we don't have a tevent context yet */ Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] Fix two errors in the nss responder

2012-10-19 Thread Simo Sorce
On Fri, 2012-10-19 at 18:18 +0200, Sumit Bose wrote: > Hi, > > while testing other stuff I found those two issues. The second one is ... wow. Good job. ACK. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list s

Re: [SSSD] Unable to change ldap user passwords as the root user

2012-10-22 Thread Simo Sorce
ver hosted on a completely different machine. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[SSSD] [PATCH 0/5] RFC: Fix various tevent_req style and naming issues

2012-10-23 Thread Simo Sorce
t before I get to it, that is also welcome. Simo. Simo Sorce (5): Fix tevent_req style for krb5_auth Fix ipa_subdomain_id names and tevent_req style Fix tevent_req style for get_netgroup in ipa_id Streamline ipa_account_info handler Use an entry type mask macro to filter entry types src

[SSSD] [PATCH 2/5] Fix ipa_subdomain_id names and tevent_req style

2012-10-23 Thread Simo Sorce
Signed-off-by: Simo Sorce --- src/providers/ipa/ipa_id.c| 5 +-- src/providers/ipa/ipa_id.h| 10 ++--- src/providers/ipa/ipa_subdomains_id.c | 73 ++- 3 files changed, 36 insertions(+), 52 deletions(-) diff --git a/src/providers/ipa

[SSSD] [PATCH 1/5] Fix tevent_req style for krb5_auth

2012-10-23 Thread Simo Sorce
No functionality changes, just make the code respect the tevent_req style and naming conventions and enhance readability by adding some helper functions. Signed-off-by: Simo Sorce --- src/providers/krb5/krb5_access.c | 6 +- src/providers/krb5/krb5_auth.c | 556

[SSSD] [PATCH 4/5] Streamline ipa_account_info handler

2012-10-23 Thread Simo Sorce
Signed-off-by: Simo Sorce --- src/providers/ipa/ipa_id.c | 128 - 1 file changed, 69 insertions(+), 59 deletions(-) diff --git a/src/providers/ipa/ipa_id.c b/src/providers/ipa/ipa_id.c index ab0d8924013941943babb32a96f03560aea8c7f3

[SSSD] [PATCH 3/5] Fix tevent_req style for get_netgroup in ipa_id

2012-10-23 Thread Simo Sorce
Also do not intermix two tevent_req sequences Signed-off-by: Simo Sorce --- src/providers/ipa/ipa_id.c | 151 + 1 file changed, 71 insertions(+), 80 deletions(-) diff --git a/src/providers/ipa/ipa_id.c b/src/providers/ipa/ipa_id.c index

[SSSD] [PATCH 5/5] Use an entry type mask macro to filter entry types

2012-10-23 Thread Simo Sorce
Avoids hardcodinf magic numbers everywhere and selfdocuments why a mask is being applied. Signed-off-by: Simo Sorce --- src/providers/data_provider.h | 1 + src/providers/ipa/ipa_id.c| 2 +- src/providers/ipa/ipa_subdomains_id.c | 2 +- src/providers/ldap/ldap_id.c

Re: [SSSD] [PATCH] KRB5: Return error when principal selection fails

2012-10-23 Thread Simo Sorce
On Tue, 2012-10-23 at 15:22 +0200, Jakub Hrozek wrote: > The bug was causing segfaults in the ldap_child if the keytab was > misconfigured. > > https://fedorahosted.org/sssd/ticket/1594 ACK Simo. -- Simo Sorce * Red Hat, Inc * New York

[SSSD] [PATCH 1/4] Code can only check for cached passwords

2012-10-23 Thread Simo Sorce
Make it clear to the API users that we can not take arbitrary auth tokens. We can only take a password for now so simplify and clarify the interface. Signed-off-by: Simo Sorce --- src/db/sysdb.h | 3 +-- src/db/sysdb_ops.c | 12 +--- src/providers/krb5

[SSSD] [PATCH 0/4] Create and use an auth token object

2012-10-23 Thread Simo Sorce
patchset larger. Fixes: https://fedorahosted.org/sssd/ticket/1586 Simo Sorce (4): Code can only check for cached passwords Add function to safely wipe memory. Add authtok utility functions. Change pam data auth tokens. Makefile.am| 4 + src/db

[SSSD] [PATCH 2/4] Add function to safely wipe memory.

2012-10-23 Thread Simo Sorce
This is useful for wiping passwords, as it prevents the compiler from optimizing out a memset to zero before a free() Signed-off-by: Simo Sorce --- src/util/util.c | 9 + src/util/util.h | 10 ++ 2 files changed, 19 insertions(+) diff --git a/src/util/util.c b/src/util/util.c

[SSSD] [PATCH 3/4] Add authtok utility functions.

2012-10-23 Thread Simo Sorce
These functions allow handling of auth tokens in a completely opaque way, with clear semantics and accessor fucntions that guarantee consistency, proper access to data and error conditions. Signed-off-by: Simo Sorce --- Makefile.am| 2 + src/util/authtok.c | 146

Re: [SSSD] [PATCH] sudo refresh: handle errors properly

2012-10-23 Thread Simo Sorce
On Tue, 2012-10-23 at 14:39 +0200, Pavel Březina wrote: > I found this while working on #1596. > > In some scenario we ended up in printing uninitialized dp_error and > error in debug message. ACK Simo. -- Simo Sorce * Red Hat, In

Re: [SSSD] [PATCH] sss_cache: Remove fastcache even if sssd is not running.

2012-10-24 Thread Simo Sorce
ule. > I did not want to make this function non static and put it to a header > file because it is not intended to be used directly. This is right, thanks for not doing it. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCHES] Allow password authentication for sub-domains

2012-10-24 Thread Simo Sorce
= ENOMEM; > +goto done; > +} If this operation is done after inspecting a MS-PAC shouldn't we use the principal that comes with it ? For AD domains especially that is important because AD domains can use a UPN that differs from the ac

Re: [SSSD] [PATCHES] Allow password authentication for sub-domains

2012-10-26 Thread Simo Sorce
the canonical domain name even > if the user used the flat/short name at the login prompt > 0019: makes sub-domains case-insensitive > Ack and pushed to Master and sssd-1-9 Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mail

Re: [SSSD] [PATCH] sss_cache: Remove fastcache even if sssd is not running.

2012-10-26 Thread Simo Sorce
On Fri, 2012-10-26 at 18:33 +0200, Michal Židek wrote: > On 10/24/2012 05:04 PM, Simo Sorce wrote: > > In sssd_nss you also need to take a F_WRLCK on the first byte of the > > file, however in sssd_nss case you want to retry a few times just in > > case you races with sss_cach

Re: [SSSD] [PATCH] sss_cache: Remove fastcache even if sssd is not running.

2012-10-26 Thread Simo Sorce
DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to mark memory cache file > %s " > + "as recycled.\n", mc_filename)); > +goto done; > +} > + > +ret = EOK; > +done: > +if (locked) { > +mc_lock.l_type = F_UNLCK; > +

Re: [SSSD] [PATCH] sss_cache: Remove fastcache even if sssd is not running.

2012-10-29 Thread Simo Sorce
On Mon, 2012-10-29 at 12:53 +0100, Michal Židek wrote: > On 10/29/2012 12:41 PM, Michal Židek wrote: > > On 10/26/2012 07:53 PM, Simo Sorce wrote: > >> On Fri, 2012-10-26 at 18:33 +0200, Michal Židek wrote: > >>> --- a/src/responder/nss/nsssrv_mmap_cache.c

Re: [SSSD] [PATCH] sss_cache: Remove fastcache even if sssd is not running.

2012-10-30 Thread Simo Sorce
On Tue, 2012-10-30 at 10:39 +0100, Michal Židek wrote: > On 10/29/2012 03:53 PM, Simo Sorce wrote: > > > > Codewise looks ok, but I still see duplication of the code used to lock > > the file. > > > > I was wondering, would it make sense to split this patch

Re: [SSSD] [PATCHES] change responder contexts hierarchy

2012-10-30 Thread Simo Sorce
we do not need to special case > responder shut down in dp request destructor anymore. More information > may be found in the list archive: > https://lists.fedorahosted.org/pipermail/sssd-devel/2012-October/011663.html Nack please do not call talloc_free froma destructor, it is not

Re: [SSSD] [PATCHES] change responder contexts hierarchy

2012-10-30 Thread Simo Sorce
t table as the first thing when deallocating rctx > > Isn't the order guaranteed by the talloc itself? The children are freed in arbitrary order, however all children of a specific context are freed before the parent. Simo. -- Simo Sorce * Red Hat, Inc * New York

Re: [SSSD] [PATCH] LDAP: Fix off-by-one error when saving ghost user

2012-10-31 Thread Simo Sorce
On Wed, 2012-10-31 at 18:30 +0100, Jakub Hrozek wrote: > https://fedorahosted.org/sssd/ticket/1612 ACK. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.

Re: [SSSD] [PATCH 1/1] LDAP: Better debug logging when saving groups

2012-10-31 Thread Simo Sorce
n", > + name ? name : "Unknown", > + strerror(ret))); Given you already print the strerror() in all previous debug messages, does it make sense to repeat it here ? (Otherwise ACK, patch looks good) Simo. -- Simo Sorce * Red Hat, Inc * New York _

Re: [SSSD] [PATCH 1/1] LDAP: Better debug logging when saving groups

2012-10-31 Thread Simo Sorce
On Wed, 2012-10-31 at 14:36 -0400, Stephen Gallagher wrote: > On Wed 31 Oct 2012 02:16:02 PM EDT, Simo Sorce wrote: > > On Wed, 2012-10-31 at 13:18 -0400, Stephen Gallagher wrote: > >> fail: > >> -DEBUG(2, ("Failed to save group [%s]\n", >

[SSSD] [PATCH 0/5] Fix various tevent_req style and naming issues

2012-10-31 Thread Simo Sorce
tches work fine for me, so I am now dropping the RFC and asking for review and inclusion in master. Simo Sorce (5): Fix tevent_req style for krb5_auth Fix ipa_subdomain_id names and tevent_req style Fix tevent_req style for get_netgroup in ipa_id Streamline ipa_account_info handler Use an

[SSSD] [PATCH 2/5] Fix ipa_subdomain_id names and tevent_req style

2012-10-31 Thread Simo Sorce
--- src/providers/ipa/ipa_id.c|5 +- src/providers/ipa/ipa_id.h| 10 ++-- src/providers/ipa/ipa_subdomains_id.c | 73 + 3 files changed, 36 insertions(+), 52 deletions(-) diff --git a/src/providers/ipa/ipa_id.c b/src/providers/ipa/ip

[SSSD] [PATCH 4/5] Streamline ipa_account_info handler

2012-10-31 Thread Simo Sorce
--- src/providers/ipa/ipa_id.c | 128 1 files changed, 69 insertions(+), 59 deletions(-) diff --git a/src/providers/ipa/ipa_id.c b/src/providers/ipa/ipa_id.c index ab0d8924013941943babb32a96f03560aea8c7f3..7afa6df6fbd841309d33866caf080be12eac170f 10

[SSSD] [PATCH 3/5] Fix tevent_req style for get_netgroup in ipa_id

2012-10-31 Thread Simo Sorce
Also do not intermix two tevent_req sequences --- src/providers/ipa/ipa_id.c | 151 +--- 1 files changed, 71 insertions(+), 80 deletions(-) diff --git a/src/providers/ipa/ipa_id.c b/src/providers/ipa/ipa_id.c index f61236b965b77ca1058a14cb7e425ac2ff65723e

[SSSD] [PATCH 1/5] Fix tevent_req style for krb5_auth

2012-10-31 Thread Simo Sorce
No functionality changes, just make the code respect the tevent_req style and naming conventions and enhance readability by adding some helper functions. --- src/providers/krb5/krb5_access.c |6 +- src/providers/krb5/krb5_auth.c | 556 -- src/provider

[SSSD] [PATCH 5/5] Use an entry type mask macro to filter entry types

2012-10-31 Thread Simo Sorce
Avoids hardcodinf magic numbers everywhere and selfdocuments why a mask is being applied. --- src/providers/data_provider.h |1 + src/providers/ipa/ipa_id.c|2 +- src/providers/ipa/ipa_subdomains_id.c |2 +- src/providers/ldap/ldap_id.c |2 +- src/provi

[SSSD] [PATCH 2/4] Add function to safely wipe memory.

2012-10-31 Thread Simo Sorce
This is useful for wiping passwords, as it prevents the compiler from optimizing out a memset to zero before a free() --- src/util/util.c |9 + src/util/util.h | 10 ++ 2 files changed, 19 insertions(+), 0 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index b8

[SSSD] [PATCH 0/4] Create and use an auth token object

2012-10-31 Thread Simo Sorce
://fedorahosted.org/sssd/ticket/1586 Simo. Simo Sorce (4): Code can only check for cached passwords Add function to safely wipe memory. Add authtok utility functions. Change pam data auth tokens. Makefile.am|4 + src/db/sysdb.h

[SSSD] [PATCH 3/4] Add authtok utility functions.

2012-10-31 Thread Simo Sorce
= \ diff --git a/src/util/authtok.c b/src/util/authtok.c new file mode 100644 index ..1f45953378021e9d30559030326134794965b240 --- /dev/null +++ b/src/util/authtok.c @@ -0,0 +1,195 @@ +/* + SSSD - auth utils + + Copyright (C) Simo Sorce 2012 + + This

<    4   5   6   7   8   9   10   11   12   13   >