[PATCH] SSSD: Add the options to specify a UID and GID to run as

[Jakub Hrozek]

Hi,

I guess I can safely send another round of patches that were already
tested by Michal and me.

The first patch adds new command line options --uid and --gid to all SSSD
servers, making it possible to switch to another user ID if needed. So
far all the servers still run as root.

The other patches are trivial refactoring for unit tests (2 and 3). I
don't like the #ifdefs in server.c, but I couldn't think of another
way..

The final patch is a unit test for server_setup, exercising different
options.

[Pavel Reichl]

On 10/13/2014 04:08 PM, Jakub Hrozek wrote:
> +void test_run_as_root_daemon(void **state)
> +{
> int ret;
> struct main_context *main_ctx;
> struct passwd *sssd;
> pid_t pid;
> char *pidfile;
> /* Must root as root, real or fake */
> assert_int_equal(geteuid(), 0);
> sssd = getpwnam("sssd");
> assert_non_null(sssd);

I think that sssd variable is copy&paste mistake here.
> pidfile = talloc_asprintf(NULL, "%s/%s.pid", TEST_PID_PATH, __FUNCTION__);
> /* Make sure there are no leftovers */
> unlink(pidfile);
> pid = fork();
> if (pid == 0) {
>    ret = server_setup(__FUNCTION__, FLAGS_PID_FILE,
>                       0, 0, __FUNCTION__, &main_ctx);
>    assert_int_equal(ret, 0);
>    server_loop(main_ctx);
>    exit(0);
> }
> wait_for_bg_server(pidfile);
> talloc_free(pidfile);
>
> +}

[Jakub Hrozek]

