[Buildroot] [PATCH 1/2] sudo: Add ldap support for sudoers rules

[Buildroot] [PATCH 1/2] sudo: Add ldap support for sudoers rules

[Chris Frederick]

Added Config.in options to enable/disable the option, and check options
in sudo.mk to add openldap as a dependancy and compile with --with-ldap.

Signed-off-by: Chris Frederick <cdf123@cdf123.net>
---
 package/sudo/Config.in | 12 ++++++++++++
 package/sudo/sudo.mk   |  5 +++++
 2 files changed, 17 insertions(+)

diff --git a/package/sudo/Config.in b/package/sudo/Config.in
index cbef15d..2e9e35a 100644
--- a/package/sudo/Config.in
+++ b/package/sudo/Config.in
@@ -9,3 +9,15 @@ config BR2_PACKAGE_SUDO
 	  but still allow people to get their work done.
 
 	  http://www.sudo.ws/sudo/
+
+if BR2_PACKAGE_SUDO && BR2_PACKAGE_OPENLDAP
+config BR2_PACKAGE_SUDO_LDAP
+	bool "ldap integration"
+	depends on BR2_PACKAGE_SUDO
+	help
+	  Allows you to manage sudoers rules in a centralized ldap
+	  directory.
+
+	  http://www.sudo.ws/man/1.8.15/sudoers.ldap.man.html
+endif
+
diff --git a/package/sudo/sudo.mk b/package/sudo/sudo.mk
index f28312a..b4d071c 100644
--- a/package/sudo/sudo.mk
+++ b/package/sudo/sudo.mk
@@ -30,6 +30,11 @@ else
 SUDO_CONF_OPTS += --without-pam
 endif
 
+ifeq ($(BR2_PACKAGE_SUDO_LDAP),y)
+SUDO_DEPENDENCIES += openldap
+SUDO_CONF_OPTS += --with-ldap
+endif
+
 # mksigname/mksiglist needs to run on build host to generate source files
 define SUDO_BUILD_MKSIGNAME_MKSIGLIST_HOST
 	$(MAKE) $(HOST_CONFIGURE_OPTS) \
-- 
2.7.3

[Buildroot] [PATCH 2/2] postgresql: enable ldap support for pg_service.conf

[Chris Frederick]

Just needed to add --with-ldap to POSTGRESQL_CONF_OPTS and add openldap
to POSTGRESQL_DEPENDENCIES when BR2_PACKAGE_OPENLDAP is on

Signed-off-by: Chris Frederick <cdf123@cdf123.net>
---
 package/postgresql/postgresql.mk | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/package/postgresql/postgresql.mk b/package/postgresql/postgresql.mk
index 7179bbe..ab615d0 100644
--- a/package/postgresql/postgresql.mk
+++ b/package/postgresql/postgresql.mk
@@ -57,6 +57,11 @@ POSTGRESQL_DEPENDENCIES += openssl
 POSTGRESQL_CONF_OPTS += --with-openssl
 endif
 
+ifeq ($(BR2_PACKAGE_OPENLDAP),y)
+POSTGRESQL_DEPENDENCIES += openldap
+POSTGRESQL_CONF_OPTS += --with-ldap
+endif
+
 define POSTGRESQL_USERS
 	postgres -1 postgres -1 * /var/lib/pgsql /bin/sh - PostgreSQL Server
 endef
-- 
2.7.3

[Buildroot] [PATCH 2/2] postgresql: enable ldap support for pg_service.conf

[Peter Seiderer]

Hello Chris,

On Wed, 19 Oct 2016 11:08:30 -0500, Chris Frederick <cdf123@cdf123.net> wrote:

