Lucas <[email protected]> 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},