On Tue, Oct 14, 2014 at 05:11:49PM +0200, Pavel Reichl wrote:
> On 10/13/2014 04:08 PM, Jakub Hrozek wrote:
> > +void test_run_as_root_daemon(void **state)
> > +{
> > int ret;
> > struct main_context *main_ctx;
> > struct passwd *sssd;
> > pid_t pid;
> > char *pidfile;
> > /* Must root as root, real or fake */
> > assert_int_equal(geteuid(), 0);
> > sssd = getpwnam("sssd");
> > assert_non_null(sssd);
>
> I think that sssd variable is copy&paste mistake here.

Why? The test attempts to resolve the user sssd and makes sure sssd is a
valid pointer to struct passwd..

[Pavel Reichl]

On 10/13/2014 04:08 PM, Jakub Hrozek wrote:
> Hi,
>
> I guess I can safely send another round of patches that were already
> tested by Michal and me.
>
> The first patch adds new command line options --uid and --gid to all SSSD
> servers, making it possible to switch to another user ID if needed. So
> far all the servers still run as root.
>
> The other patches are trivial refactoring for unit tests (2 and 3). I
> don't like the #ifdefs in server.c, but I couldn't think of another
> way..
>
> The final patch is a unit test for server_setup, exercising different
> options.
>
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

I'm seeing this minor problem:

../../../../src/tests/cwrap/../../../src/util/domain_info_utils.c: In 
function ‘subdomain_enumerates’:
../../../../src/tests/cwrap/../../../src/util/domain_info_utils.c:77:9: 
error: ‘for’ loop initial declarations are only allowed in C99 mode
          for (int i=0; parent->sd_enumerate[i]; i++) {
          ^
../../../../src/tests/cwrap/../../../src/util/domain_info_utils.c:77:9: 
note: use option -std=c99 or -std=gnu99 to compile your code
make[3]: *** [../../../src/util/server_tests-domain_info_utils.o] Error 1

I attached patch that (applies on top of these patches) which resolves 
the problem for me. Feel free to squeeze it in.

[Jakub Hrozek]

On Tue, Oct 14, 2014 at 06:22:05PM +0200, Pavel Reichl wrote:
> On 10/13/2014 04:08 PM, Jakub Hrozek wrote:
> > Hi,
> >
> > I guess I can safely send another round of patches that were already
> > tested by Michal and me.
> >
> > The first patch adds new command line options --uid and --gid to all SSSD
> > servers, making it possible to switch to another user ID if needed. So
> > far all the servers still run as root.
> >
> > The other patches are trivial refactoring for unit tests (2 and 3). I
> > don't like the #ifdefs in server.c, but I couldn't think of another
> > way..
> >
> > The final patch is a unit test for server_setup, exercising different
> > options.
> >
> > sssd-devel mailing list
> > [e-mail]
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
> I'm seeing this minor problem:
>
> ../../../../src/tests/cwrap/../../../src/util/domain_info_utils.c: In
> function ‘subdomain_enumerates’:
> ../../../../src/tests/cwrap/../../../src/util/domain_info_utils.c:77:9:
> error: ‘for’ loop initial declarations are only allowed in C99 mode
>          for (int i=0; parent->sd_enumerate[i]; i++) {
>          ^
> ../../../../src/tests/cwrap/../../../src/util/domain_info_utils.c:77:9:
> note: use option -std=c99 or -std=gnu99 to compile your code
> make[3]: *** [../../../src/util/server_tests-domain_info_utils.o] Error 1

I think your patch addresses a legitimate problem in the old code and
should be included separately. ACK to the patch. See the attached patchset..

[Pavel Reichl]

On 10/15/2014 03:05 PM, Jakub Hrozek wrote:
> On Tue, Oct 14, 2014 at 06:22:05PM +0200, Pavel Reichl wrote:
> > On 10/13/2014 04:08 PM, Jakub Hrozek wrote:
> > > Hi,
> > >
> > > I guess I can safely send another round of patches that were already
> > > tested by Michal and me.
> > >
> > > The first patch adds new command line options --uid and --gid to all SSSD
> > > servers, making it possible to switch to another user ID if needed. So
> > > far all the servers still run as root.
> > >
> > > The other patches are trivial refactoring for unit tests (2 and 3). I
> > > don't like the #ifdefs in server.c, but I couldn't think of another
> > > way..
> > >
> > > The final patch is a unit test for server_setup, exercising different
> > > options.
> > >
> > > sssd-devel mailing list
> > > [e-mail]
> > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> >
> > I'm seeing this minor problem:
> >
> > ../../../../src/tests/cwrap/../../../src/util/domain_info_utils.c: In
> > function ‘subdomain_enumerates’:
> > ../../../../src/tests/cwrap/../../../src/util/domain_info_utils.c:77:9:
> > error: ‘for’ loop initial declarations are only allowed in C99 mode
> >           for (int i=0; parent->sd_enumerate[i]; i++) {
> >           ^
> > ../../../../src/tests/cwrap/../../../src/util/domain_info_utils.c:77:9:
> > note: use option -std=c99 or -std=gnu99 to compile your code
> > make[3]: *** [../../../src/util/server_tests-domain_info_utils.o] Error 1
>
> I think your patch addresses a legitimate problem in the old code and
> should be included separately. ACK to the patch. See the attached patchset..
>
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

ACK to all.

[Jakub Hrozek]

On Wed, Oct 15, 2014 at 05:52:01PM +0200, Pavel Reichl wrote:
> On 10/15/2014 03:05 PM, Jakub Hrozek wrote:
> > On Tue, Oct 14, 2014 at 06:22:05PM +0200, Pavel Reichl wrote:
> > > On 10/13/2014 04:08 PM, Jakub Hrozek wrote:
> > > > Hi,
> > > >
> > > > I guess I can safely send another round of patches that were already
> > > > tested by Michal and me.
> > > >
> > > > The first patch adds new command line options --uid and --gid to all SSSD
> > > > servers, making it possible to switch to another user ID if needed. So
> > > > far all the servers still run as root.
> > > >
> > > > The other patches are trivial refactoring for unit tests (2 and 3). I
> > > > don't like the #ifdefs in server.c, but I couldn't think of another
> > > > way..
> > > >
> > > > The final patch is a unit test for server_setup, exercising different
> > > > options.
> > > >
> > > > sssd-devel mailing list
> > > > [e-mail]
> > > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > >
> > > I'm seeing this minor problem:
> > >
> > > ../../../../src/tests/cwrap/../../../src/util/domain_info_utils.c: In
> > > function ‘subdomain_enumerates’:
> > > ../../../../src/tests/cwrap/../../../src/util/domain_info_utils.c:77:9:
> > > error: ‘for’ loop initial declarations are only allowed in C99 mode
> > >          for (int i=0; parent->sd_enumerate[i]; i++) {
> > >          ^
> > > ../../../../src/tests/cwrap/../../../src/util/domain_info_utils.c:77:9:
> > > note: use option -std=c99 or -std=gnu99 to compile your code
> > > make[3]: *** [../../../src/util/server_tests-domain_info_utils.o] Error 1
> >
> > I think your patch addresses a legitimate problem in the old code and
> > should be included separately. ACK to the patch. See the attached patchset..
> >
> > sssd-devel mailing list
> > [e-mail]
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
> ACK to all.

Except the one you wrote :-)

I will delay pushing these patches until the views are merged. I think
it makes sense to push the patches related to the non-root work
separately, there is a high chance of breaking stuff.

[Jakub Hrozek]

On Wed, Oct 15, 2014 at 05:52:01PM +0200, Pavel Reichl wrote:
> On 10/15/2014 03:05 PM, Jakub Hrozek wrote:
> > On Tue, Oct 14, 2014 at 06:22:05PM +0200, Pavel Reichl wrote:
> > > On 10/13/2014 04:08 PM, Jakub Hrozek wrote:
> > > > Hi,
> > > >
> > > > I guess I can safely send another round of patches that were already
> > > > tested by Michal and me.
> > > >
> > > > The first patch adds new command line options --uid and --gid to all SSSD
> > > > servers, making it possible to switch to another user ID if needed. So
> > > > far all the servers still run as root.
> > > >
> > > > The other patches are trivial refactoring for unit tests (2 and 3). I
> > > > don't like the #ifdefs in server.c, but I couldn't think of another
> > > > way..
> > > >
> > > > The final patch is a unit test for server_setup, exercising different
> > > > options.
> > > >
> > > > sssd-devel mailing list
> > > > [e-mail]
> > > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > >
> > > I'm seeing this minor problem:
> > >
> > > ../../../../src/tests/cwrap/../../../src/util/domain_info_utils.c: In
> > > function ‘subdomain_enumerates’:
> > > ../../../../src/tests/cwrap/../../../src/util/domain_info_utils.c:77:9:
> > > error: ‘for’ loop initial declarations are only allowed in C99 mode
> > >          for (int i=0; parent->sd_enumerate[i]; i++) {
> > >          ^
> > > ../../../../src/tests/cwrap/../../../src/util/domain_info_utils.c:77:9:
> > > note: use option -std=c99 or -std=gnu99 to compile your code
> > > make[3]: *** [../../../src/util/server_tests-domain_info_utils.o] Error 1
> >
> > I think your patch addresses a legitimate problem in the old code and
> > should be included separately. ACK to the patch. See the attached patchset..
> >
> > sssd-devel mailing list
> > [e-mail]
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
> ACK to all.

I found a bug in the IFP initialization. The rest of the patches is
unchanged, only ifpsrv.c changed.

[Jakub Hrozek]

On Mon, Oct 20, 2014 at 01:24:38PM +0200, Jakub Hrozek wrote:
> I found a bug in the IFP initialization. The rest of the patches is
> unchanged, only ifpsrv.c changed.

Sorry, one more respin. I added a patch to chown the debug logs.
Initially, I wanted to open the debug logs as root and then just pass
the fd, but then I realized this wouldn't work for logfile rotation. So
I'm afraid we need to chown the log files..

The other patches are unchanged, just a new one was added.

[Pavel Reichl]

On 10/20/2014 02:53 PM, Jakub Hrozek wrote:
> On Mon, Oct 20, 2014 at 01:24:38PM +0200, Jakub Hrozek wrote:
> > I found a bug in the IFP initialization. The rest of the patches is
> > unchanged, only ifpsrv.c changed.
>
> Sorry, one more respin. I added a patch to chown the debug logs.
> Initially, I wanted to open the debug logs as root and then just pass
> the fd, but then I realized this wouldn't work for logfile rotation. So
> I'm afraid we need to chown the log files..
>
> The other patches are unchanged, just a new one was added.
>
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Sorry test linking fails for me:

../../../src/db/server_tests-sysdb_search.o: In function 
`sysdb_getpwnam_with_views':
/home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:104: 
undefined reference to `sysdb_search_user_override_by_name'
/home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:126: 
undefined reference to `sysdb_add_overrides_to_object'
../../../src/db/server_tests-sysdb_search.o: In function 
`sysdb_getpwuid_with_views':
/home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:209: 
undefined reference to `sysdb_search_user_override_by_uid'
/home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:231: 
undefined reference to `sysdb_add_overrides_to_object'
../../../src/db/server_tests-sysdb_search.o: In function 
`sysdb_enumpwent_with_views':
/home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:317: 
undefined reference to `sysdb_add_overrides_to_object'
../../../src/db/server_tests-sysdb_search.o: In function 
`sysdb_getgrnam_with_views':
/home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:396: 
undefined reference to `sysdb_search_group_override_by_name'
/home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:428: 
undefined reference to `sysdb_add_overrides_to_object'
/home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:435: 
undefined reference to `sysdb_add_group_member_overrides'
../../../src/db/server_tests-sysdb_search.o: In function 
`sysdb_getgrgid_with_views':
/home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:548: 
undefined reference to `sysdb_search_group_override_by_gid'
/home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:580: 
undefined reference to `sysdb_add_overrides_to_object'
/home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:587: 
undefined reference to `sysdb_add_group_member_overrides'
../../../src/db/server_tests-sysdb_search.o: In function 
`sysdb_enumgrent_with_views':
/home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:737: 
undefined reference to `sysdb_add_overrides_to_object'
/home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:745: 
undefined reference to `sysdb_add_group_member_overrides'
../../../src/db/server_tests-sysdb_search.o: In function 
`sysdb_initgroups_with_views':
/home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:959: 
undefined reference to `sysdb_add_overrides_to_object'
../../../src/db/server_tests-sysdb_search.o: In function 
`sysdb_get_user_attr_with_views':
/home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:1083: 
undefined reference to `sysdb_search_user_override_attrs_by_name'
/home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:1105: 
undefined reference to `sysdb_add_overrides_to_object'

Do I have to apply some views patches first?
Sorry, I'm getting lost in patch-sets dependencies.
You should push it all already. :-)

[Jakub Hrozek]

On Mon, Oct 20, 2014 at 04:49:10PM +0200, Pavel Reichl wrote:
> On 10/20/2014 02:53 PM, Jakub Hrozek wrote:
> > On Mon, Oct 20, 2014 at 01:24:38PM +0200, Jakub Hrozek wrote:
> > > I found a bug in the IFP initialization. The rest of the patches is
> > > unchanged, only ifpsrv.c changed.
> >
> > Sorry, one more respin. I added a patch to chown the debug logs.
> > Initially, I wanted to open the debug logs as root and then just pass
> > the fd, but then I realized this wouldn't work for logfile rotation. So
> > I'm afraid we need to chown the log files..
> >
> > The other patches are unchanged, just a new one was added.
> >
> > sssd-devel mailing list
> > [e-mail]
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
> Sorry test linking fails for me:
>
> ../../../src/db/server_tests-sysdb_search.o: In function
> `sysdb_getpwnam_with_views':
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:104:
> undefined reference to `sysdb_search_user_override_by_name'
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:126:
> undefined reference to `sysdb_add_overrides_to_object'
> ../../../src/db/server_tests-sysdb_search.o: In function
> `sysdb_getpwuid_with_views':
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:209:
> undefined reference to `sysdb_search_user_override_by_uid'
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:231:
> undefined reference to `sysdb_add_overrides_to_object'
> ../../../src/db/server_tests-sysdb_search.o: In function
> `sysdb_enumpwent_with_views':
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:317:
> undefined reference to `sysdb_add_overrides_to_object'
> ../../../src/db/server_tests-sysdb_search.o: In function
> `sysdb_getgrnam_with_views':
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:396:
> undefined reference to `sysdb_search_group_override_by_name'
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:428:
> undefined reference to `sysdb_add_overrides_to_object'
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:435:
> undefined reference to `sysdb_add_group_member_overrides'
> ../../../src/db/server_tests-sysdb_search.o: In function
> `sysdb_getgrgid_with_views':
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:548:
> undefined reference to `sysdb_search_group_override_by_gid'
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:580:
> undefined reference to `sysdb_add_overrides_to_object'
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:587:
> undefined reference to `sysdb_add_group_member_overrides'
> ../../../src/db/server_tests-sysdb_search.o: In function
> `sysdb_enumgrent_with_views':
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:737:
> undefined reference to `sysdb_add_overrides_to_object'
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:745:
> undefined reference to `sysdb_add_group_member_overrides'
> ../../../src/db/server_tests-sysdb_search.o: In function
> `sysdb_initgroups_with_views':
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:959:
> undefined reference to `sysdb_add_overrides_to_object'
> ../../../src/db/server_tests-sysdb_search.o: In function
> `sysdb_get_user_attr_with_views':
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:1083:
> undefined reference to `sysdb_search_user_override_attrs_by_name'
> /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:1105:
> undefined reference to `sysdb_add_overrides_to_object'
>
> Do I have to apply some views patches first?
> Sorry, I'm getting lost in patch-sets dependencies.

Yes, I didn't rebase the patches on top of views. Here goes a rebased
version..
> You should push it all already. :-)

Working on it :-)

[Pavel Reichl]

On 10/20/2014 05:40 PM, Jakub Hrozek wrote:
> On Mon, Oct 20, 2014 at 04:49:10PM +0200, Pavel Reichl wrote:
> > On 10/20/2014 02:53 PM, Jakub Hrozek wrote:
> > > On Mon, Oct 20, 2014 at 01:24:38PM +0200, Jakub Hrozek wrote:
> > > > I found a bug in the IFP initialization. The rest of the patches is
> > > > unchanged, only ifpsrv.c changed.
> > >
> > > Sorry, one more respin. I added a patch to chown the debug logs.
> > > Initially, I wanted to open the debug logs as root and then just pass
> > > the fd, but then I realized this wouldn't work for logfile rotation. So
> > > I'm afraid we need to chown the log files..
> > >
> > > The other patches are unchanged, just a new one was added.
> > >
> > > sssd-devel mailing list
> > > [e-mail]
> > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> >
> > Sorry test linking fails for me:
> >
> > ../../../src/db/server_tests-sysdb_search.o: In function
> > `sysdb_getpwnam_with_views':
> > /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:104:
> > undefined reference to `sysdb_search_user_override_by_name'
> > /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:126:
> > undefined reference to `sysdb_add_overrides_to_object'
> > ../../../src/db/server_tests-sysdb_search.o: In function
> > `sysdb_getpwuid_with_views':
> > /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:209:
> > undefined reference to `sysdb_search_user_override_by_uid'
> > /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:231:
> > undefined reference to `sysdb_add_overrides_to_object'
> > ../../../src/db/server_tests-sysdb_search.o: In function
> > `sysdb_enumpwent_with_views':
> > /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:317:
> > undefined reference to `sysdb_add_overrides_to_object'
> > ../../../src/db/server_tests-sysdb_search.o: In function
> > `sysdb_getgrnam_with_views':
> > /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:396:
> > undefined reference to `sysdb_search_group_override_by_name'
> > /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:428:
> > undefined reference to `sysdb_add_overrides_to_object'
> > /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:435:
> > undefined reference to `sysdb_add_group_member_overrides'
> > ../../../src/db/server_tests-sysdb_search.o: In function
> > `sysdb_getgrgid_with_views':
> > /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:548:
> > undefined reference to `sysdb_search_group_override_by_gid'
> > /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:580:
> > undefined reference to `sysdb_add_overrides_to_object'
> > /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:587:
> > undefined reference to `sysdb_add_group_member_overrides'
> > ../../../src/db/server_tests-sysdb_search.o: In function
> > `sysdb_enumgrent_with_views':
> > /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:737:
> > undefined reference to `sysdb_add_overrides_to_object'
> > /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:745:
> > undefined reference to `sysdb_add_group_member_overrides'
> > ../../../src/db/server_tests-sysdb_search.o: In function
> > `sysdb_initgroups_with_views':
> > /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:959:
> > undefined reference to `sysdb_add_overrides_to_object'
> > ../../../src/db/server_tests-sysdb_search.o: In function
> > `sysdb_get_user_attr_with_views':
> > /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:1083:
> > undefined reference to `sysdb_search_user_override_attrs_by_name'
> > /home/developer/sssd/x86_64/src/tests/cwrap/../../../../src/tests/cwrap/../../../src/db/sysdb_search.c:1105:
> > undefined reference to `sysdb_add_overrides_to_object'
> >
> > Do I have to apply some views patches first?
> > Sorry, I'm getting lost in patch-sets dependencies.
>
> Yes, I didn't rebase the patches on top of views. Here goes a rebased
> version..
> > You should push it all already. :-)
>
> Working on it :-)

