On Tue, Nov 05, 2019 at 01:57:09PM -0300, Alexandre Hamada wrote:
> Hi Claudio.
>
> FYI, I've added some support for https. Not sure if this might be useful,
> but here is the code for ta_parse_buffer.
>
> I´ve also modified tal_parse to display a warn message instead of exiting on
> invalid basename.
I decided to currently ignore https URI and just print a warn message.
The code needs some major change so that multiple URI are correclty
supported (which is currently not the case) and that https:// would be
preferred. First of all the process responsible for fetching repos needs
to learn about https://. Then I think rsync_uri_parse() needs to be
reworked. At that point it may be possible to add https:// support to that
function and make it a generic URI parser.
Thanks for the diff anyway.
--
:wq Claudio
> Regards,
> Alexandre Hamada
>
> static int
> https_uri_parse(const char **hostp, size_t *hostsz,
> const char **pathp, size_t *pathsz,
> enum rtype *rtypep, const char *uri)
> {
> const char *host, *path;
> size_t sz;
>
> /* Initialise all output values to NULL or 0. */
>
> if (hostsz != NULL)
> *hostsz = 0;
> if (pathsz != NULL)
> *pathsz = 0;
> if (hostp != NULL)
> *hostp = 0;
> if (pathp != NULL)
> *pathp = 0;
> if (rtypep != NULL)
> *rtypep = RTYPE_EOF;
>
> /* Case-insensitive https URI. */
> if (strncasecmp(uri, "https://";, 8)) {
> warnx("%s: not using https schema", uri);
> return 0;
> }
>
> /* Parse the non-zero-length hostname. */
>
> host = uri + 8;
>
> if ((path = strchr(host, '/')) == NULL) {
> warnx("%s: missing https path", uri);
> return 0;
> } else if (path == host) {
> warnx("%s: zero-length https path", uri);
> return 0;
> }
>
> if (hostp != NULL)
> *hostp = host;
> if (hostsz != NULL)
> *hostsz = path - host;
>
> path++;
> sz = strlen(path);
>
> if (pathp != NULL)
> *pathp = path;
> if (pathsz != NULL)
> *pathsz = sz;
>
> if (rtypep != NULL && sz > 4) {
> if (strcasecmp(path + sz - 4, ".roa") == 0)
> *rtypep = RTYPE_ROA;
> else if (strcasecmp(path + sz - 4, ".mft") == 0)
> *rtypep = RTYPE_MFT;
> else if (strcasecmp(path + sz - 4, ".cer") == 0)
> *rtypep = RTYPE_CER;
> else if (strcasecmp(path + sz - 4, ".crl") == 0)
> *rtypep = RTYPE_CRL;
> }
>
> return 1;
> }
>
> /*
> * Inner function for parsing RFC 7730 from a buffer.
> * Returns a valid pointer on success, NULL otherwise.
> * The pointer must be freed with tal_free().
> */
> static struct tal *
> tal_parse_buffer(const char *fn, char *buf)
> {
> char *nl, *line;
> unsigned char *b64 = NULL;
> size_t sz;
> int rc = 0, b64sz;
> struct tal *tal = NULL;
> enum rtype rp;
> EVP_PKEY *pkey = NULL;
>
> if ((tal = calloc(1, sizeof(struct tal))) == NULL)
> err(EXIT_FAILURE, NULL);
>
> /* Begin with the URI section, comment section already removed. */
> while ((nl = strchr(buf, '\n')) != NULL) {
> line = buf;
> *nl = '\0';
>
> /* advance buffer to next line */
> buf = nl + 1;
>
> /* Zero-length line is end of section. */
> if (*line == '\0')
> break;
>
> /* Append to list of URIs. */
> tal->uri = reallocarray(tal->uri,
> tal->urisz + 1, sizeof(char *));
> if (tal->uri == NULL)
> err(EXIT_FAILURE, NULL);
>
> tal->uri[tal->urisz] = strdup(line);
> if (tal->uri[tal->urisz] == NULL)
> err(EXIT_FAILURE, NULL);
> tal->urisz++;
>
> /* Make sure we're a proper rsync/https URI. */
> if (strncasecmp(line, "https://";, 8) == 0) {
> if (!https_uri_parse(NULL, NULL, NULL, NULL, &rp, line)) {
> warnx("%s: RFC 8630 section 2.2: "
> "failed to parse URL: %s", fn, line);
> goto out;
> }
> } else if (!rsync_uri_parse(NULL, NULL, NULL, NULL, NULL, NULL, &rp,
> line)) {
> warnx("%s: RFC 8630 section 2.2: "
> "failed to parse URL: %s", fn, line);
> goto out;
> }
> if (rp != RTYPE_CER) {
> warnx("%s: RFC 7730 section 2.1: "
> "not a certificate URL: %s", fn, line);
> goto out;
> }
>
> }
>
> if (tal->urisz == 0) {
> warnx("%s: no URIs in manifest part", fn);
> goto out;
> } else if (tal->urisz > 1)
> warnx("%s: multiple URIs: using the first", fn);
>
> sz = strlen(buf);
> if (sz == 0) {
> warnx("%s: RFC 7730 section 2.1: subjectPublicKeyInfo: "
> "zero-length public key", fn);
> goto out;
> }
>
> /* Now the BASE64-encoded public key. */
> sz = ((sz + 2) / 3) * 4 + 1;
> if ((b64 = malloc(sz)) == NULL)
> err(EXIT_FAILURE, NULL);
> if ((b64sz = b64_pton(buf, b64, sz)) < 0)
> errx(EXIT_FAILURE, "b64_pton");
>
> tal->pkey = b64;
> tal->pkeysz = b64sz;
>
> /* Make sure it's a valid public key. */
> pkey = d2i_PUBKEY(NULL, (const unsigned char **)&b64, b64sz);
> if (pkey == NULL) {
> cryptowarnx("%s: RFC 7730 section 2.1: subjectPublicKeyInfo: "
> "failed public key parse", fn);
> goto out;
> }
> rc = 1;
> out:
> if (rc == 0) {
> tal_free(tal);
> tal = NULL;
> }
> EVP_PKEY_free(pkey);
> return tal;
> }
>
> /*
> * Parse a TAL from a file conformant to RFC 7730.
> * Returns the encoded data or NULL on failure.
> * Failure can be any number of things: failure to open file, allocate
> * memory, bad syntax, etc.
> */
> struct tal *
> tal_parse(const char *fn, char *buf)
> {
> struct tal *p;
> char *d;
> size_t dlen;
>
> p = tal_parse_buffer(fn, buf);
>
> if (p != NULL) {
> /* extract the TAL basename (without .tal suffix) */
> d = basename((char*)fn);
> if (d == NULL) {
> warnx("%s: basename", fn);
> } else {
> dlen = strlen(d);
> if (strcasecmp(d + dlen - 4, ".tal") == 0)
> dlen -= 4;
> if ((p->descr = malloc(dlen + 1)) == NULL)
> err(EXIT_FAILURE, NULL);
> memcpy(p->descr, d, dlen);
> p->descr[dlen] = 0;
> }
> }
> return p;
> }
>
>
>
> On 05/11/2019 11:01, Alexandre Hamada wrote:
> > Hi Claudio,
> >
> > I was testing some tal files, and when it contains an https url (RFC
> > 8630), it generates a segmentation fault.
> >
> > Thus, I would like to suggest adding the following on tal_parse/tal_free.
> >
> > Example:
> > https://rir.docker/ta/ta.cer
> >
> > MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAoFVWio0auchdC1mCbp4N
> > pSG9b43tXDIIOHHsc/6fTg0wdFdX68qHFYAwTlXVriXs5j0WmB8DZ1qiE4nyYfEj
> > kVIU6bVbz8WZ03BpajGO1AfbasBdbAr4SWjl8YCyCFjfojij2SnH/lbjxca5mSa0
> > wJAT4LEG/B4kMROcKRSebTCi8iDxMYha1GFFXBD9ze9qpEoUE3ZobQfITMNd8xZx
> > mZyg6bXXnT8qj7m5IrCs7dD4qrhjPTKbxwlpXFlS4pNdh9p9H/gFHxF7SbOLIq4m
> > 2rBQKNCMzvFnFgWJyM2QqiCb89WJf6kvyBjMbh7h/HygaXy8SuXUprExudQy8OZp
> > xwIDAQAB
> >
> >
> > Regards,
> > Alexandre Hamada
> >
> > @@ -142,18 +142,19 @@ tal_parse(const char *fn, char *buf)
> >
> > p = tal_parse_buffer(fn, buf);
> >
> > - /* extract the TAL basename (without .tal suffix) */
> > - d = basename((char*)fn);
> > - if (d == NULL)
> > - err(EXIT_FAILURE, "%s: basename", fn);
> > - dlen = strlen(d);
> > - if (strcasecmp(d + dlen - 4, ".tal") == 0)
> > - dlen -= 4;
> > - if ((p->descr = malloc(dlen + 1)) == NULL)
> > - err(EXIT_FAILURE, NULL);
> > - memcpy(p->descr, d, dlen);
> > - p->descr[dlen] = 0;
> > -
> > + if(p != NULL) {
> > + /* extract the TAL basename (without .tal suffix) */
> > + d = basename((char*)fn);
> > + if (d == NULL)
> > + err(EXIT_FAILURE, "%s: basename", fn);
> > + dlen = strlen(d);
> > + if (strcasecmp(d + dlen - 4, ".tal") == 0)
> > + dlen -= 4;
> > + if ((p->descr = malloc(dlen + 1)) == NULL)
> > + err(EXIT_FAILURE, NULL);
> > + memcpy(p->descr, d, dlen);
> > + p->descr[dlen] = 0;
> > + }
> > return p;
> > }
> >
> > @@ -231,9 +232,15 @@ tal_free(struct tal *p)
> > for (i = 0; i < p->urisz; i++)
> > free(p->uri[i]);
> >
> > - free(p->pkey);
> > - free(p->uri);
> > - free(p->descr);
> > + if (p->pkey != NULL) {
> > + free(p->pkey);
> > + }
> > + if (p->uri != NULL) {
> > + free(p->uri);
> > + }
> > + if (p->descr != NULL) {
> > + free(p->descr);
> > + }
> > free(p);
> > }
> >
>
