Stuart Henderson <s...@spacehopper.org> wrote:
> On 2022/12/18 03:06, Lucas wrote:
> > 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.
>
> Why do you need to chown a certificate? It is published to the world
> anyway in Certificate Transparency logs, what's the problem with
> root-owned and world-readable?
You're absolutely right. And even without CT, the certificate is public,
otherwise you won't be able to establish a TLS session. I guess the
obsessive in me wanted matching ownership for key and cert.
> (There would be more reason to do this for a key, but the existing
> handling seems good enough for that).
Yes, and that partly makes it even less of a case for this patch. There
is still the need to chown to the right user, and currently the handling
of that is manually by the operator. Arguably, currently that only
happens when issuing a certificate for the very first time, or on manual
secret key rotation.
Nevertheless, dropping the cert part is easy, with the upside of not
needing chown pledge anymore in the patch. It was a fun learning
experience. I'll stop pursuing the patch, but leave the final version
anyways.
-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
@@ -131,7 +131,7 @@ plain domain name forms.
The common name is included automatically if this option is present,
but there is no automatic conversion/inclusion between "www." and
plain domain name forms.
-.It Ic domain key Ar file Op Ar keytype
+.It Ic domain key Ar file Oo Ar keytype Oc Oo Ic owner Ar user : Ns Ar group Oc
The private key file for which the certificate will be obtained.
.Ar keytype
can be
@@ -146,6 +146,18 @@ or secp384r1 for
.Cm rsa
or secp384r1 for
.Cm ecdsa ) .
+If
+.Cm owner
+is given, set the key owner 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.
.It Ic domain certificate Ar file
The filename of the certificate that will be issued.
This is optional if
blob - 4b43b6ef4ac302706859bf14b681cf8052aa55c8
file + usr.sbin/acme-client/extern.h
--- usr.sbin/acme-client/extern.h
+++ usr.sbin/acme-client/extern.h
@@ -209,7 +209,7 @@ int keyproc(int, const char *, const char **,
size_t
int fileproc(int, const char *, const char *, const char *,
const char *);
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 - 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);
}
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>
#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>
@@ -71,6 +73,7 @@ void print_config(struct acme_conf *);
struct keyfile *conf_new_keyfile(struct acme_conf *, char *);
void clear_config(struct acme_conf *);
const char* kt2txt(enum keytype);
+const char* owner2txt(uid_t, gid_t);
void print_config(struct acme_conf *);
int conf_check_file(char *);
@@ -94,6 +97,10 @@ typedef struct {
union {
int64_t number;
char *string;
+ struct {
+ uid_t uid;
+ gid_t gid;
+ } owner;
} v;
int lineno;
} YYSTYPE;
@@ -102,6 +109,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 +118,7 @@ typedef struct {
%token <v.number> NUMBER
%type <v.string> string
%type <v.number> keytype
+%type <v.owner> owner_id
%%
@@ -219,7 +228,7 @@ authorityoptsl : API URL STRING {
err(EXIT_FAILURE, "strdup");
auth->api = s;
}
- | ACCOUNT KEY STRING keytype{
+ | ACCOUNT KEY STRING keytype {
char *s;
if (auth->account != NULL) {
yyerror("duplicate account");
@@ -298,7 +307,7 @@ domainoptsl : ALTERNATIVE NAMES '{' optnl altname_l '}
err(EXIT_FAILURE, "strdup");
domain->domain = s;
}
- | DOMAIN KEY STRING keytype {
+ | DOMAIN KEY STRING keytype owner_id {
char *s;
if (domain->key != NULL) {
yyerror("duplicate key");
@@ -317,6 +326,8 @@ domainoptsl : ALTERNATIVE NAMES '{' optnl altname_l '}
}
domain->key = s;
domain->keytype = $4;
+ domain->owner.uid = $5.uid;
+ domain->owner.gid = $5.gid;
}
| DOMAIN CERT STRING {
char *s;
@@ -418,9 +429,52 @@ altname : STRING {
/*
* XXX we could check if altname is duplicate
* or identical to domain->domain
- */
+ */
}
+owner_id : OWNER NUMBER {
+ $$.uid = $2;
+ $$.gid = -1;
+ }
+ | OWNER STRING {
+ char *user, *group;
+ struct passwd *pw;
+ struct group *gr;
+
+ $$.uid = $$.gid = -1;
+
+ user = $2;
+ 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($2);
+ YYERROR;
+ }
+ $$.uid = pw->pw_uid;
+ }
+
+ if (group != NULL && *group) {
+ if ((gr = getgrnam(group)) == NULL) {
+ yyerror("failed to get group: %s",
+ group);
+ free($2);
+ YYERROR;
+ }
+ $$.gid = gr->gr_gid;
+ }
+
+ free($2);
+ }
+ | { $$.uid = $$.gid = -1; }
+ ;
+
%%
struct keywords {
@@ -470,6 +524,7 @@ lookup(char *s)
{"key", KEY},
{"name", NAME},
{"names", NAMES},
+ {"owner", OWNER},
{"rsa", RSA},
{"sign", SIGN},
{"url", URL},
@@ -1039,6 +1094,33 @@ void
}
}
+const char *
+owner2txt(uid_t uid, gid_t gid)
+{
+ static char buf[128];
+ struct passwd *pw;
+ struct group *gr;
+
+ buf[0] = '\0';
+ if (uid == (uid_t)-1 && gid == (gid_t)-1)
+ return buf;
+
+ strlcat(buf, "owner ", sizeof(buf));
+
+ if (uid != (uid_t)-1) {
+ pw = getpwuid(uid);
+ strlcat(buf, pw->pw_name, sizeof(buf));
+ }
+
+ if (gid != (gid_t)-1) {
+ gr = getgrgid(gid);
+ strlcat(buf, ":", sizeof(buf));
+ strlcat(buf, gr->gr_name, sizeof(buf));
+ }
+
+ return buf;
+}
+
void
print_config(struct acme_conf *xconf)
{
@@ -1072,8 +1154,8 @@ print_config(struct acme_conf *xconf)
if (f)
printf(" }\n");
if (d->key != NULL)
- printf("\tdomain key \"%s\" %s\n", d->key, kt2txt(
- d->keytype));
+ printf("\tdomain key \"%s\" %s %s\n", d->key, kt2txt(
+ d->keytype), owner2txt(d->owner.uid, d->owner.gid));
if (d->cert != NULL)
printf("\tdomain certificate \"%s\"\n", d->cert);
if (d->chain != NULL)