Thanks, ACK to all.
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[Jakub Hrozek]

On Mon, Oct 20, 2014 at 06:03:17PM +0200, Pavel Reichl wrote:
> Thanks, ACK to all.

master:
    * 2689efa614826d45cab60ea1186d44b8bdd243ad
    * 940c94bc9a29165987cb9d3f71c4a4ec76e7a1fc
    * e718e60607149079e5c8ddc32bdd5c90b1c7a364
    * 3fd66df4813d1410c1a6187c80e3a23395b14aed
    * 4546e283498ffe2511cb566b9159714c671e326b
    * ac40d2f2b2b2fc35c95389f5e28febd580bd2b7a

[Pavel Reichl]

On 10/20/2014 02:53 PM, Jakub Hrozek wrote:
> On Mon, Oct 20, 2014 at 01:24:38PM +0200, Jakub Hrozek wrote:
> > I found a bug in the IFP initialization. The rest of the patches is
> > unchanged, only ifpsrv.c changed.
>
> Sorry, one more respin. I added a patch to chown the debug logs.
> Initially, I wanted to open the debug logs as root and then just pass
> the fd, but then I realized this wouldn't work for logfile rotation. So
> I'm afraid we need to chown the log files..
>
> The other patches are unchanged, just a new one was added.
>
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

The changes LGTM I have just one nitpick
> +/* In cases SSSD used to run as the root user, but runs as the SSSD 
> user now,
> we need to chown the log files
> */
>
> +int chown_debug_file(const char *filename,
>                 uid_t uid, gid_t gid)
>
> +{
> char *logpath;
> const char *log_file;
> errno_t ret;
> if (filename == NULL) {
>    log_file = debug_log_file;
> } else {
>    log_file = filename;
> }
> ret = asprintf(&logpath, "%s/%s.log", LOG_PATH, log_file);
> if (ret == -1) {
>    return ENOMEM;
> }
> errno = 0;

No need to set errno as we check ret val of chown and not directly errno.
> ret = chown(logpath, uid, gid);
> free(logpath);
> if (ret != 0) {
>    ret = errno;
>    DEBUG(SSSDBG_FATAL_FAILURE, "chown failed for [%s]: [%d]\n",
>          log_file, ret);
>    return ret;
> }
> return EOK;
>
> +}

[Jakub Hrozek]

On Mon, Oct 20, 2014 at 04:53:42PM +0200, Pavel Reichl wrote:
> On 10/20/2014 02:53 PM, Jakub Hrozek wrote:
> > On Mon, Oct 20, 2014 at 01:24:38PM +0200, Jakub Hrozek wrote:
> > > I found a bug in the IFP initialization. The rest of the patches is
> > > unchanged, only ifpsrv.c changed.
> >
> > Sorry, one more respin. I added a patch to chown the debug logs.
> > Initially, I wanted to open the debug logs as root and then just pass
> > the fd, but then I realized this wouldn't work for logfile rotation. So
> > I'm afraid we need to chown the log files..
> >
> > The other patches are unchanged, just a new one was added.
> >
> > sssd-devel mailing list
> > [e-mail]
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
> The changes LGTM I have just one nitpick
> > +/* In cases SSSD used to run as the root user, but runs as the SSSD user
> > now,
> > we need to chown the log files
> > */
> >
> > +int chown_debug_file(const char *filename,
> >                 uid_t uid, gid_t gid)
> >
> > +{
> > char *logpath;
> > const char *log_file;
> > errno_t ret;
> > if (filename == NULL) {
> >    log_file = debug_log_file;
> > } else {
> >    log_file = filename;
> > }
> > ret = asprintf(&logpath, "%s/%s.log", LOG_PATH, log_file);
> > if (ret == -1) {
> >    return ENOMEM;
> > }
> > errno = 0;
>
> No need to set errno as we check ret val of chown and not directly errno.

OK, since we check the ret,  you're right it's not needed.
> > ret = chown(logpath, uid, gid);
> > free(logpath);
> > if (ret != 0) {
> >    ret = errno;
> >    DEBUG(SSSDBG_FATAL_FAILURE, "chown failed for [%s]: [%d]\n",
> >          log_file, ret);
> >    return ret;
> > }
> > return EOK;
> >
> > +}
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel