Date: Fri, 13 Jul 2012 16:14:17 -0600 From: "Justin T. Gibbs" <gibbs@FreeBSD.org> To: current@FreeBSD.org Cc: des@FreeBSD.org Subject: PAM passwdqc, strict aliasing, and WARNS Message-ID: <F7FA2B4D-BD56-4EC4-9D93-B5AA847063D7@FreeBSD.org>
next in thread | raw e-mail | index | archive | help
--Apple-Mail=_410A4854-04E8-4B9A-B54B-38F6603F628C
Content-Transfer-Encoding: 7bit
Content-Type: text/plain;
charset=us-ascii
Someone who has yet to confess added -Werror to the global CFLAGS
(via /etc/make.conf) for one of our systems at work. Before I
figured out that this was the cause of builds failing, I hacked up
pam_passwdc to resolve the problem. This gets the module to
WARNS=2, but to go farther, the "logically const" issues with this
code will need to be sorted out.
Is this change worth committing? Is this the best way to resolve
the strict aliasing issues in this code?
Thanks,
Justin
--Apple-Mail=_410A4854-04E8-4B9A-B54B-38F6603F628C
Content-Disposition: attachment;
filename=pam_passwdqc.diff
Content-Type: application/octet-stream;
name="pam_passwdqc.diff"
Content-Transfer-Encoding: 7bit
Change 575701 by justing@justing_ns1_spectrabsd on 2012/07/13 15:57:57
Make the PAM password strength checking module WARNS=2 safe.
lib/libpam/modules/pam_passwdqc/Makefile:
Bump WARNS to 2.
contrib/pam_modules/pam_passwdqc/pam_passwdqc.c:
Bump _XOPEN_SOURCE and _XOPEN_VERSION from 500 to 600
so that vsnprint() is declared.
Use the two new union types (pam_conv_item_t and
pam_text_item_t) to resolve strict aliasing violations
caused by casts to comply with the pam_get_item() API taking
a "const void **" for all item types.
Correct a CLANG "printf-like function with more arguments
than format" warning.
Affected files ...
... //SpectraBSD/stable/contrib/pam_modules/pam_passwdqc/pam_passwdqc.c#2 edit
... //SpectraBSD/stable/lib/libpam/modules/pam_passwdqc/Makefile#2 edit
Differences ...
==== //SpectraBSD/stable/contrib/pam_modules/pam_passwdqc/pam_passwdqc.c#2 (text) ====
@@ -2,9 +2,9 @@
* Copyright (c) 2000-2002 by Solar Designer. See LICENSE.
*/
-#define _XOPEN_SOURCE 500
+#define _XOPEN_SOURCE 600
#define _XOPEN_SOURCE_EXTENDED
-#define _XOPEN_VERSION 500
+#define _XOPEN_VERSION 600
#include <stdio.h>
#include <stdlib.h>
#include <stdarg.h>
@@ -38,7 +38,16 @@
#else
#define lo_const const
#endif
-typedef lo_const void *pam_item_t;
+
+typedef union {
+ struct pam_conv *conv;
+ lo_const void *item;
+} pam_conv_item_t;
+
+typedef union {
+ char *text;
+ lo_const void *item;
+} pam_text_item_t;
#include "passwdqc.h"
@@ -132,21 +141,21 @@
static int converse(pam_handle_t *pamh, int style, lo_const char *text,
struct pam_response **resp)
{
- struct pam_conv *conv;
+ pam_conv_item_t conv;
struct pam_message msg, *pmsg;
int status;
- status = pam_get_item(pamh, PAM_CONV, (pam_item_t *)&conv);
+ status = pam_get_item(pamh, PAM_CONV, &conv.item);
if (status != PAM_SUCCESS)
return status;
pmsg = &msg;
msg.msg_style = style;
- msg.msg = text;
+ msg.msg = (char *)text;
*resp = NULL;
- return conv->conv(1, (lo_const struct pam_message **)&pmsg, resp,
- conv->appdata_ptr);
+ return conv.conv->conv(1, (lo_const struct pam_message **)&pmsg, resp,
+ conv.conv->appdata_ptr);
}
#ifdef __GNUC__
@@ -294,8 +303,11 @@
}
if (argc) {
- say(pamh, PAM_ERROR_MSG, getuid() != 0 ?
- MESSAGE_MISCONFIGURED : MESSAGE_INVALID_OPTION, *argv);
+ if (getuid() != 0) {
+ say(pamh, PAM_ERROR_MSG, MESSAGE_MISCONFIGURED);
+ } else {
+ say(pamh, PAM_ERROR_MSG, MESSAGE_INVALID_OPTION, *argv);
+ }
return PAM_ABORT;
}
@@ -311,7 +323,7 @@
#ifdef HAVE_SHADOW
struct spwd *spw;
#endif
- char *user, *oldpass, *newpass, *randompass;
+ pam_text_item_t user, oldpass, newpass, randompass;
const char *reason;
int ask_oldauthtok;
int randomonly, enforce, retries_left, retry_wanted;
@@ -353,34 +365,34 @@
if (flags & PAM_PRELIM_CHECK)
return status;
- status = pam_get_item(pamh, PAM_USER, (pam_item_t *)&user);
+ status = pam_get_item(pamh, PAM_USER, &user.item);
if (status != PAM_SUCCESS)
return status;
- status = pam_get_item(pamh, PAM_OLDAUTHTOK, (pam_item_t *)&oldpass);
+ status = pam_get_item(pamh, PAM_OLDAUTHTOK, &oldpass.item);
if (status != PAM_SUCCESS)
return status;
if (params.flags & F_NON_UNIX) {
pw = &fake_pw;
- pw->pw_name = user;
+ pw->pw_name = user.text;
pw->pw_gecos = "";
} else {
- pw = getpwnam(user);
+ pw = getpwnam(user.text);
endpwent();
if (!pw)
return PAM_USER_UNKNOWN;
if ((params.flags & F_CHECK_OLDAUTHTOK) && getuid() != 0) {
- if (!oldpass)
+ if (!oldpass.text)
status = PAM_AUTH_ERR;
else
#ifdef HAVE_SHADOW
if (!strcmp(pw->pw_passwd, "x")) {
- spw = getspnam(user);
+ spw = getspnam(user.text);
endspent();
if (spw) {
- if (strcmp(crypt(oldpass, spw->sp_pwdp),
- spw->sp_pwdp))
+ if (strcmp(crypt(oldpass.text,
+ spw->sp_pwdp), spw->sp_pwdp))
status = PAM_AUTH_ERR;
memset(spw->sp_pwdp, 0,
strlen(spw->sp_pwdp));
@@ -388,7 +400,7 @@
status = PAM_AUTH_ERR;
} else
#endif
- if (strcmp(crypt(oldpass, pw->pw_passwd),
+ if (strcmp(crypt(oldpass.text, pw->pw_passwd),
pw->pw_passwd))
status = PAM_AUTH_ERR;
}
@@ -405,13 +417,14 @@
enforce = params.flags & F_ENFORCE_ROOT;
if (params.flags & F_USE_AUTHTOK) {
- status = pam_get_item(pamh, PAM_AUTHTOK,
- (pam_item_t *)&newpass);
+ status = pam_get_item(pamh, PAM_AUTHTOK, &newpass.item);
if (status != PAM_SUCCESS)
return status;
- if (!newpass || (check_max(¶ms, pamh, newpass) && enforce))
+ if (!newpass.text ||
+ (check_max(¶ms, pamh, newpass.text) && enforce))
return PAM_AUTHTOK_ERR;
- reason = _passwdqc_check(¶ms.qc, newpass, oldpass, pw);
+ reason = _passwdqc_check(¶ms.qc, newpass.text,
+ oldpass.text, pw);
if (reason) {
say(pamh, PAM_ERROR_MSG, MESSAGE_WEAKPASS, reason);
if (enforce)
@@ -457,13 +470,13 @@
return status;
}
- randompass = _passwdqc_random(¶ms.qc);
- if (randompass) {
+ randompass.text = _passwdqc_random(¶ms.qc);
+ if (randompass.text) {
status = say(pamh, PAM_TEXT_INFO, randomonly ?
- MESSAGE_RANDOMONLY : MESSAGE_RANDOM, randompass);
+ MESSAGE_RANDOMONLY : MESSAGE_RANDOM, randompass.text);
if (status != PAM_SUCCESS) {
- _pam_overwrite(randompass);
- randompass = NULL;
+ _pam_overwrite(randompass.text);
+ randompass.text = NULL;
}
} else
if (randomonly) {
@@ -477,29 +490,30 @@
status = PAM_AUTHTOK_ERR;
if (status != PAM_SUCCESS) {
- if (randompass) _pam_overwrite(randompass);
+ if (randompass.text) _pam_overwrite(randompass.text);
return status;
}
- newpass = strdup(resp->resp);
+ newpass.text = strdup(resp->resp);
_pam_drop_reply(resp, 1);
- if (!newpass) {
- if (randompass) _pam_overwrite(randompass);
+ if (!newpass.text) {
+ if (randompass.text) _pam_overwrite(randompass.text);
return PAM_AUTHTOK_ERR;
}
- if (check_max(¶ms, pamh, newpass) && enforce) {
+ if (check_max(¶ms, pamh, newpass.text) && enforce) {
status = PAM_AUTHTOK_ERR;
retry_wanted = 1;
}
reason = NULL;
if (status == PAM_SUCCESS &&
- (!randompass || !strstr(newpass, randompass)) &&
+ (!randompass.text || !strstr(newpass.text, randompass.text)) &&
(randomonly ||
- (reason = _passwdqc_check(¶ms.qc, newpass, oldpass, pw)))) {
+ (reason = _passwdqc_check(¶ms.qc, newpass.text,
+ oldpass.text, pw)))) {
if (randomonly)
say(pamh, PAM_ERROR_MSG, MESSAGE_NOTRANDOM);
else
@@ -515,7 +529,7 @@
PROMPT_NEWPASS2, &resp);
if (status == PAM_SUCCESS) {
if (resp && resp->resp) {
- if (strcmp(newpass, resp->resp)) {
+ if (strcmp(newpass.text, resp->resp)) {
status = say(pamh,
PAM_ERROR_MSG, MESSAGE_MISTYPED);
if (status == PAM_SUCCESS) {
@@ -529,11 +543,11 @@
}
if (status == PAM_SUCCESS)
- status = pam_set_item(pamh, PAM_AUTHTOK, newpass);
+ status = pam_set_item(pamh, PAM_AUTHTOK, newpass.text);
- if (randompass) _pam_overwrite(randompass);
- _pam_overwrite(newpass);
- free(newpass);
+ if (randompass.text) _pam_overwrite(randompass.text);
+ _pam_overwrite(newpass.text);
+ free(newpass.text);
if (retry_wanted && --retries_left > 0) {
status = say(pamh, PAM_TEXT_INFO, MESSAGE_RETRY);
==== //SpectraBSD/stable/lib/libpam/modules/pam_passwdqc/Makefile#2 (text) ====
@@ -7,7 +7,7 @@
SRCS= pam_passwdqc.c passwdqc_check.c passwdqc_random.c wordset_4k.c
MAN= pam_passwdqc.8
-WARNS?= 0
+WARNS?= 2
CFLAGS+= -I${SRCDIR}
DPADD= ${LIBCRYPT}
--Apple-Mail=_410A4854-04E8-4B9A-B54B-38F6603F628C--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?F7FA2B4D-BD56-4EC4-9D93-B5AA847063D7>