Skip to content

Commit

Permalink
1. Fix bug in ocrc.c#combinecredentials where a null user+pwd
Browse files Browse the repository at this point in the history
generates garbage. This in turn interferes with using .netrc
because the garbage user+pwd can will override the
.netrc. Note that this may work ok sometimes
if the garbage happens to start with a nul character.

2. It turns out that the user:pwd combination needs to support
character escaping. One reason is the user may contain an '@' character.
The other is that modern password rules make it not unlikely that
the password will contain characters that interfere with url parsing.
So, the rule I have implemented is that all occurrences of the user:pwd
format must escape any dodgy characters. The escape format is URL escaping
of the form %XX. This applies both to user:pwd
embedded in a URL as well as the use of HTTP.CREDENTIALS.USERPASSWORD
in a .dodsrc/.daprc file. The user and password in .netrc must not
be escaped. This is now documented in docs/auth.md

The fix for #2 actually obviated #1. Now, internally, the user and pwd
are stored separately and not in the user:pwd format. They are combined
(and escaped) only when needed.
  • Loading branch information
DennisHeimbigner committed Aug 29, 2017
1 parent e226cc7 commit bc9e41a
Show file tree
Hide file tree
Showing 16 changed files with 306 additions and 150 deletions.
57 changes: 43 additions & 14 deletions docs/auth.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
netCDF Authorization Support
============================
======================================
<!-- double header is needed to workaround doxygen bug -->

