On Sun, Feb 07, 2021 at 06:48:36PM +0100, Patrick Steinhardt wrote:
> It's currently not easily possible to separate configuration of an
> interface and credentials. This makes it hard to distribute
> configuration across a set of nodes which use wpa_supplicant without
> also having to store credentials in the same file. While this can be
> solved via scripting, having a native way to achieve this would be
> preferable.
>
> Turns out there already is a framework to have external password
> storages. It only has a single "test" backend though, which is kind of
> an in-memory store which gets initialized with all passwords up front.
> This isn't really suitable for above usecase: the backend cannot be
> initialized as part of the central configuration given that it needs the
> credentials, and we want to avoid scripting.
>
> This commit thus extends the infrastructure to implement a new backend,
> which instead uses a simple configuration file containing key-value
> pairs. The file follows the format which wpa_supplicant.conf(5) uses:
> empty lines and comments are ignored, while passwords can be specified
> with simple `password-name=password-value` assignments.
This sounds like a reasonable addition.
> diff --git a/src/utils/ext_password_file.c b/src/utils/ext_password_file.c
> +#include "common.h"
> +#include "ext_password_i.h"
> +#include "utils/config.h"
I'd prefer to list the src/*/*.h before wpa_supplicant/*.h in include
statements.
> +static void * ext_password_file_init(const char *params)
> +{
> + struct ext_password_file_data *data;
> +
> + if (!params) {
> + wpa_printf(MSG_ERROR, "EXT PW FILE: no path given");
> + return NULL;
> + }
> +
> + data = os_zalloc(sizeof(*data));
> + if (data == NULL)
> + return NULL;
> + data->path = os_strdup(params);
> +
> + return data;
That os_strdup() might fail. It would be better check for that here and
return NULL instead of assuming everything is fine and later crashing
due to NULL pointer dereferencing..
> +static void ext_password_file_deinit(void *ctx)
> +{
> + struct ext_password_file_data *data = ctx;
> +
> + str_clear_free(data->path);
str_clear_free() sounds a bit heavy for a path name, but well, if that
contains some secure information.. However:
> +static struct wpabuf * ext_password_file_get(void *ctx, const char *name)
> +{
> + struct ext_password_file_data *data = ctx;
> + struct wpabuf *password = NULL;
> + char buf[512], *pos;
This buf[] is used to read the actual passwords, so it would be more
useful to explicitly clear that memory after use here.. And probably not
the best design to use wpa_printf(MSG_ERROR, "stuff with the raw line
from the password file") to get passwords exposed in debug logs and/or
stdout. Maybe just print the line number without any of the payload.
> + while (wpa_config_get_line(buf, sizeof(buf), f, &line, &pos)) {
> +done:
> + fclose(f);
> + return password;
In other words, forced_memzero(buf, sizeof(buf)) before returning from
the function.
> diff --git a/wpa_supplicant/Makefile b/wpa_supplicant/Makefile
And also similar changes for wpa_supplicant/Android.mk.
--
Jouni Malinen PGP id EFC895FA
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