> Just needed to add --with-ldap to POSTGRESQL_CONF_OPTS and add openldap
> to POSTGRESQL_DEPENDENCIES when BR2_PACKAGE_OPENLDAP is on
> 
> Signed-off-by: Chris Frederick <cdf123@cdf123.net>
> ---
>  package/postgresql/postgresql.mk | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/package/postgresql/postgresql.mk b/package/postgresql/postgresql.mk
> index 7179bbe..ab615d0 100644
> --- a/package/postgresql/postgresql.mk
> +++ b/package/postgresql/postgresql.mk
> @@ -57,6 +57,11 @@ POSTGRESQL_DEPENDENCIES += openssl
>  POSTGRESQL_CONF_OPTS += --with-openssl
>  endif
>  
> +ifeq ($(BR2_PACKAGE_OPENLDAP),y)
> +POSTGRESQL_DEPENDENCIES += openldap
> +POSTGRESQL_CONF_OPTS += --with-ldap
> +endif
> +
>  define POSTGRESQL_USERS
>  	postgres -1 postgres -1 * /var/lib/pgsql /bin/sh - PostgreSQL Server
>  endef

Tested-by: Peter Seiderer <ps.report@gmx.net>

Compile tested (and checked for the 'build with LDAP support... yes' configure output).

Regards,
Peter

[Buildroot] [PATCH 1/2] sudo: Add ldap support for sudoers rules

[Arnout Vandecappelle]

 Hi Chris,

 Thank you for your patch. I have a few comments below. Can you fix and
resubmit? I have marked your patch as "Changes Requested" in our patch tracking
system so if you don't resubmit we will forget about it.

On 19-10-16 18:08, Chris Frederick wrote:
> Added Config.in options to enable/disable the option, and check options
> in sudo.mk to add openldap as a dependancy and compile with --with-ldap.
> 
> Signed-off-by: Chris Frederick <cdf123@cdf123.net>
> ---
>  package/sudo/Config.in | 12 ++++++++++++
>  package/sudo/sudo.mk   |  5 +++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/package/sudo/Config.in b/package/sudo/Config.in
> index cbef15d..2e9e35a 100644
> --- a/package/sudo/Config.in
> +++ b/package/sudo/Config.in
> @@ -9,3 +9,15 @@ config BR2_PACKAGE_SUDO
>  	  but still allow people to get their work done.
>  
>  	  http://www.sudo.ws/sudo/
> +
> +if BR2_PACKAGE_SUDO && BR2_PACKAGE_OPENLDAP
> +config BR2_PACKAGE_SUDO_LDAP

 As far as I know, there is no need to make this option configurable. Just
enable LDAP support when BR2_PACKASGE_OPENLDAP is selected, like you do for
postgresql.

> +	bool "ldap integration"
> +	depends on BR2_PACKAGE_SUDO
> +	help
> +	  Allows you to manage sudoers rules in a centralized ldap
> +	  directory.
> +
> +	  http://www.sudo.ws/man/1.8.15/sudoers.ldap.man.html
> +endif
> +
> diff --git a/package/sudo/sudo.mk b/package/sudo/sudo.mk
> index f28312a..b4d071c 100644
> --- a/package/sudo/sudo.mk
> +++ b/package/sudo/sudo.mk
> @@ -30,6 +30,11 @@ else
>  SUDO_CONF_OPTS += --without-pam
>  endif
>  
> +ifeq ($(BR2_PACKAGE_SUDO_LDAP),y)
> +SUDO_DEPENDENCIES += openldap
> +SUDO_CONF_OPTS += --with-ldap

 We also prefer to explicitly disable options, to avoid problems with e.g.
looking up on the host. So add

else
SUDO_CONF_OPTS += --without-ldap

or if that is not possible, a comment why not.

 Regards,
 Arnout

> +endif
> +
>  # mksigname/mksiglist needs to run on build host to generate source files
>  define SUDO_BUILD_MKSIGNAME_MKSIGLIST_HOST
>  	$(MAKE) $(HOST_CONFIGURE_OPTS) \
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 2/2] postgresql: enable ldap support for pg_service.conf

[Arnout Vandecappelle]

