Lucas <lu...@sexy.is> wrote: > Hi tech@, > > The following patch expands acme-client config file `domain` blocks to > allow for a `owner user:group` directive, which allows to get rid of > customs scripts that "fix" permissions for issued certs, mostly needed > in ports land. I don't find it too invasive, so I thought it could be > merged. Most of the code and manpage bits were taken from vmd. > > Noteworthy details: > > 1. fileproc.c pledge is expanded with chown. In particular, I don't > understand pledge manpage: I was under the impression that wpath and > fattr would allow for fchown, but I got an "Operation not supported" > while testing. I guess this is related to the paragraph stating that > some syscalls be allowed with limitations under some categories. > 2. if the private key already exists, keyproc.c won't interfere with its > ownership, the same way it does now. The fchown call can be moved if > "fixing" the ownership is desirable.
After checking chown(2), -1 seems more sensible as the default values for [gu]id, instead of 0. -Lucas diff /usr/src commit - 93aad84f8cf14cfaff5b9cdb67494e561810ddc4 path + /usr/src blob - eb5f19eb298c117c3957faa0ed6ced14972ffaca file + usr.sbin/acme-client/acme-client.conf.5 --- usr.sbin/acme-client/acme-client.conf.5 +++ usr.sbin/acme-client/acme-client.conf.5 @@ -197,6 +197,17 @@ will be used. If it is not specified, a default of .Pa /var/www/acme will be used. +.It Ic owner Ar user : Ns Ar group +Set the owner of the key and certificate files create to +.Ar user +and +.Ar group . +If only +.Ar user +is given, only the user is set. +If only +.Pf : Ar group +is given, only the group is set. .El .Sh FILES .Bl -tag -width /etc/examples/acme-client.conf -compact blob - 4b43b6ef4ac302706859bf14b681cf8052aa55c8 file + usr.sbin/acme-client/extern.h --- usr.sbin/acme-client/extern.h +++ usr.sbin/acme-client/extern.h @@ -207,9 +207,9 @@ int fileproc(int, const char *, const char *, const int revokeproc(int, const char *, int, int, const char *const *, size_t); int fileproc(int, const char *, const char *, const char *, - const char *); + const char *, uid_t, gid_t); int keyproc(int, const char *, const char **, size_t, - enum keytype); + enum keytype, uid_t, gid_t); int netproc(int, int, int, int, int, int, int, struct authority_c *, const char *const *, size_t); blob - b8b8651c31907c6d37147c779f302481a1cb3c86 file + usr.sbin/acme-client/fileproc.c --- usr.sbin/acme-client/fileproc.c +++ usr.sbin/acme-client/fileproc.c @@ -29,7 +29,8 @@ serialise(const char *real, const char *v, size_t vsz, #include "extern.h" static int -serialise(const char *real, const char *v, size_t vsz, const char *v2, size_t v2sz) +serialise(const char *real, const char *v, size_t vsz, const char *v2, + size_t v2sz, uid_t uid, gid_t gid) { int fd; char *tmp; @@ -64,6 +65,10 @@ serialise(const char *real, const char *v, size_t vsz, warn("fchmod"); goto out; } + if (fchown(fd, uid, gid) == -1) { + warn("fchown"); + goto out; + } if ((ssize_t)vsz != write(fd, v, vsz)) { warnx("write"); goto out; @@ -91,7 +96,7 @@ fileproc(int certsock, const char *certdir, const char int fileproc(int certsock, const char *certdir, const char *certfile, const char - *chainfile, const char *fullchainfile) + *chainfile, const char *fullchainfile, uid_t uid, gid_t gid) { char *csr = NULL, *ch = NULL; size_t chsz, csz; @@ -108,7 +113,7 @@ fileproc(int certsock, const char *certdir, const char * rpath and cpath for rename, wpath and cpath for * writing to the temporary. fattr for fchmod. */ - if (pledge("stdio cpath wpath rpath fattr", NULL) == -1) { + if (pledge("stdio cpath wpath rpath fattr chown", NULL) == -1) { warn("pledge"); goto out; } @@ -173,7 +178,7 @@ fileproc(int certsock, const char *certdir, const char goto out; if (chainfile) { - if (!serialise(chainfile, ch, chsz, NULL, 0)) + if (!serialise(chainfile, ch, chsz, NULL, 0, uid, gid)) goto out; dodbg("%s: created", chainfile); @@ -190,7 +195,7 @@ fileproc(int certsock, const char *certdir, const char goto out; if (certfile) { - if (!serialise(certfile, csr, csz, NULL, 0)) + if (!serialise(certfile, csr, csz, NULL, 0, uid, gid)) goto out; dodbg("%s: created", certfile); @@ -204,7 +209,7 @@ fileproc(int certsock, const char *certdir, const char */ if (fullchainfile) { if (!serialise(fullchainfile, csr, csz, ch, - chsz)) + chsz, uid, gid)) goto out; dodbg("%s: created", fullchainfile); blob - a3b6666c2796c30b97c5628e5f3d350a765c87cb file + usr.sbin/acme-client/keyproc.c --- usr.sbin/acme-client/keyproc.c +++ usr.sbin/acme-client/keyproc.c @@ -75,7 +75,7 @@ keyproc(int netsock, const char *keyfile, const char * */ int keyproc(int netsock, const char *keyfile, const char **alts, size_t altsz, - enum keytype keytype) + enum keytype keytype, uid_t uid, gid_t gid) { char *der64 = NULL, *der = NULL, *dercp; char *sans = NULL, *san = NULL; @@ -97,7 +97,18 @@ keyproc(int netsock, const char *keyfile, const char * prev = umask((S_IWUSR | S_IXUSR) | S_IRWXG | S_IRWXO); if ((f = fopen(keyfile, "r")) == NULL && errno == ENOENT) { + int fd; + f = fopen(keyfile, "wx"); + if (f != NULL) { + fd = fileno(f); + if (fd >= 0) { + if (fchown(fd, uid, gid) == -1) { + warn("chown %s", keyfile); + goto out; + } + } + } newkey = 1; } umask(prev); blob - bec17254297fb1e770411c1cb8ddb718e150ee0f file + usr.sbin/acme-client/main.c --- usr.sbin/acme-client/main.c +++ usr.sbin/acme-client/main.c @@ -248,7 +248,7 @@ main(int argc, char *argv[]) close(file_fds[1]); c = keyproc(key_fds[0], domain->key, (const char **)alts, altsz, - domain->keytype); + domain->keytype, domain->owner.uid, domain->owner.gid); exit(c ? EXIT_SUCCESS : EXIT_FAILURE); } @@ -319,7 +319,7 @@ main(int argc, char *argv[]) close(dns_fds[0]); close(rvk_fds[0]); c = fileproc(file_fds[1], certdir, domain->cert, domain->chain, - domain->fullchain); + domain->fullchain, domain->owner.uid, domain->owner.gid); /* * This is different from the other processes in that it * can return 2 if the certificates were updated. blob - 3954f62a0d0894d8fd243944e8393c2e9dc2e70e file + usr.sbin/acme-client/parse.h --- usr.sbin/acme-client/parse.h +++ usr.sbin/acme-client/parse.h @@ -17,6 +17,7 @@ #ifndef PARSE_H #define PARSE_H +#include <sys/types.h> /* XXX */ #include <sys/queue.h> #define AUTH_MAXLEN 120 /* max length of an authority_c name */ @@ -54,6 +55,10 @@ struct domain_c { char *fullchain; char *auth; char *challengedir; + struct { + uid_t uid; + gid_t gid; + } owner; }; struct altname_c { blob - 2b0d55f20b1947c94defc1c3b07d790e8bea56c8 file + usr.sbin/acme-client/parse.y --- usr.sbin/acme-client/parse.y +++ usr.sbin/acme-client/parse.y @@ -30,7 +30,9 @@ #include <ctype.h> #include <err.h> #include <errno.h> +#include <grp.h> #include <limits.h> +#include <pwd.h> #include <stdarg.h> #include <stdio.h> #include <stdlib.h> @@ -94,6 +96,10 @@ typedef struct { union { int64_t number; char *string; + struct { + uid_t uid; + gid_t gid; + } owner; } v; int lineno; } YYSTYPE; @@ -102,6 +108,7 @@ typedef struct { %token AUTHORITY URL API ACCOUNT CONTACT %token DOMAIN ALTERNATIVE NAME NAMES CERT FULL CHAIN KEY SIGN WITH CHALLENGEDIR +%token OWNER %token YES NO %token INCLUDE %token ERROR @@ -110,6 +117,7 @@ typedef struct { %token <v.number> NUMBER %type <v.string> string %type <v.number> keytype +%type <v.owner> owner_id %% @@ -393,6 +401,10 @@ domainoptsl : ALTERNATIVE NAMES '{' optnl altname_l '} err(EXIT_FAILURE, "strdup"); domain->challengedir = s; } + | OWNER owner_id { + domain->owner.uid = $2.uid; + domain->owner.gid = $2.gid; + } ; altname_l : altname optcommanl altname_l @@ -421,6 +433,48 @@ altname : STRING { */ } +owner_id : NUMBER { + $$.uid = $1; + $$.gid = -1; + } + | STRING { + char *user, *group; + struct passwd *pw; + struct group *gr; + + $$.uid = $$.gid = -1; + + user = $1; + if ((group = strchr(user, ':')) != NULL) { + if (group == user) + user = NULL; + *group++ = '\0'; + } + + if (user != NULL && *user) { + if ((pw = getpwnam(user)) == NULL) { + yyerror("failed to get user: %s", + user); + free($1); + YYERROR; + } + $$.uid = pw->pw_uid; + } + + if (group != NULL && *group) { + if ((gr = getgrnam(group)) == NULL) { + yyerror("failed to get group: %s", + group); + free($1); + YYERROR; + } + $$.gid = gr->gr_gid; + } + + free($1); + } + ; + %% struct keywords { @@ -470,6 +524,7 @@ lookup(char *s) {"key", KEY}, {"name", NAME}, {"names", NAMES}, + {"owner", OWNER}, {"rsa", RSA}, {"sign", SIGN}, {"url", URL},