5c5dc75641
On 2/19/23 18:09, David Mudrich wrote:
> I am working on a RAM based Linux OS from source, and try to use
> latest versions of all software. I found shadow needs libbsd's
> readpassphrase(3) as superior alternative to getpass(3). While
> considering if I a) include libbsd, or include libbsd's code of
> readpassphrase(3) into shadow, found, that libbsd's readpassphrase(3)
> never returns \n or \r
> <https://cgit.freedesktop.org/libbsd/tree/src/readpassphrase.c>
> line 122, while agetpass() uses a check for \n in agetpass.c line 108.
> I assume it always fails.
Indeed, it always failed. I made a mistake when writing agetpass(),
assuming that readpassphrase(3) would keep newlines.
>
> I propose a check of len == PASS_MAX - 1, with false positive error for
> exactly PASS_MAX - 1 long passwords.
Instead, I added an extra byte to the allocation to allow a maximum
password length of PASS_MAX (which is the maximum for getpass(3), which
we're replacing.
While doing that, I notice that my previous implementation also had
another bug (minor): The maximum password length was PASS_MAX - 1
instead of PASS_MAX. That's also fixed in this commit.
Reported-by: David Mudrich <dmudrich@gmx.de>
Fixes: 155c9421b9 ("libmisc: agetpass(), erase_pass(): Add functions for getting passwords safely")
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
130 lines
3.1 KiB
C
130 lines
3.1 KiB
C
/*
|
|
* SPDX-FileCopyrightText: 2022, Alejandro Colomar <alx@kernel.org>
|
|
*
|
|
* SPDX-License-Identifier: BSD-3-Clause
|
|
*/
|
|
|
|
|
|
#include <config.h>
|
|
|
|
#include <limits.h>
|
|
#include <readpassphrase.h>
|
|
#include <stdio.h>
|
|
#include <stdlib.h>
|
|
#include <string.h>
|
|
|
|
#ident "$Id$"
|
|
|
|
#include "prototypes.h"
|
|
|
|
|
|
#if !defined(PASS_MAX)
|
|
#define PASS_MAX BUFSIZ - 1
|
|
#endif
|
|
|
|
|
|
/*
|
|
* SYNOPSIS
|
|
* [[gnu::malloc(erase_pass)]]
|
|
* char *agetpass(const char *prompt);
|
|
*
|
|
* void erase_pass(char *pass);
|
|
*
|
|
* ARGUMENTS
|
|
* agetpass()
|
|
* prompt String to be printed before reading a password.
|
|
*
|
|
* erase_pass()
|
|
* pass password previously returned by agetpass().
|
|
*
|
|
* DESCRIPTION
|
|
* agetpass()
|
|
* This function is very similar to getpass(3). It has several
|
|
* advantages compared to getpass(3):
|
|
*
|
|
* - Instead of using a static buffer, agetpass() allocates memory
|
|
* through malloc(3). This makes the function thread-safe, and
|
|
* also reduces the visibility of the buffer.
|
|
*
|
|
* - agetpass() doesn't call realloc(3) internally. Some
|
|
* implementations of getpass(3), such as glibc, do that, as a
|
|
* consequence of calling getline(3). That's a bug in glibc,
|
|
* which allows leaking prefixes of passwords in freed memory.
|
|
*
|
|
* - agetpass() doesn't overrun the output buffer. If the input
|
|
* password is too long, it simply fails. Some implementations
|
|
* of getpass(3), share the same bug that gets(3) has.
|
|
*
|
|
* As soon as possible, the password obtained from agetpass() be
|
|
* erased by calling erase_pass(), to avoid possibly leaking the
|
|
* password.
|
|
*
|
|
* erase_pass()
|
|
* This function first clears the password, by calling
|
|
* explicit_bzero(3) (or an equivalent call), and then frees the
|
|
* allocated memory by calling free(3).
|
|
*
|
|
* NULL is a valid input pointer, and in such a case, this call is
|
|
* a no-op.
|
|
*
|
|
* RETURN VALUE
|
|
* agetpass() returns a newly allocated buffer containing the
|
|
* password on success. On error, errno is set to indicate the
|
|
* error, and NULL is returned.
|
|
*
|
|
* ERRORS
|
|
* agetpass()
|
|
* This function may fail for any errors that malloc(3) or
|
|
* readpassphrase(3) may fail, and in addition it may fail for the
|
|
* following errors:
|
|
*
|
|
* ENOBUFS
|
|
* The input password was longer than PASS_MAX.
|
|
*
|
|
* CAVEATS
|
|
* If a password is passed twice to erase_pass(), the behavior is
|
|
* undefined.
|
|
*/
|
|
|
|
|
|
char *
|
|
agetpass(const char *prompt)
|
|
{
|
|
char *pass;
|
|
size_t len;
|
|
|
|
/*
|
|
* Since we want to support passwords upto PASS_MAX, we need
|
|
* PASS_MAX bytes for the password itself, and one more byte for
|
|
* the terminating '\0'. We also want to detect truncation, and
|
|
* readpassphrase(3) doesn't detect it, so we need some trick.
|
|
* Let's add one more byte, and if the password uses it, it
|
|
* means the introduced password was longer than PASS_MAX.
|
|
*/
|
|
pass = malloc(PASS_MAX + 2);
|
|
if (pass == NULL)
|
|
return NULL;
|
|
|
|
if (readpassphrase(prompt, pass, PASS_MAX + 2, RPP_REQUIRE_TTY) == NULL)
|
|
goto fail;
|
|
|
|
len = strlen(pass);
|
|
if (len == PASS_MAX + 1) {
|
|
errno = ENOBUFS;
|
|
goto fail;
|
|
}
|
|
|
|
return pass;
|
|
|
|
fail:
|
|
freezero(pass, PASS_MAX + 2);
|
|
return NULL;
|
|
}
|
|
|
|
|
|
void
|
|
erase_pass(char *pass)
|
|
{
|
|
freezero(pass, PASS_MAX + 2);
|
|
}
|