diff mbox

[1/2] PAM support in Busybox if linux-pam is built

Message ID 1346729322-17868-1-git-send-email-golubovsky@gmail.com
State Superseded
Headers show

Commit Message

Dimitry Golubovsky Sept. 4, 2012, 3:28 a.m. UTC
Signed-off-by: Dmitry <golubovsky@gmail.com>
---
 package/busybox/Config.in  |   11 +++++++++++
 package/busybox/busybox.mk |   12 ++++++++++++
 2 files changed, 23 insertions(+), 0 deletions(-)

Comments

Yann E. MORIN Sept. 4, 2012, 6:13 p.m. UTC | #1
Dmitry, All,

On Tuesday 04 September 2012 05:28:41 Dmitry wrote:
> Signed-off-by: Dmitry <golubovsky@gmail.com>
> ---
>  package/busybox/Config.in  |   11 +++++++++++
>  package/busybox/busybox.mk |   12 ++++++++++++
>  2 files changed, 23 insertions(+), 0 deletions(-)
> 
> diff --git a/package/busybox/Config.in b/package/busybox/Config.in
> index dedcf18..2a9cbf1 100644
> --- a/package/busybox/Config.in
> +++ b/package/busybox/Config.in
> @@ -62,6 +62,17 @@ config BR2_PACKAGE_BUSYBOX_WATCHDOG
>  	  Install the watchdog daemon startup script,
>  	  that just start at the boot the busybox watchdog daemon.
>  
> +config BR2_PACKAGE_BUSYBOX_PAM
> +	bool "Enable PAM support in Busybox"
> +	default n
> +	depends on BR2_PACKAGE_LINUX_PAM
> +	help
> +	  If this item is selected, Busybox login will use the PAM stack
> +	  for local logins.

>         Local logins with null password are allowed
> +	  for users with records in /etc/passwd ("default" and "root").

> +	  The default PAM configuration file requires user accounts with
> +	  nonzero length passwords.

These two sentences tend to contradict each other. What about:

    The default PAM configuration in buildroot allows local users
    (those with records in /etc/passwd and /etc/shadow) with null
    passwords to log in.

Then the second sentence can go away, because the PAM config files patch
will come before that patch. ;-]

If you can come with another formulation that is not contradictory, that
is fine by me, too. ;-) 

>  if BR2_PACKAGE_BUSYBOX_WATCHDOG
>  
>  config BR2_PACKAGE_BUSYBOX_WATCHDOG_PERIOD
> diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
> index 33f8633..21942c6 100644
> --- a/package/busybox/busybox.mk
> +++ b/package/busybox/busybox.mk
> @@ -164,6 +164,17 @@ define BUSYBOX_INSTALL_WATCHDOG_SCRIPT
>  endef
>  endif
>  
> +ifeq ($(BR2_PACKAGE_BUSYBOX_PAM),y)
> +BUSYBOX_DEPENDENCIES += linux-pam
> +define BUSYBOX_ENABLE_PAM

To be more in line with other options, I'd suggest this be BUSYBOX_SET_PAM
(we already have a bunch of BUSYBOX_SET_XXX, although there are variations).
But I have no strong objection to keeping ENABLE.

Otherwise, LGTM. When you resubmit (with at least the help clarification):
    Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.
Dimitry Golubovsky Sept. 4, 2012, 6:23 p.m. UTC | #2
Yann,

On Tue, Sep 4, 2012 at 2:13 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:

>>         Local logins with null password are allowed
>> +       for users with records in /etc/passwd ("default" and "root").
>
>> +       The default PAM configuration file requires user accounts with
>> +       nonzero length passwords.
>
> These two sentences tend to contradict each other. What about:

No, they do not contradict each other because they are about two
different files (which may be not so clear as it is written - but I
tried to keep the text short).

The first sentence is about /etc/pam.d/login.

The second is about /etc/pam.d/default (which means any program that
calls pam_start with arbitrary program name and no match in PAM
configs uses /etc/pam.d/default).

