From 29193e306fa08cdf97489d33d285bc5c0fd9d5b4 Mon Sep 17 00:00:00 2001 From: Reyk Floeter Date: Fri, 30 Jun 2017 21:30:05 +0200 Subject: [PATCH] Sanitize strings that we get from the cloud backends --- agent/azure.c | 40 ++++++++++++++++++++++++++----------- agent/cloudinit.c | 32 +++++++++++++++++++----------- agent/http.c | 3 ++- agent/main.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++- agent/main.h | 12 ++++++++++-- agent/xml.c | 2 +- 6 files changed, 112 insertions(+), 27 deletions(-) diff --git a/agent/azure.c b/agent/azure.c index a43bd79..6d70185 100644 --- a/agent/azure.c +++ b/agent/azure.c @@ -326,7 +326,8 @@ azure_goalstate(struct system_config *sc) if ((xe = xml_findl(&xml.ox_root, "GoalState", "Container", "ContainerId", NULL)) == NULL || - (az->az_container = strdup(xe->xe_data)) == NULL) { + (az->az_container = + get_word(xe->xe_data, xe->xe_datalen)) == NULL) { log_debug("%s: unexpected container id", __func__); goto done; } @@ -334,7 +335,8 @@ azure_goalstate(struct system_config *sc) if ((xe = xml_findl(&xml.ox_root, "GoalState", "Container", "RoleInstanceList", "RoleInstance", "InstanceId", NULL)) == NULL || - (sc->sc_instance = strdup(xe->xe_data)) == NULL) { + (sc->sc_instance = + get_word(xe->xe_data, xe->xe_datalen)) == NULL) { log_debug("%s: unexpected instance id", __func__); goto done; } @@ -649,7 +651,7 @@ azure_getovfenv(struct system_config *sc) { struct xml xml; struct xmlelem *xp, *xe, *xk, *xv; - const char *sshfp, *sshval; + char *sshfp, *sshval, *str; int mount = 0, ret = -1, fd = -1; FILE *fp; @@ -698,18 +700,21 @@ azure_getovfenv(struct system_config *sc) if ((xv = xml_findl(&xk->xe_head, "Fingerprint", NULL)) != NULL) - sshfp = xv->xe_data; + sshfp = get_word(xv->xe_data, xv->xe_datalen); if ((xv = xml_findl(&xk->xe_head, "Value", NULL)) != NULL) - sshval = xv->xe_data; + sshval = get_line(xv->xe_data, xv->xe_datalen); if (agent_addpubkey(sc, sshval, sshfp) != 0) log_warnx("failed to add ssh pubkey"); + free(sshfp); + free(sshval); } } if ((xe = xml_findl(&xp->xe_head, "HostName", NULL)) != NULL) { - if ((sc->sc_hostname = strdup(xe->xe_data)) == NULL) { + if ((sc->sc_hostname = + get_word(xe->xe_data, xe->xe_datalen)) == NULL) { log_debug("%s: hostname failed", __func__); goto done; } @@ -717,31 +722,44 @@ azure_getovfenv(struct system_config *sc) if ((xe = xml_findl(&xp->xe_head, "UserName", NULL)) != NULL) { free(sc->sc_username); - if ((sc->sc_username = strdup(xe->xe_data)) == NULL) { + if ((sc->sc_username = + get_word(xe->xe_data, xe->xe_datalen)) == NULL) { log_debug("%s: username failed", __func__); goto done; } } if ((xe = xml_findl(&xp->xe_head, "UserPassword", NULL)) != NULL) { - if ((sc->sc_password = calloc(1, 128)) == NULL || - crypt_newhash(xe->xe_data, "bcrypt,a", - sc->sc_password, 128) != 0) { + if ((sc->sc_password = calloc(1, 128)) == NULL) { log_debug("%s: password failed", __func__); goto done; } + /* Allow any non-NUL character as input */ + str = strndup(xe->xe_data, xe->xe_datalen); + if (str == NULL || + crypt_newhash(str, "bcrypt,a", + sc->sc_password, 128) != 0) { + log_debug("%s: password hashing failed", __func__); + free(sc->sc_password); + sc->sc_password = NULL; + free(str); + goto done; + } + free(str); /* Replace unencrypted password with hash */ free(xe->xe_tag); xe->xe_tag = strdup("UserPasswordHash"); + /* Update element for xml_print() below */ explicit_bzero(xe->xe_data, xe->xe_datalen); free(xe->xe_data); xe->xe_data = strdup(sc->sc_password); xe->xe_datalen = strlen(sc->sc_password); } else if ((xe = xml_findl(&xp->xe_head, "UserPasswordHash", NULL)) != NULL) { - if ((sc->sc_password = strdup(xe->xe_data)) != NULL) { + if ((sc->sc_password = + get_word(xe->xe_data, xe->xe_datalen)) != NULL) { log_debug("%s: password hash failed", __func__); goto done; } diff --git a/agent/cloudinit.c b/agent/cloudinit.c index a50997d..a46e82a 100644 --- a/agent/cloudinit.c +++ b/agent/cloudinit.c @@ -30,7 +30,8 @@ #include "xml.h" static int cloudinit_fetch(struct system_config *); -static char *cloudinit_get(struct system_config *, const char *, size_t *); +static char *cloudinit_get(struct system_config *, const char *, + enum strtype); int ec2(struct system_config *sc) @@ -58,7 +59,7 @@ cloudinit(struct system_config *sc) } static char * -cloudinit_get(struct system_config *sc, const char *path, size_t *strsz) +cloudinit_get(struct system_config *sc, const char *path, enum strtype type) { struct httpget *g = NULL; char *str = NULL; @@ -68,11 +69,20 @@ cloudinit_get(struct system_config *sc, const char *path, size_t *strsz) g = http_get(&sc->sc_addr, 1, sc->sc_endpoint, 80, path, NULL, 0, NULL); if (g != NULL && g->code == 200 && g->bodypartsz > 0) { - if ((str = calloc(1, g->bodypartsz + 1)) != NULL) { - memcpy(str, g->bodypart, g->bodypartsz); - if (strsz != NULL) - *strsz = g->bodypartsz; + switch (type) { + case TEXT: + /* multi-line string, always printable */ + str = get_string(g->bodypart, g->bodypartsz); + break; + case LINE: + str = get_line(g->bodypart, g->bodypartsz); + break; + case WORD: + str = get_word(g->bodypart, g->bodypartsz); + break; } + + log_debug("%s: '%s'", __func__, str); } http_get_free(g); @@ -90,24 +100,24 @@ cloudinit_fetch(struct system_config *sc) /* instance-id */ if ((sc->sc_instance = cloudinit_get(sc, - "/latest/meta-data/instance-id", NULL)) == NULL) + "/latest/meta-data/instance-id", WORD)) == NULL) goto fail; /* hostname */ if ((sc->sc_hostname = cloudinit_get(sc, - "/latest/meta-data/local-hostname", NULL)) == NULL) + "/latest/meta-data/local-hostname", WORD)) == NULL) goto fail; /* pubkey */ if ((str = cloudinit_get(sc, - "/latest/meta-data/public-keys/0/openssh-key", NULL)) == NULL) + "/latest/meta-data/public-keys/0/openssh-key", LINE)) == NULL) goto fail; if (agent_addpubkey(sc, str, NULL) != 0) goto fail; /* optional username - this is an extension by meta-data(8) */ if ((str = cloudinit_get(sc, - "/latest/meta-data/username", NULL)) != NULL) { + "/latest/meta-data/username", WORD)) != NULL) { free(sc->sc_username); sc->sc_username = str; str = NULL; @@ -115,7 +125,7 @@ cloudinit_fetch(struct system_config *sc) /* userdata */ if ((sc->sc_userdata = cloudinit_get(sc, - "/latest/user-data", &sc->sc_userdatalen)) == NULL) + "/latest/user-data", TEXT)) == NULL) goto fail; ret = 0; diff --git a/agent/http.c b/agent/http.c index 59210bd..f871745 100644 --- a/agent/http.c +++ b/agent/http.c @@ -33,6 +33,7 @@ #include #include "http.h" +#include "main.h" #define DEFAULT_CA_FILE "/etc/ssl/cert.pem" @@ -587,7 +588,7 @@ http_head_parse(const struct http *http, struct httpxfer *trans, size_t *sz) } *ccp++ = '\0'; - while (isspace((int)*ccp)) + while (isspace((unsigned char)*ccp)) ccp++; h[hsz].key = cp; h[hsz++].val = ccp; diff --git a/agent/main.c b/agent/main.c index 7a6f2e9..68df50d 100644 --- a/agent/main.c +++ b/agent/main.c @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -239,6 +240,53 @@ enable_output(struct system_config *sc, int fd, int oldfd) return (0); } +char * +get_string(u_int8_t *ptr, size_t len) +{ + size_t i; + + /* + * We don't use vis(3) here because the string should not be + * modified and only validated for printable characters and proper + * NUL-termination. From relayd. + */ + for (i = 0; i < len; i++) + if (!(isprint((unsigned char)ptr[i]) || + isspace((unsigned char)ptr[i]))) + break; + + return strndup(ptr, i); +} + +char * +get_line(u_int8_t *ptr, size_t len) +{ + size_t i; + + /* Like the previous, but without newlines */ + for (i = 0; i < len; i++) + if (!isprint((unsigned char)ptr[i]) || + (isspace((unsigned char)ptr[i]) && + !isblank((unsigned char)ptr[i]))) + break; + + return strndup(ptr, i); +} + +char * +get_word(u_int8_t *ptr, size_t len) +{ + size_t i; + + /* Like the previous, but without spaces and newlines */ + for (i = 0; i < len; i++) + if (!isprint((unsigned char)ptr[i]) || + isspace((unsigned char)ptr[i])) + break; + + return strndup(ptr, i); +} + static struct system_config * agent_init(void) { @@ -507,7 +555,7 @@ agent_configure(struct system_config *sc, int noaction) "#############################################################\n" "EOF\n", "a", "/etc/rc.firsttime") != 0) - log_warnx("userdata failed"); + log_warnx("ssh fingerprints failed"); return (0); } diff --git a/agent/main.h b/agent/main.h index 29a0728..cd4617e 100644 --- a/agent/main.h +++ b/agent/main.h @@ -24,6 +24,12 @@ #include "http.h" +enum strtype { + WORD, + LINE, + TEXT +}; + struct ssh_pubkey { char *ssh_keyval; char *ssh_keyfp; @@ -37,8 +43,7 @@ struct system_config { char *sc_username; char *sc_password; char *sc_pubkey; - unsigned char *sc_userdata; - size_t sc_userdatalen; + char *sc_userdata; char *sc_endpoint; char *sc_instance; @@ -65,6 +70,9 @@ int shell(const char *, ...); int shellout(const char *, char **, const char *, ...); int disable_output(struct system_config *, int); int enable_output(struct system_config *, int, int); +char *get_string(u_int8_t *, size_t); +char *get_line(u_int8_t *, size_t); +char *get_word(u_int8_t *, size_t); int agent_addpubkey(struct system_config *, const char *, const char *); int agent_setpubkey(struct system_config *, const char *, const char *); int agent_configure(struct system_config *, int); diff --git a/agent/xml.c b/agent/xml.c index c971562..049cbae 100644 --- a/agent/xml.c +++ b/agent/xml.c @@ -259,7 +259,7 @@ xml_char_data(void *data, const char *s, int len) off_t off = 0; for (i = 0; i < len && s[i] != '\0'; i++) { - if (!isspace(s[i])) { + if (!isspace((unsigned char)s[i])) { ok = 1; break; }