On Tue, Jun 24, 2014 at 10:55:44AM -0700, Philip Guenther wrote:
> On Tue, Jun 24, 2014 at 9:01 AM, Sébastien Marie <
> [email protected]> wrote:
> 
> > As I see not activity or feedback for this one line patch, I think it
> > need more explain ?
> >
> 
> Sorry, the patch is incorrect; per RFC 3986, the parser is correct to split
> the authority on the first '@'.  The correct method to include an '@' in
> the userinfo part is to percent-encode it as %40.
> 

Thanks for pointing me the good RFC for the format of userinfo in URI.

But, I am not sure to understand *where* is located my problem: if it is
a problem in ftp(1), or a problem in specific service I try to used with
ftp (dyndns server, the http reply come from ngnix server).

Currently ftp(1) manage http, https with url_get function. The basic
auth implemented here do *not* try to decode any urlencoded information.

So pass "user%40example.com:password" as userinfo in URI will result to
base64 "user%40example.com:password" (and not
"[email protected]:password").

The dyndns service reply with "401 Unauthorized".


Comparing with curl(1) (in ports), a user:password information passed as
argument is passed to base64 "as it", but a user:password passed in the
URI is firstly urldecoded before base64.

$ curl -v -u '[email protected]:password' 'http://localhost/'
Authorization: Basic bmFtZUBleGFtcGxlLmNvbTpwYXNzd29yZA==

$ curl -v 'http://name%40example.com:password@localhost/'
Authorization: Basic bmFtZUBleGFtcGxlLmNvbTpwYXNzd29yZA==

"bmFtZUBleGFtcGxlLmNvbTpwYXNzd29yZA==" is "[email protected]:password"


So, I think ftp(1) should urldecode userinfo before base64. I propose
another patch that implement urldecoding of userinfo (as it come from
URI).

Some comments about the patch:
 - I stop use p variable for userinfo parse: I declare and use two new
   variables, userinfo and userinfoend. I hope the code is more
   readable.

 - for urldecoding, I use an already defined function. Memory is
   allocated and checked by this function.

 - I didn't try to parse userinfo as "user:password". The userinfo
   string is passed "as it" to urldecode.

Thanks.
-- 
Sébastien Marie



Index: fetch.c
===================================================================
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.122
diff -u -p -r1.122 fetch.c
--- fetch.c     20 May 2014 01:25:23 -0000      1.122
+++ fetch.c     25 Jun 2014 06:49:11 -0000
@@ -474,16 +474,30 @@ noslash:
         */
        if (proxyenv == NULL &&
            (!strcmp(scheme, HTTP_URL) || !strcmp(scheme, HTTPS_URL))) {
-               if ((p = strchr(host, '@')) != NULL) {
-                       size_t authlen = (strlen(host) + 5) * 4 / 3;
-                       *p = 0; /* Kill @ */
+               char *userinfo, *userinfoend;
+
+               /* extract userinfo part */
+               if ((userinfoend = strchr(host, '@')) != NULL) {
+                       size_t authlen;
+
+                       /* separate userinfo and host components */
+                       userinfo = host;
+                       host = userinfoend + 1;
+                       *userinfoend = '\0';
+
+                       /* urldecode userinfo */
+                       userinfo = urldecode(userinfo);
+
+                       /* build Basic auth */
+                       authlen = (strlen(userinfo) + 5) * 4 / 3;
                        if ((auth = malloc(authlen)) == NULL)
                                err(1, "Can't allocate memory for "
                                    "authorization");
-                       if (b64_ntop(host, strlen(host),
+                       if (b64_ntop(userinfo, strlen(userinfo),
                            auth, authlen) == -1)
                                errx(1, "error in base64 encoding");
-                       host = p + 1;
+
+                       free(userinfo);
                }
        }
 #endif /* SMALL */

Reply via email to