8000 Allow root-owned SSL private keys in libpq, not only the backend. · postgres/postgres@a59c795 · GitHub
[go: up one dir, main page]
Skip to content

Commit a59c795

Browse files
tglsfdctglsfdc
committed
Allow root-owned SSL private keys in libpq, not only the backend.
This change makes libpq apply the same private-key-file ownership and permissions checks that we have used in the backend since commit 9a83564. Namely, that the private key can be owned by either the current user or root (with different file permissions allowed in the two cases). This allows system-wide management of key files, which is just as sensible on the client side as the server, particularly when the client is itself some application daemon. Sync the comments about this between libpq and the backend, too. David Steele Discussion: https://postgr.es/m/f4b7bc55-97ac-9e69-7398-335e212f7743@pgmasters.net
1 parent 12d768e commit a59c795

File tree

2 files changed

+50
-18
lines changed

2 files changed

+50
-18
lines changed

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

Lines changed: 13 additions & 15 deletions
< 8000 td data-grid-cell-id="diff-5c7a4af26b3335601ef304bbd3d21ebe20c888250a85d5e971cdba1358ac77bd-148-149-2" data-line-anchor="diff-5c7a4af26b3335601ef304bbd3d21ebe20c888250a85d5e971cdba1358ac77bdR149" data-selected="false" role="gridcell" style="background-color:var(--bgColor-default);padding-right:24px" tabindex="-1" valign="top" class="focusable-grid-cell diff-text-cell right-side-diff-cell left-side">
ereport(loglevel,
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ check_ssl_key_file_permissions(const char *ssl_key_file, bool isServerStart)
143143
return false;
144144
}
145145

146+
/* Key file must be a regular file */
146147
if (!S_ISREG(buf.st_mode))
147148
{
148149
@@ -153,9 +154,19 @@ check_ssl_key_file_permissions(const char *ssl_key_file, bool isServerStart)
153154
}
154155

155156
/*
156-
* Refuse to load key files owned by users other than us or root.
157+
* Refuse to load key files owned by users other than us or root, and
158+
* require no public access to the key file. If the file is owned by us,
159+
* require mode 0600 or less. If owned by root, require 0640 or less to
160+
* allow read access through either our gid or a supplementary gid that
161+
* allows us to read system-wide certificates.
157162
*
158-
* XXX surely we can check this on Windows somehow, too.
163+
* Note that similar checks are performed in
164+
* src/interfaces/libpq/fe-secure-openssl.c so any changes here may need
165+
* to be made there as well.
166+
*
167+
* Ideally we would do similar permissions checks on Windows, but it is
168+
* not clear how that would work since Unix-style permissions may not be
169+
* available.
159170
*/
160171
#if !defined(WIN32) && !defined(__CYGWIN__)
161172
if (buf.st_uid != geteuid() && buf.st_uid != 0)
@@ -166,20 +177,7 @@ check_ssl_key_file_permissions(const char *ssl_key_file, bool isServerStart)
166177
ssl_key_file)));
167178
return false;
168179
}
169-
#endif
170180

171-
/*
172-
* Require no public access to key file. If the file is owned by us,
173-
* require mode 0600 or less. If owned by root, require 0640 or less to
174-
* allow read access through our gid, or a supplementary gid that allows
175-
* to read system-wide certificates.
176-
*
177-
* XXX temporarily suppress check when on Windows, because there may not
178-
* be proper support for Unix-y file permissions. Need to think of a
179-
* reasonable check to apply on Windows. (See also the data directory
180-
* permission check in postmaster.c)
181-
*/
182-
#if !defined(WIN32) && !defined(__CYGWIN__)
183181
if ((buf.st_uid == geteuid() && buf.st_mode & (S_IRWXG | S_IRWXO)) ||
184182
(buf.st_uid == 0 && buf.st_mode & (S_IWGRP | S_IXGRP | S_IRWXO)))
185183
{

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

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,11 +1245,45 @@ initialize_SSL(PGconn *conn)
12451245
fnbuf);
12461246
return -1;
12471247
}
1248-
#ifndef WIN32
1249-
if (!S_ISREG(buf.st_mode) || buf.st_mode & (S_IRWXG | S_IRWXO))
1248+
1249+
/* Key file must be a regular file */
1250+
if (!S_ISREG(buf.st_mode))
1251+
{
1252+
appendPQExpBuffer(&conn->errorMessage,
1253+
libpq_gettext("private key file \"%s\" is not a regular file"),
1254+
fnbuf);
1255+
return -1;
1256+
}
1257+
1258+
/*
1259+
* Refuse to load key files owned by users other than us or root, and
1260+
* require no public access to the key file. If the file is owned by
1261+
* us, require mode 0600 or less. If owned by root, require 0640 or
1262+
* less to allow read access through either our gid or a supplementary
1263+
* gid that allows us to read system-wide certificates.
1264+
*
1265+
* Note that similar checks are performed in
1266+
* src/backend/libpq/be-secure-common.c so any changes here may need
1267+
* to be made there as well.
1268+
*
1269+
* Ideally we would do similar permissions checks on Windows, but it
1270+
* is not clear how that would work since Unix-style permissions may
1271+
* not be available.
1272+
*/
1273+
#if !defined(WIN32) && !defined(__CYGWIN__)
1274+
if (buf.st_uid != geteuid() && buf.st_uid != 0)
1275+
{
1276+
appendPQExpBuffer(&conn->errorMessage,
1277+
libpq_gettext("private key file \"%s\" must be owned by the current user or root\n"),
1278+
fnbuf);
1279+
return -1;
1280+
}
1281+
1282+
if ((buf.st_uid == geteuid() && buf.st_mode & (S_IRWXG | S_IRWXO)) ||
1283+
(buf.st_uid == 0 && buf.st_mode & (S_IWGRP | S_IXGRP | S_IRWXO)))
12501284
{
12511285
appendPQExpBuffer(&conn->errorMessage,
1252-
libpq_gettext("private key file \"%s\" has group or world access; permissions should be u=rw (0600) or less\n"),
1286+
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"),
12531287
fnbuf);
12541288
return -1;
12551289
}

0 commit comments

Comments
 (0)
0