On 19-10-16 18:08, Chris Frederick wrote:
> Just needed to add --with-ldap to POSTGRESQL_CONF_OPTS and add openldap
> to POSTGRESQL_DEPENDENCIES when BR2_PACKAGE_OPENLDAP is on
> 
> Signed-off-by: Chris Frederick <cdf123@cdf123.net>
> ---
>  package/postgresql/postgresql.mk | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/package/postgresql/postgresql.mk b/package/postgresql/postgresql.mk
> index 7179bbe..ab615d0 100644
> --- a/package/postgresql/postgresql.mk
> +++ b/package/postgresql/postgresql.mk
> @@ -57,6 +57,11 @@ POSTGRESQL_DEPENDENCIES += openssl
>  POSTGRESQL_CONF_OPTS += --with-openssl
>  endif
>  
> +ifeq ($(BR2_PACKAGE_OPENLDAP),y)
> +POSTGRESQL_DEPENDENCIES += openldap
> +POSTGRESQL_CONF_OPTS += --with-ldap

 Same here: add --without-ldap.

 Regards,
 Arnout

> +endif
> +
>  define POSTGRESQL_USERS
>  	postgres -1 postgres -1 * /var/lib/pgsql /bin/sh - PostgreSQL Server
>  endef
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 2/2] postgresql: enable ldap support for pg_service.conf

[Peter Seiderer]

Hello Arnout, Chris,

On Fri, 21 Oct 2016 21:14:16 +0200, Arnout Vandecappelle <arnout@mind.be> wrote:

> 
> 
> On 19-10-16 18:08, Chris Frederick wrote:
> > Just needed to add --with-ldap to POSTGRESQL_CONF_OPTS and add openldap
> > to POSTGRESQL_DEPENDENCIES when BR2_PACKAGE_OPENLDAP is on
> > 
> > Signed-off-by: Chris Frederick <cdf123@cdf123.net>
> > ---
> >  package/postgresql/postgresql.mk | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/package/postgresql/postgresql.mk b/package/postgresql/postgresql.mk
> > index 7179bbe..ab615d0 100644
> > --- a/package/postgresql/postgresql.mk
> > +++ b/package/postgresql/postgresql.mk
> > @@ -57,6 +57,11 @@ POSTGRESQL_DEPENDENCIES += openssl
> >  POSTGRESQL_CONF_OPTS += --with-openssl
> >  endif
> >  
> > +ifeq ($(BR2_PACKAGE_OPENLDAP),y)
> > +POSTGRESQL_DEPENDENCIES += openldap
> > +POSTGRESQL_CONF_OPTS += --with-ldap
> 
>  Same here: add --without-ldap.

Not needed for this option (as with openssl), because there is no
auto detection in case the option is not given (as opposed to the
readline and zlib options), see [1] for discussion of this...

Should have added a comment about this back in 2014...

Regards,
Peter

[1] http://lists.busybox.net/pipermail/buildroot/2014-March/093215.html

> 
>  Regards,
>  Arnout
> 
> > +endif
> > +
> >  define POSTGRESQL_USERS
> >  	postgres -1 postgres -1 * /var/lib/pgsql /bin/sh - PostgreSQL Server
> >  endef
> > 
> 

[Buildroot] [PATCH 1/2] sudo: Add ldap support for sudoers rules

[Chris Frederick]

Hi Arnout,

Comments below...

On 10/21/16 14:13, Arnout Vandecappelle wrote:
>  Hi Chris,
> 
>  Thank you for your patch. I have a few comments below. Can you fix and
> resubmit? I have marked your patch as "Changes Requested" in our patch tracking
> system so if you don't resubmit we will forget about it.
> 
> On 19-10-16 18:08, Chris Frederick wrote:
>> Added Config.in options to enable/disable the option, and check options
>> in sudo.mk to add openldap as a dependancy and compile with --with-ldap.
>>
>> Signed-off-by: Chris Frederick <cdf123@cdf123.net>
>> ---
>>  package/sudo/Config.in | 12 ++++++++++++
>>  package/sudo/sudo.mk   |  5 +++++
>>  2 files changed, 17 insertions(+)
>>
>> diff --git a/package/sudo/Config.in b/package/sudo/Config.in
>> index cbef15d..2e9e35a 100644
>> --- a/package/sudo/Config.in
>> +++ b/package/sudo/Config.in
>> @@ -9,3 +9,15 @@ config BR2_PACKAGE_SUDO
>>  	  but still allow people to get their work done.
>>  
>>  	  http://www.sudo.ws/sudo/
>> +
>> +if BR2_PACKAGE_SUDO && BR2_PACKAGE_OPENLDAP
>> +config BR2_PACKAGE_SUDO_LDAP
> 
>  As far as I know, there is no need to make this option configurable. Just
> enable LDAP support when BR2_PACKASGE_OPENLDAP is selected, like you do for
> postgresql.

