User-Agent: Cyrus-JMAP/3.5.0-alpha0-4514-g2bdc19e04f-fm-20211209.002-g2bdc19e0
Mime-Version: 1.0
Message-Id: <f8af7833-6dd5-43b5-a093-0cd41678b99a@www.fastmail.com>
In-Reply-To: <20211214213430.029b34c5@vostro>
References: <20211213212929.925-1-git@jfarr.cc>
 <20211214213430.029b34c5@vostro>
Date: Wed, 15 Dec 2021 00:09:36 +0100
From: "Jan Hendrik Farr" <git@jfarr.cc>
To: "Timo Teras" <timo.teras@iki.fi>
Cc: ~alpine/apk-tools@lists.alpinelinux.org
Subject: Re: [PATCH 1/1] libfetch: Allow special characters in http basic auth
Content-Type: text/plain

Hi,

> Thanks! There is a related bug
> https://gitlab.alpinelinux.org/alpine/apk-tools/-/issues/10775
>
> If this get's fixed, add to commit message:
> ref #10775
>

Looks like this patch should fix that issue.



> Is the upstream file somewhere?

Yes. As I understand, libfetch.c was originally taken from NetBSD (with certain bits taken from FreeBSD): https://github.com/NetBSD/src/blob/trunk/external/bsd/fetch/dist/libfetch/fetch.c

That file has no fix for this and has the same issue. I took this fix from the FreeBSD version: https://cgit.freebsd.org/src/tree/lib/libfetch/fetch.c

I took the functions fetch_hexval and fetch_pctdecode (both exact copies) from that file and integrated them into fetchParseURL.



> minor readability issue; i'd prefer something like:
>   if (!dlen)
>     return NULL;
>   dlen--;
>   *dst++ = c;
>
> To make the error path and normal path more obvious.

Since fetch_pctdecode is an exact copy from FreeBSD I would prefer to keep it unchanged.



> This will terminate also with ':' in the string now. And will handle
> the rest of the URL incorrectly, and break password with ':'.

Usernames used for http basic authentication are not allowed to contain ":" according RFC7617 section 2.1 (https://datatracker.ietf.org/doc/html/rfc7617#section-2.1). They are allowed to contain other special characters though and the password is allowed to contain special characters including ":". The characters ":", "@", or "/" present in the username or password must be encoded using percent encoding per RFC1738 section 3.1 (https://datatracker.ietf.org/doc/html/rfc1738#section-3.1). So the only ":" present in this part of the string will be the separator between username and password.

Example:

username: foo@123/bar
password: pass1234:hello:

A correct proxy setting (or other URL) using these credentials should be encoded by the user as:

http://foo%40123%2Fbar:pass1234%3Ahello%3A@example.com

libfetch should then decode the percent encoding to obtain the username and password in plain text, append them using a ":", and then encode the result using base64 to be used during http basic authentication:

result: Zm9vQDEyMy9iYXI6cGFzczEyMzQ6aGVsbG86

The decoding of the percent encoding is added in this patch and the appending of username and password and base64 encoding is already correctly handled in http.c.



Would you prefer for me to open a merge request on your Gitlab instead? If yes: Which branch should I open the request for?


With kind regards,
Jan