>
> To be more in line with other options, I'd suggest this be BUSYBOX_SET_PAM
> (we already have a bunch of BUSYBOX_SET_XXX, although there are variations).
> But I have no strong objection to keeping ENABLE.

I see a whole variety of names:

define BUSYBOX_CONFIGURE_CMDS
	$(BUSYBOX_SET_LARGEFILE)
	$(BUSYBOX_SET_IPV6)
	$(BUSYBOX_SET_RPC)
	$(BUSYBOX_PREFER_STATIC)
	$(BUSYBOX_SET_MDEV)
	$(BUSYBOX_NETKITBASE)
	$(BUSYBOX_NETKITTELNET)
	$(BUSYBOX_INTERNAL_SHADOW_PASSWORDS)
	$(BUSYBOX_DISABLE_MMU_APPLETS)
	$(BUSYBOX_SET_INIT)
	$(BUSYBOX_SET_WATCHDOG)

and there is "DISABLE" ;)

I'd keep "ENABLE".

Thanks.
Dimitry Golubovsky Sept. 5, 2012, 3:19 a.m. UTC | #3
Yann,

On Tue, Sep 4, 2012 at 2:13 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:

> Otherwise, LGTM. When you resubmit (with at least the help clarification):
>     Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

OK, the patches have been reordered, the help message has been
expanded to mention both files, and where they come form, and that
users are advised to supply their own PAM config.

Thanks.
diff mbox

Patch

diff --git a/package/busybox/Config.in b/package/busybox/Config.in
index dedcf18..2a9cbf1 100644
--- a/package/busybox/Config.in
+++ b/package/busybox/Config.in
@@ -62,6 +62,17 @@  config BR2_PACKAGE_BUSYBOX_WATCHDOG
 	  Install the watchdog daemon startup script,
 	  that just start at the boot the busybox watchdog daemon.
 
+config BR2_PACKAGE_BUSYBOX_PAM
+	bool "Enable PAM support in Busybox"
+	default n
+	depends on BR2_PACKAGE_LINUX_PAM
+	help
+	  If this item is selected, Busybox login will use the PAM stack
+	  for local logins. Local logins with null password are allowed
+	  for users with records in /etc/passwd ("default" and "root").
+	  The default PAM configuration file requires user accounts with
+	  nonzero length passwords.
+
 if BR2_PACKAGE_BUSYBOX_WATCHDOG
 
 config BR2_PACKAGE_BUSYBOX_WATCHDOG_PERIOD
diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
index 33f8633..21942c6 100644
--- a/package/busybox/busybox.mk
+++ b/package/busybox/busybox.mk
@@ -164,6 +164,17 @@  define BUSYBOX_INSTALL_WATCHDOG_SCRIPT
 endef
 endif
 
+ifeq ($(BR2_PACKAGE_BUSYBOX_PAM),y)
+BUSYBOX_DEPENDENCIES += linux-pam
+define BUSYBOX_ENABLE_PAM
+	$(call KCONFIG_ENABLE_OPT,CONFIG_PAM,$(BUSYBOX_BUILD_CONFIG))
+endef
+else
+define BUSYBOX_ENABLE_PAM
+	$(call KCONFIG_DISABLE_OPT,CONFIG_PAM,$(BUSYBOX_BUILD_CONFIG))
+endef
+endif
+
 # We do this here to avoid busting a modified .config in configure
 BUSYBOX_POST_EXTRACT_HOOKS += BUSYBOX_COPY_CONFIG
 
@@ -179,6 +190,7 @@  define BUSYBOX_CONFIGURE_CMDS
 	$(BUSYBOX_DISABLE_MMU_APPLETS)
 	$(BUSYBOX_SET_INIT)
 	$(BUSYBOX_SET_WATCHDOG)
+	$(BUSYBOX_ENABLE_PAM)
 	@yes "" | $(MAKE) ARCH=$(KERNEL_ARCH) CROSS_COMPILE="$(TARGET_CROSS)" \
 		-C $(@D) oldconfig
 endef