mirror of
https://github.com/postgres/postgres.git
synced 2025-05-18 17:41:14 +03:00
Commits a59c79564 et al. tried to sync libpq's SSL key file permissions checks with what we've used for years in the backend. We did not intend to create any new failure cases, but it turns out we did: restricting the key file's ownership breaks cases where the client is allowed to read a key file despite not having the identical UID. In particular a client running as root used to be able to read someone else's key file; and having seen that I suspect that there are other, less-dubious use cases that this restriction breaks on some platforms. We don't really need an ownership check, since if we can read the key file despite its having restricted permissions, it must have the right ownership --- under normal conditions anyway, and the point of this patch is that any additional corner cases where that works should be deemed allowable, as they have been historically. Hence, just drop the ownership check, and rearrange the permissions check to get rid of its faulty assumption that geteuid() can't be zero. (Note that the comparable backend-side code doesn't have to cater for geteuid() == 0, since the server rejects that very early on.) This does have the end result that the permissions safety check used for a root user's private key file is weaker than that used for anyone else's. While odd, root really ought to know what she's doing with file permissions, so I think this is acceptable. Per report from Yogendra Suralkar. Like the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/MW3PR15MB3931DF96896DC36D21AFD47CA3D39@MW3PR15MB3931.namprd15.prod.outlook.com
196 lines
4.9 KiB
C
196 lines
4.9 KiB
C
/*-------------------------------------------------------------------------
|
|
*
|
|
* be-secure-common.c
|
|
*
|
|
* common implementation-independent SSL support code
|
|
*
|
|
* While be-secure.c contains the interfaces that the rest of the
|
|
* communications code calls, this file contains support routines that are
|
|
* used by the library-specific implementations such as be-secure-openssl.c.
|
|
*
|
|
* Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group
|
|
* Portions Copyright (c) 1994, Regents of the University of California
|
|
*
|
|
* IDENTIFICATION
|
|
* src/backend/libpq/be-secure-common.c
|
|
*
|
|
*-------------------------------------------------------------------------
|
|
*/
|
|
|
|
#include "postgres.h"
|
|
|
|
#include <sys/stat.h>
|
|
#include <unistd.h>
|
|
|
|
#include "common/string.h"
|
|
#include "libpq/libpq.h"
|
|
#include "storage/fd.h"
|
|
|
|
/*
|
|
* Run ssl_passphrase_command
|
|
*
|
|
* prompt will be substituted for %p. is_server_start determines the loglevel
|
|
* of error messages.
|
|
*
|
|
* The result will be put in buffer buf, which is of size size. The return
|
|
* value is the length of the actual result.
|
|
*/
|
|
int
|
|
run_ssl_passphrase_command(const char *prompt, bool is_server_start, char *buf, int size)
|
|
{
|
|
int loglevel = is_server_start ? ERROR : LOG;
|
|
StringInfoData command;
|
|
char *p;
|
|
FILE *fh;
|
|
int pclose_rc;
|
|
size_t len = 0;
|
|
|
|
Assert(prompt);
|
|
Assert(size > 0);
|
|
buf[0] = '\0';
|
|
|
|
initStringInfo(&command);
|
|
|
|
for (p = ssl_passphrase_command; *p; p++)
|
|
{
|
|
if (p[0] == '%')
|
|
{
|
|
switch (p[1])
|
|
{
|
|
case 'p':
|
|
appendStringInfoString(&command, prompt);
|
|
p++;
|
|
break;
|
|
case '%':
|
|
appendStringInfoChar(&command, '%');
|
|
p++;
|
|
break;
|
|
default:
|
|
appendStringInfoChar(&command, p[0]);
|
|
}
|
|
}
|
|
else
|
|
appendStringInfoChar(&command, p[0]);
|
|
}
|
|
|
|
fh = OpenPipeStream(command.data, "r");
|
|
if (fh == NULL)
|
|
{
|
|
ereport(loglevel,
|
|
(errcode_for_file_access(),
|
|
errmsg("could not execute command \"%s\": %m",
|
|
command.data)));
|
|
goto error;
|
|
}
|
|
|
|
if (!fgets(buf, size, fh))
|
|
{
|
|
if (ferror(fh))
|
|
{
|
|
explicit_bzero(buf, size);
|
|
ereport(loglevel,
|
|
(errcode_for_file_access(),
|
|
errmsg("could not read from command \"%s\": %m",
|
|
command.data)));
|
|
goto error;
|
|
}
|
|
}
|
|
|
|
pclose_rc = ClosePipeStream(fh);
|
|
if (pclose_rc == -1)
|
|
{
|
|
explicit_bzero(buf, size);
|
|
ereport(loglevel,
|
|
(errcode_for_file_access(),
|
|
errmsg("could not close pipe to external command: %m")));
|
|
goto error;
|
|
}
|
|
else if (pclose_rc != 0)
|
|
{
|
|
explicit_bzero(buf, size);
|
|
ereport(loglevel,
|
|
(errcode_for_file_access(),
|
|
errmsg("command \"%s\" failed",
|
|
command.data),
|
|
errdetail_internal("%s", wait_result_to_str(pclose_rc))));
|
|
goto error;
|
|
}
|
|
|
|
/* strip trailing newline and carriage return */
|
|
len = pg_strip_crlf(buf);
|
|
|
|
error:
|
|
pfree(command.data);
|
|
return len;
|
|
}
|
|
|
|
|
|
/*
|
|
* Check permissions for SSL key files.
|
|
*/
|
|
bool
|
|
check_ssl_key_file_permissions(const char *ssl_key_file, bool isServerStart)
|
|
{
|
|
int loglevel = isServerStart ? FATAL : LOG;
|
|
struct stat buf;
|
|
|
|
if (stat(ssl_key_file, &buf) != 0)
|
|
{
|
|
ereport(loglevel,
|
|
(errcode_for_file_access(),
|
|
errmsg("could not access private key file \"%s\": %m",
|
|
ssl_key_file)));
|
|
return false;
|
|
}
|
|
|
|
/* Key file must be a regular file */
|
|
if (!S_ISREG(buf.st_mode))
|
|
{
|
|
ereport(loglevel,
|
|
(errcode(ERRCODE_CONFIG_FILE_ERROR),
|
|
errmsg("private key file \"%s\" is not a regular file",
|
|
ssl_key_file)));
|
|
return false;
|
|
}
|
|
|
|
/*
|
|
* Refuse to load key files owned by users other than us or root, and
|
|
* require no public access to the key file. If the file is owned by us,
|
|
* require mode 0600 or less. If owned by root, require 0640 or less to
|
|
* allow read access through either our gid or a supplementary gid that
|
|
* allows us to read system-wide certificates.
|
|
*
|
|
* Note that roughly similar checks are performed in
|
|
* src/interfaces/libpq/fe-secure-openssl.c so any changes here may need
|
|
* to be made there as well. The environment is different though; this
|
|
* code can assume that we're not running as root.
|
|
*
|
|
* Ideally we would do similar permissions checks on Windows, but it is
|
|
* not clear how that would work since Unix-style permissions may not be
|
|
* available.
|
|
*/
|
|
#if !defined(WIN32) && !defined(__CYGWIN__)
|
|
if (buf.st_uid != geteuid() && buf.st_uid != 0)
|
|
{
|
|
ereport(loglevel,
|
|
(errcode(ERRCODE_CONFIG_FILE_ERROR),
|
|
errmsg("private key file \"%s\" must be owned by the database user or root",
|
|
ssl_key_file)));
|
|
return false;
|
|
}
|
|
|
|
if ((buf.st_uid == geteuid() && buf.st_mode & (S_IRWXG | S_IRWXO)) ||
|
|
(buf.st_uid == 0 && buf.st_mode & (S_IWGRP | S_IXGRP | S_IRWXO)))
|
|
{
|
|
ereport(loglevel,
|
|
(errcode(ERRCODE_CONFIG_FILE_ERROR),
|
|
errmsg("private key file \"%s\" has group or world access",
|
|
ssl_key_file),
|
|
errdetail("File must have permissions u=rw (0600) or less if owned by the database user, or permissions u=rw,g=r (0640) or less if owned by root.")));
|
|
return false;
|
|
}
|
|
#endif
|
|
|
|
return true;
|
|
}
|