From my experience with sudo built with ldap, it stops checking /etc/sudoers for rules and instead looks for /etc/ldap.conf.  According to the
link in the help text, /etc/sudoers is only read for defaults, everything else needs to come from ldap.  And if /etc/ldap.conf hasn't been
configured by the user, sudo will fail with a "no valid sudoers sources found" error.  So I wanted to make sure a user explicitly selected the
option.  That's just been my experience using this in buildroot and gentoo.  I should probably add that to the help.

How about this:

help:
  Allows you to manage sudoers rules in a centralized ldap
  directory.  This restricts the /etc/sudoers file from
  defining rules, only defaults will be read. All rules will
  need to be provided via ldap configured in /etc/ldap.conf

  http://www.sudo.ws/man/1.8.15/sudoers.ldap.man.html

>> +	bool "ldap integration"
>> +	depends on BR2_PACKAGE_SUDO
>> +	help
>> +	  Allows you to manage sudoers rules in a centralized ldap
>> +	  directory.
>> +
>> +	  http://www.sudo.ws/man/1.8.15/sudoers.ldap.man.html
>> +endif
>> +
>> diff --git a/package/sudo/sudo.mk b/package/sudo/sudo.mk
>> index f28312a..b4d071c 100644
>> --- a/package/sudo/sudo.mk
>> +++ b/package/sudo/sudo.mk
>> @@ -30,6 +30,11 @@ else
>>  SUDO_CONF_OPTS += --without-pam
>>  endif
>>  
>> +ifeq ($(BR2_PACKAGE_SUDO_LDAP),y)
>> +SUDO_DEPENDENCIES += openldap
>> +SUDO_CONF_OPTS += --with-ldap
> 
>  We also prefer to explicitly disable options, to avoid problems with e.g.
> looking up on the host. So add
> 
> else
> SUDO_CONF_OPTS += --without-ldap

Will do.

> or if that is not possible, a comment why not.
> 
>  Regards,
>  Arnout
> 
>> +endif
>> +
>>  # mksigname/mksiglist needs to run on build host to generate source files
>>  define SUDO_BUILD_MKSIGNAME_MKSIGLIST_HOST
>>  	$(MAKE) $(HOST_CONFIGURE_OPTS) \
>>
> 

[Buildroot] [PATCH 1/2] sudo: Add ldap support for sudoers rules

[Arnout Vandecappelle]

