[PATCH] PAM Support for Busybox Login
Denis Vlasenko
vda.linux at googlemail.com
Sun Oct 1 10:00:02 UTC 2006
On Friday 29 September 2006 23:26, Thaddeus Ternes wrote:
> Previously, a patch for Busybox login was shared on the list that
> added PAM support to 1.0.0. That patch no longer applies to 1.2.1, so
> I did some work to fix it up, as well as make it a bit easier to build
> (thanks for the help on that, Jason).
>
> Here's the original patch:
> http://www.busybox.net/lists/busybox/2004-December/013257.html
>
> And attached is my swing at it (should patch cleanly against 1.2.1).
>
> All comments appreciated.
+#define PWLOOKUP \
+ if (!( pw = getpwnam ( username ))) { \
+ pw_copy.pw_name = "UNKNOWN"; \
+ pw_copy.pw_passwd = "!"; \
+ opt_fflag = 0; \
+ failed = 1; \
+ } else \
+ pw_copy = *pw; \
+ pw = &pw_copy; \
Please do it this way:
#define PWLOOKUP do { statement; statement; } while(0)
It will behave exactly like single C statement then.
+#ifdef CONFIG_PAM
#ifdef does not detect typos in macro names. #if ENABLE_PAM is better.
+ // TODO: decide how this comment should be worded (since we have the MACRO above)
+ /* PAM authentication goes before (and instead of) all the
+ * other stuff. The reason we initialize the pw struct
+ * before going on with PAM is that the pw struct is used
+ * later on, and this way saved us some changing of that code.
+ */
+ if ((pamret = pam_start( "login", username, &conv, &pamh )) != PAM_SUCCESS)
Generally avoid assignments inside if().
pamret = pam_start( "login", username, &conv, &pamh );
if (pamret != PAM_SUCCESS)
generates the same assembly code, but easier to read.
It is not an iron rule. In nested "else ifs", for example,
it is ok...
+ {
+ /* pam_start() failed. We failed to initialize the PAM
+ * structures, and should probably carry on with login,
+ * so we avoid locking people out.
+ */
+ fprintf(stderr, "PAM initialization failed: %s\n", pam_strerror(pamh, pamret));
+ failed = 1;
1. bb_error_msg will do the same, but smaller (no stderr parameter, no \n)
2. Indentation is broken. (At least
+ goto auth_ok;
+ }
+ else
+ { // continuing with pam authentication
Regarding '{': usually we do
if() {
...
} else {
+ // set TTY (so things like securetty work)
+ if((pamret = pam_set_item(pamh, PAM_TTY, tty)) != PAM_SUCCESS)
+ {
+ fprintf(stderr, "Failed to pam_set_item TTY: %s\n", pam_strerror(pamh, pamret));
+ failed = 1;
+ goto auth_ok;
+ }
+ else if ( ( pamret = pam_authenticate( pamh, 0 ) ) == PAM_SUCCESS )
+ {
+ // Then check that the account is healthy.
+ if ( ( pamret = pam_acct_mgmt( pamh, 0 ) ) != PAM_SUCCESS ) // No, it isn't
+ fprintf( stderr, "User not allowed access: %s\n",pam_strerror( pamh, pamret ) );
+ else
+ { // read user back
+ char *pamuser;
+ if(pam_get_item(pamh, PAM_USER, (const void **) &pamuser)!= PAM_SUCCESS)
+ fprintf(stderr, "pam_get_item failed on username\n");
+ else
+ strcpy(username, pamuser);
+ }
+ }
+ // If we get here, the user was authenticated, and is
+ // granted access.
+ if ( pam_end( pamh, pamret ) != PAM_SUCCESS )
We typically format it like this: if (pam_end(pamh, pamret) != PAM_SUCCESS)
+ fprintf( stderr, "PAM cleaning up failed\n" );
+
+ if ( pamret == PAM_SUCCESS )
+ failed = 0;
+ else
+ failed = 1;
+ PWLOOKUP
+ goto auth_ok;
+ }
+ // Everything from here to auth_ok: is skipped when running
+ // PAM. This is all PAM's responsibility anyway. The exception
+ // is if pam_start() failed, which would indicate a more serious
+ // error in the PAM library. In that case, we carry on with
+ // standard procedure.
+#else
+ PWLOOKUP
+#endif /* CONFIG_PAM */
if (( pw-> pw_passwd [0] == '!' ) || ( pw-> pw_passwd[0] == '*' ))
failed = 1;
Generally I like it.
Maybe will add CONFIG_DESKTOP to make such options
visible only for fat "desktop replacement"-class bbox.
So that embedded people will not feel like
bbox is going to be horribly bloated now.
--
vda
More information about the busybox
mailing list