# netCDF Authorization Support {#Header}
Expand Down Expand Up @@ -38,8 +38,11 @@ This username and password will be used if the server asks for
authentication. Note that only simple password authentication
is supported in this format.
Specifically note that [redirection-based](#REDIR)
authorization will not work with this because the username and password
will only be used on the initial request, not the redirection
authorization may not work with this because the username and password
will only be used on the initial request, not the redirection.
Note also that the `user:password` form may contain characters that must be
escaped. See the <a href="#USERPWDESCAPE">password escaping</a> section to see
how to properly escape the user and password.

## RC File Authentication {#DODSRC}
The netcdf library supports an _rc_ file mechanism to allow the passing
Expand Down Expand Up @@ -134,6 +137,27 @@ specifies the absolute path of the .netrc file.
See [redirection authorization](#REDIR)
for information about using .netrc.

## Password Escaping {#USERPWDESCAPE}
With current password rules, it is is not unlikely that the password
will contain characters that need to be escaped. Similarly, the user
may contain characters such as '@' that need to be escaped. To support this,
it is assumed that all occurrences of `user:`password` use URL (i.e. %%XX)
escaping for at least the characters in the table below.
Note that escaping must be used when the user+pwd is embedded in the URL.
It must also be used when the user+pwd is specified in the `.dodsrc/.daprc` file
via HTTP.CREDENTIALS.USERPASSWORD.
Escaping should not be used in the `.netrc` file.

The relevant characters and their escapes are as follows.
<table>
<tr><th>Character</th><th>Escaped Form</th>
<tr><td>'@'</td><td>%40</td>
<tr><td>':'</td><td>%3a</td>
<tr><td>'?'</td><td>%3f</td>
<tr><td>'#'</td><td>%23</td>
<tr><td>'/'</td><td>%2f</td>
</table>

## Redirection-Based Authentication {#REDIR}

Some sites provide authentication by using a third party site
Expand All @@ -150,16 +174,18 @@ using the _https_ protocol (note the use of _https_ instead of _http_).
4. URS sends a redirect (with authorization information) to send
the client back to the SOI to actually obtain the data.

It turns out that libcurl uses the password in the `.daprc`
file (or from the url)
only for the initial connection. This causes problems because
the redirected connection is the one that actually requires the password.
This is where the `.netrc` file comes in. Libcurl will use `.netrc` for
the redirected connection. It is possible to cause libcurl to use
the `.daprc` password always, but this introduces a security hole
because it may send the initial user+pwd to the redirection site.
In summary, if you are using redirection, then you must create a `.netrc`
file to hold the password for the site to which the redirection is sent.
It turns out that libcurl, by default, uses the password in the
`.daprc` file (or from the url) for all connections that request
a password. This causes problems because only the the specific
redirected connection is the one that actually requires the password.
This is where the `.netrc` file comes in. Libcurl will use `.netrc`
for the redirected connection. It is possible to cause libcurl
to use the `.daprc` password always, but this introduces a
security hole because it may send the initial user+pwd to every
server in the redirection chain.
In summary, if you are using redirection, then you are
''strongly'' encouraged to create a `.netrc` file to hold the
password for the site to which the redirection is sent.

The format of this `.netrc` file will contain lines that
typically look like this.
Expand All @@ -170,11 +196,14 @@ where the machine, mmmmmm, is the hostname of the machine to
which the client is redirected for authorization, and the
login and password are those needed to authenticate on that machine.

The `.netrc` file can be specified by
The location of the `.netrc` file can be specified by
putting the following line in your `.daprc`/`.dodsrc` file.

HTTP.NETRC=<path to netrc file>

If not specified, then libcurl will look first in the current
directory, and then in the HOME directory.

One final note. In using this, you MUST
to specify a real file in the file system to act as the
cookie jar file (HTTP.COOKIEJAR) so that the
Expand Down
16 changes: 9 additions & 7 deletions include/ncuri.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@
#define NCU_ECONSTRAINTS (11)

/* Define flags to control what is included by ncuribuild*/
#define NCURIPATH 1
#define NCURIPWD 2
#define NCURIQUERY 4
#define NCURIFRAG 8
#define NCURIENCODE 16 /* If output should be encoded */
#define NCURIPATH 1
#define NCURIPWD 2
#define NCURIQUERY 4
#define NCURIFRAG 8
#define NCURIENCODE 16 /* If output should be encoded */
#define NCURIBASE (NCURIPWD|NCURIPATH)
#define NCURISVC (NCURIQUERY|NCURIBASE) /* for sending to server */
#define NCURIALL (NCURIPATH|NCURIPWD|NCURIQUERY|NCURIFRAG) /* for rebuilding after changes */
Expand Down Expand Up @@ -81,9 +81,11 @@ extern const char* ncurilookup(NCURI*, const char* param);
extern const char* ncuriquerylookup(NCURI*, const char* param);

/* URL Encode/Decode */
extern char* ncuriencode(char* s, char* allowable);
extern char* ncuridecode(char* s);
extern char* ncuridecodeonly(char* s, char*);
/* Encode using specified character set */
extern char* ncuriencodeonly(char* s, char* allowable);
/* Encode user or pwd */
extern char* ncuriencodeuserpwd(char* s);

#if defined(_CPLUSPLUS_) || defined(__CPLUSPLUS__) || defined(__CPLUSPLUS)
}
Expand Down
16 changes: 11 additions & 5 deletions libdap4/d4curlfunctions.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,11 @@ set_curlflag(NCD4INFO* state, int flag)
{
int ret = NC_NOERR;
switch (flag) {
case CURLOPT_USERPWD:
if(state->curl->creds.userpwd != NULL) {
CHECK(state, CURLOPT_USERPWD, state->curl->creds.userpwd);
case CURLOPT_USERPWD: /* Do both user and pwd */
if(state->curl->creds.user != NULL
&& state->curl->creds.pwd != NULL) {
CHECK(state, CURLOPT_USERNAME, state->curl->creds.user);
CHECK(state, CURLOPT_PASSWORD, state->curl->creds.pwd);
CHECK(state, CURLOPT_HTTPAUTH, (OPTARG)CURLAUTH_ANY);
}
break;
Expand Down Expand Up @@ -107,8 +109,10 @@ set_curlflag(NCD4INFO* state, int flag)
if(state->curl->proxy.host != NULL) {
CHECK(state, CURLOPT_PROXY, state->curl->proxy.host);
CHECK(state, CURLOPT_PROXYPORT, (OPTARG)(long)state->curl->proxy.port);
if(state->curl->proxy.userpwd) {
CHECK(state, CURLOPT_PROXYUSERPWD, state->curl->proxy.userpwd);
if(state->curl->proxy.user != NULL
&& state->curl->proxy.pwd != NULL) {
CHECK(state, CURLOPT_PROXYUSERNAME, state->curl->proxy.user);
CHECK(state, CURLOPT_PROXYPASSWORD, state->curl->proxy.pwd);
#ifdef CURLOPT_PROXYAUTH
CHECK(state, CURLOPT_PROXYAUTH, (long)CURLAUTH_ANY);
#endif
Expand Down Expand Up @@ -264,6 +268,7 @@ NCD4_curl_protocols(NCD4globalstate* state)
}


#if 0
/*
"Inverse" of set_curlflag;
Given a flag and value, it updates state.
Expand Down Expand Up @@ -349,6 +354,7 @@ NCD4_set_curlstate(NCD4INFO* state, int flag, void* value)
done:
return THROW(ret);
}
#endif

void
NCD4_curl_printerror(NCD4INFO* state)
Expand Down
1 change: 0 additions & 1 deletion libdap4/d4curlfunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ extern ncerror NCD4_set_flags_perfetch(NCD4INFO*);
extern ncerror NCD4_set_flags_perlink(NCD4INFO*);

extern ncerror NCD4_set_curlflag(NCD4INFO*,int);
extern ncerror NCD4_set_curlstate(NCD4INFO* state, int flag, void* value);

extern void NCD4_curl_debug(NCD4INFO* state);

Expand Down
6 changes: 4 additions & 2 deletions libdap4/d4file.c
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,10 @@ freeCurl(NCD4curl* curl)
nullfree(curl->ssl.cainfo);
nullfree(curl->ssl.capath);
nullfree(curl->proxy.host);
nullfree(curl->proxy.userpwd);
nullfree(curl->creds.userpwd);
nullfree(curl->proxy.user);
nullfree(curl->proxy.pwd);
nullfree(curl->creds.user);
nullfree(curl->creds.pwd);
if(curl->curlflags.createdflags & COOKIECREATED)
d4removecookies(curl->curlflags.cookiejar);
nullfree(curl->curlflags.cookiejar);
Expand Down
Loading

0 comments on commit bc9e41a

Please sign in to comment.