Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit9e3dbc6

Browse files
committed
Remove misguided SSL key file ownership check in libpq.
Commitsa59c795 et al. tried to sync libpq's SSL key filepermissions checks with what we've used for years in the backend.We did not intend to create any new failure cases, but it turns outwe did: restricting the key file's ownership breaks cases where theclient is allowed to read a key file despite not having the identicalUID. In particular a client running as root used to be able to readsomeone else's key file; and having seen that I suspect that there areother, less-dubious use cases that this restriction breaks on someplatforms.We don't really need an ownership check, since if we can read the keyfile despite its having restricted permissions, it must have the rightownership --- under normal conditions anyway, and the point of thispatch is that any additional corner cases where that works should bedeemed allowable, as they have been historically. Hence, just dropthe ownership check, and rearrange the permissions check to get ridof its faulty assumption that geteuid() can't be zero. (Note that thecomparable 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 usedfor a root user's private key file is weaker than that used foranyone else's. While odd, root really ought to know what she's doingwith 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
1 parent036cffb commit9e3dbc6

File tree

2 files changed

+20
-19
lines changed

2 files changed

+20
-19
lines changed

‎src/backend/libpq/be-secure-common.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,9 +160,10 @@ check_ssl_key_file_permissions(const char *ssl_key_file, bool isServerStart)
160160
* allow read access through either our gid or a supplementary gid that
161161
* allows us to read system-wide certificates.
162162
*
163-
* Note that similar checks are performed in
163+
* Note thatroughlysimilar checks are performed in
164164
* src/interfaces/libpq/fe-secure-openssl.c so any changes here may need
165-
* to be made there as well.
165+
* to be made there as well. The environment is different though; this
166+
* code can assume that we're not running as root.
166167
*
167168
* Ideally we would do similar permissions checks on Windows, but it is
168169
* not clear how that would work since Unix-style permissions may not be

‎src/interfaces/libpq/fe-secure-openssl.c

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1198,31 +1198,31 @@ initialize_SSL(PGconn *conn)
11981198
}
11991199

12001200
/*
1201-
* Refuse to load key files owned by users other than us or root, and
1202-
* require no public access to the key file. If the file is owned by
1203-
* us, require mode 0600 or less. If owned by root, require 0640 or
1204-
* less to allow read access through either our gid or a supplementary
1205-
* gid that allows us to read system-wide certificates.
1201+
* Refuse to load world-readable key files. We accept root-owned
1202+
* files with mode 0640 or less, so that we can access system-wide
1203+
* certificates if we have a supplementary group membership that
1204+
* allows us to read 'em. For files with non-root ownership, require
1205+
* mode 0600 or less. We need not check the file's ownership exactly;
1206+
* if we're able to read it despite it having such restrictive
1207+
* permissions, it must have the right ownership.
12061208
*
1207-
* Note that similar checks are performed in
1209+
* Note: be very careful about tightening these rules. Some people
1210+
* expect, for example, that a client process running as root should
1211+
* be able to use a non-root-owned key file.
1212+
*
1213+
* Note that roughly similar checks are performed in
12081214
* src/backend/libpq/be-secure-common.c so any changes here may need
1209-
* to be made there as well.
1215+
* to be made there as well. However, this code caters for the case
1216+
* of current user == root, while that code does not.
12101217
*
12111218
* Ideally we would do similar permissions checks on Windows, but it
12121219
* is not clear how that would work since Unix-style permissions may
12131220
* not be available.
12141221
*/
12151222
#if !defined(WIN32)&& !defined(__CYGWIN__)
1216-
if (buf.st_uid!=geteuid()&&buf.st_uid!=0)
1217-
{
1218-
printfPQExpBuffer(&conn->errorMessage,
1219-
libpq_gettext("private key file \"%s\" must be owned by the current user or root\n"),
1220-
fnbuf);
1221-
return-1;
1222-
}
1223-
1224-
if ((buf.st_uid==geteuid()&&buf.st_mode& (S_IRWXG |S_IRWXO))||
1225-
(buf.st_uid==0&&buf.st_mode& (S_IWGRP |S_IXGRP |S_IRWXO)))
1223+
if (buf.st_uid==0 ?
1224+
buf.st_mode& (S_IWGRP |S_IXGRP |S_IRWXO) :
1225+
buf.st_mode& (S_IRWXG |S_IRWXO))
12261226
{
12271227
printfPQExpBuffer(&conn->errorMessage,
12281228
libpq_gettext("private key file \"%s\" has group or world access; file must have permissions u=rw (0600) or less if owned by the current user, or permissions u=rw,g=r (0640) or less if owned by root\n"),

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp