E auth: improve clearing out passwords from memory.

Optimising compilers (like gcc/clang with -O1 or above) were optimising
out the memset(). Until link time optimisations are good enough, this
will prevent them from doing so. The best solution would be to use
memset_s() (c11), though it's not readily available yet. This is the
first step towards using memset_s() with a fallback for systems who
don't have it. A better solution, is to put it in Eina, to prevent LTO
completely. This will have to be done after the EFL release.
Even this is not entirely safe though, but at least it protects us from
some memory disclosure issues.

This doesn't solve the fact that we may store a copy of the password in
other places, like the input system. We need to address that too.

Thanks to Matthew Garrett for pointing this out or Twitter.
This commit is contained in:
Tom Hacohen 2015-04-21 10:07:42 +01:00
parent 9eaac75ae2
commit acfdda6c7f
4 changed files with 25 additions and 11 deletions

View File

@ -138,7 +138,7 @@ e_auth_begin(char *passwd)
/* child */
int pamerr;
E_Auth da;
char *current_user, *p;
char *current_user;
struct sigaction action;
_e_auth_child_pid = fork();
@ -158,8 +158,8 @@ e_auth_begin(char *passwd)
eina_strlcpy(da.user, current_user, sizeof(da.user));
eina_strlcpy(da.passwd, passwd, sizeof(da.passwd));
/* security - null out passwd string once we are done with it */
for (p = passwd; *p; p++)
*p = 0;
e_util_memclear(passwd, strlen(passwd));
da.pam.handle = NULL;
da.pam.conv.conv = NULL;
da.pam.conv.appdata_ptr = NULL;
@ -173,10 +173,8 @@ e_auth_begin(char *passwd)
pamerr = pam_authenticate(da.pam.handle, 0);
pam_end(da.pam.handle, pamerr);
/* security - null out passwd string once we are done with it */
memset(da.passwd, 0, sizeof(da.passwd));
/* break compiler optimization */
if (da.passwd[0] || da.passwd[3])
fprintf(stderr, "ACK!\n");
e_util_memclear(da.passwd, sizeof(da.passwd));
if (pamerr == PAM_SUCCESS)
{
free(current_user);

View File

@ -1423,3 +1423,19 @@ e_util_evas_objects_above_print_smart(Evas_Object *o)
fprintf(stderr, "[%p] - %s(%s) %s\n", a, evas_object_type_get(a), evas_object_name_get(a), evas_object_visible_get(a) ? "VISIBLE" : "HIDDEN");
}
}
/*
* NOTICE: This function should not be used by external modules!!!
*
* This function is just a hack to allow us to "securely" clear sensitive
* info until memset_s() is readily available, or at least we move this hack
* to Eina.
*
* This is going to work until link time optimizations are good enough.
* Hopefully by then, we'll be able to properly use memset_s().
*/
EAPI void
e_util_memclear(void *s, size_t n)
{
memset(s, 0, n);
}

View File

@ -63,6 +63,8 @@ EAPI void e_util_evas_objects_above_print_smart(Evas_Object *o);
EAPI void e_util_string_list_free(Eina_List *l);
EAPI void e_util_memclear(void *s, size_t n);
static inline void
e_util_pointer_center(const E_Client *ec)
{

View File

@ -96,10 +96,8 @@ _text_passwd_update(void)
static void
_lokker_null(void)
{
memset(edd->passwd, 0, sizeof(char) * PASSWD_LEN);
/* break compiler optimization */
if (edd->passwd[0] || edd->passwd[3])
fprintf(stderr, "ACK!\n");
e_util_memclear(edd->passwd, PASSWD_LEN);
_text_passwd_update();
}