On 24-10-16 16:10, Chris Frederick wrote:
> Hi Arnout,
> 
> Comments below...
> 
> On 10/21/16 14:13, Arnout Vandecappelle wrote:
>> >  Hi Chris,
>> > 
>> >  Thank you for your patch. I have a few comments below. Can you fix and
>> > resubmit? I have marked your patch as "Changes Requested" in our patch tracking
>> > system so if you don't resubmit we will forget about it.
>> > 
>> > On 19-10-16 18:08, Chris Frederick wrote:
>>> >> Added Config.in options to enable/disable the option, and check options
>>> >> in sudo.mk to add openldap as a dependancy and compile with --with-ldap.
>>> >>
>>> >> Signed-off-by: Chris Frederick <cdf123@cdf123.net>
>>> >> ---
>>> >>  package/sudo/Config.in | 12 ++++++++++++
>>> >>  package/sudo/sudo.mk   |  5 +++++
>>> >>  2 files changed, 17 insertions(+)
>>> >>
>>> >> diff --git a/package/sudo/Config.in b/package/sudo/Config.in
>>> >> index cbef15d..2e9e35a 100644
>>> >> --- a/package/sudo/Config.in
>>> >> +++ b/package/sudo/Config.in
>>> >> @@ -9,3 +9,15 @@ config BR2_PACKAGE_SUDO
>>> >>  	  but still allow people to get their work done.
>>> >>  
>>> >>  	  http://www.sudo.ws/sudo/
>>> >> +
>>> >> +if BR2_PACKAGE_SUDO && BR2_PACKAGE_OPENLDAP
>>> >> +config BR2_PACKAGE_SUDO_LDAP
>> > 
>> >  As far as I know, there is no need to make this option configurable. Just
>> > enable LDAP support when BR2_PACKASGE_OPENLDAP is selected, like you do for
>> > postgresql.
> From my experience with sudo built with ldap, it stops checking /etc/sudoers for rules and instead looks for /etc/ldap.conf.  According to the
> link in the help text, /etc/sudoers is only read for defaults, everything else needs to come from ldap.  And if /etc/ldap.conf hasn't been
> configured by the user, sudo will fail with a "no valid sudoers sources found" error.  So I wanted to make sure a user explicitly selected the
> option.  That's just been my experience using this in buildroot and gentoo.  I should probably add that to the help.
> 
> How about this:
> 
> help:
>   Allows you to manage sudoers rules in a centralized ldap
>   directory.  This restricts the /etc/sudoers file from
>   defining rules, only defaults will be read. All rules will
>   need to be provided via ldap configured in /etc/ldap.conf
> 
>   http://www.sudo.ws/man/1.8.15/sudoers.ldap.man.html

 OK, then indeed it's better to make it an option.

 One small coding style nit: write it like:

if BR2_PACKAGE_SUDO
config BR2_PACKAGE_SUDO_LDAP
	depends on BR2_PACKAGE_OPENLDAP

so future sudo extensions can reuse the same conditions - not likely, but htis
is the pattern that we follow everywhere else.

 Normally we would use 'select' instead of 'depends on', but in this case
'depends' is indeed better.

 Please also summarise the above in the commit message. And you can add the
following to the commit message too:

Since the user explicitly has to provide /etc/ldap.conf, we use 'depends on' so
that the user is obliged to explicitly enable openldap before the option becomes
visible.

 Regards,
 Arnout


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 2/2] postgresql: enable ldap support for pg_service.conf

[Thomas Petazzoni]

Hello,

On Mon, 24 Oct 2016 09:13:18 -0500, Chris Frederick wrote:
> Just needed to add --with-ldap to POSTGRESQL_CONF_OPTS and add openldap
> to POSTGRESQL_DEPENDENCIES when BR2_PACKAGE_OPENLDAP is on
> 
> Signed-off-by: Chris Frederick <cdf123@cdf123.net>
> ---
>  package/postgresql/postgresql.mk | 7 +++++++
>  1 file changed, 7 insertions(+)

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 2/2] postgresql: enable ldap support for pg_service.conf

[Chris Frederick]

Just needed to add --with-ldap to POSTGRESQL_CONF_OPTS and add openldap
to POSTGRESQL_DEPENDENCIES when BR2_PACKAGE_OPENLDAP is on

Signed-off-by: Chris Frederick <cdf123@cdf123.net>
---
 package/postgresql/postgresql.mk | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/package/postgresql/postgresql.mk b/package/postgresql/postgresql.mk
index e2756bf..1b184e7 100644
--- a/package/postgresql/postgresql.mk
+++ b/package/postgresql/postgresql.mk
@@ -57,6 +57,13 @@ POSTGRESQL_DEPENDENCIES += openssl
 POSTGRESQL_CONF_OPTS += --with-openssl
 endif
 
+ifeq ($(BR2_PACKAGE_OPENLDAP),y)
+POSTGRESQL_DEPENDENCIES += openldap
+POSTGRESQL_CONF_OPTS += --with-ldap
+else
+POSTGRESQL_CONF_OPTS += --without-ldap
+endif
+
 define POSTGRESQL_USERS
 	postgres -1 postgres -1 * /var/lib/pgsql /bin/sh - PostgreSQL Server
 endef
-- 
2.7.3