Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug in ocrc.c#combinecredentials #472

Merged
merged 8 commits into from
Sep 25, 2017
Merged

Conversation

DennisHeimbigner
Copy link
Collaborator

@DennisHeimbigner DennisHeimbigner commented Aug 25, 2017

  1. Fix bug in ocrc.c#combinecredentials where a null user+pwd
    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.

where a null user+pwd generates
garbage. This in turn interferes
with using .netrc because the garbage
user+pwd can (sometimes) override
the .netrc.
Not entirely sure what is going on
because it works as is under e.g. cygwin.
In any case it needs fixing.
@DennisHeimbigner
Copy link
Collaborator Author

It occurs to me that the reason it fails only sometimes is that memory allocated
when user and pwd are null may sometimes contain data and othertimes act like
a string of length 0. This would be pretty much rantom. This might acct for why the code
worked in some environments and not others.

@DennisHeimbigner
Copy link
Collaborator Author

BTW, this should fix e-support HAW-868287

@cofinoa
Copy link

cofinoa commented Aug 28, 2017

@DennisHeimbigner
The authentication by embedding credentials in the URI, or .dodsrc or .netrc it is not working.

Neither the version 4.4.1.1, compile from the source release, it's working

My gues it's because the libcurl version. I'm using the one bundle with the ubuntu repository and its version is 7.35.0.
I'll compile a newest one and try with it.

@DennisHeimbigner
Copy link
Collaborator Author

I would be surprised if 7.35 was the problem. I made it work with 7.29.
Do a couple of things.

  1. add HTTP.VERBOSE=1 to .dodsrc and send that output
  2. do a curl --version command and send that output

@cofinoa
Copy link

cofinoa commented Aug 28, 2017

The output from the compilation made from branch netrc.dmh

antonio@xps:~$ ncdump -h "https://rda.ucar.edu/thredds/dodsC/files/g/ds083.2/grib1/2000/2000.02/fnl_20000201_06_00.grib1"
reorder:
	--	HTTP.VERBOSE	1
https://rda.ucar.edu/thredds/dodsC/files/g/ds083.2/grib1/2000/2000.02/fnl_20000201_06_00.grib1 => /tmp/occookiebN9NGm
* Hostname was NOT found in DNS cache
*   Trying 128.117.181.113...
* Connected to rda.ucar.edu (128.117.181.113) port 443 (#0)
* found 173 certificates in /etc/ssl/certs/ca-certificates.crt
* 	 server certificate verification SKIPPED
* 	 common name: rda.ucar.edu (matched)
* 	 server certificate expiration date OK
* 	 server certificate activation date OK
* 	 certificate public key: RSA
* 	 certificate version: #3
* 	 subject: C=US,postalCode=80301,ST=CO,L=Boulder,STREET=3090 Center Green Drive,O=The University Corporation for Atmospheric Research,OU=CISL,CN=rda.ucar.edu
* 	 start date: Mon, 06 Jul 2015 00:00:00 GMT

* 	 expire date: Thu, 05 Jul 2018 23:59:59 GMT

* 	 issuer: C=US,ST=MI,L=Ann Arbor,O=Internet2,OU=InCommon,CN=InCommon RSA Server CA
* 	 compression: NULL
* 	 cipher: AES-128-CBC
* 	 MAC: SHA1
> GET /thredds/dodsC/files/g/ds083.2/grib1/2000/2000.02/fnl_20000201_06_00.grib1.dds HTTP/1.1
User-Agent: oc4.5.1-development
Host: rda.ucar.edu
Accept: */*

< HTTP/1.1 401 Unauthorized
< Date: Mon, 28 Aug 2017 17:16:42 GMT
* Server Apache/2.4.6 (CentOS) is not blacklisted
< Server: Apache/2.4.6 (CentOS)
< Strict-Transport-Security: max-age=63072000; includeSubdomains; preload
< Cache-Control: private
< Expires: Wed, 31 Dec 1969 17:00:00 MST
< WWW-Authenticate: Basic realm="THREDDS Data Server"
< Content-Language: en
< Content-Length: 994
< Content-Type: text/html;charset=utf-8
< 
* Connection #0 to host rda.ucar.edu left intact
syntax error, unexpected WORD_WORD, expecting SCAN_ATTR or SCAN_DATASET or SCAN_ERROR
context: <!DOCTYPE^ html><html><head><title>Apache Tomcat/8.0.36 - Error report</title><style type="text/css">H1 {font-family:Tahoma,Arial,sans-serif;color:white;background-color:#525D76;font-size:22px;} H2 {font-family:Tahoma,Arial,sans-serif;color:white;background-color:#525D76;font-size:16px;} H3 {font-family:Tahoma,Arial,sans-serif;color:white;background-color:#525D76;font-size:14px;} BODY {font-family:Tahoma,Arial,sans-serif;color:black;background-color:white;} B {font-family:Tahoma,Arial,sans-serif;color:white;background-color:#525D76;} P {font-family:Tahoma,Arial,sans-serif;background:white;color:black;font-size:12px;}A {color : black;}A.name {color : black;}.line {height: 1px; background-color: #525D76; border: none;}</style> </head><body><h1>HTTP Status 401 - </h1><div class="line"></div><p><b>type</b> Status report</p><p><b>message</b> <u></u></p><p><b>description</b> <u>This request requires HTTP authentication.</u></p><hr class="line"><h3>Apache Tomcat/8.0.36</h3></body></html>
opt/build-github/ncdump/ncdump: https://rda.ucar.edu/thredds/dodsC/files/g/ds083.2/grib1/2000/2000.02/fnl_20000201_06_00.grib1: NetCDF: Authorization failure
antonio@xps:~$ curl --version
curl 7.35.0 (x86_64-pc-linux-gnu) libcurl/7.35.0 OpenSSL/1.0.1f zlib/1.2.8 libidn/1.28 librtmp/2.3
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtmp rtsp smtp smtps telnet tftp 
Features: AsynchDNS GSS-Negotiate IDN IPv6 Largefile NTLM NTLM_WB SSL libz TLS-SRP 

PS: The output from the curl request (I have removed the Basic token )

antonio@xps:~$ curl -v -netrc-file=.netrc https://rda.ucar.edu/thredds/dodsC/files/g/ds083.2/grib1/2000/2000.02/fnl_20000201_06_00.grib1.das
* Hostname was NOT found in DNS cache
*   Trying 128.117.181.113...
* Connected to rda.ucar.edu (128.117.181.113) port 443 (#0)
* successfully set certificate verify locations:
*   CAfile: none
  CApath: /etc/ssl/certs
* SSLv3, TLS handshake, Client hello (1):
* SSLv3, TLS handshake, Server hello (2):
* SSLv3, TLS handshake, CERT (11):
* SSLv3, TLS handshake, Server key exchange (12):
* SSLv3, TLS handshake, Server finished (14):
* SSLv3, TLS handshake, Client key exchange (16):
* SSLv3, TLS change cipher, Client hello (1):
* SSLv3, TLS handshake, Finished (20):
* SSLv3, TLS change cipher, Client hello (1):
* SSLv3, TLS handshake, Finished (20):
* SSL connection using ECDHE-RSA-AES256-GCM-SHA384
* Server certificate:
* 	 subject: C=US; postalCode=80301; ST=CO; L=Boulder; street=3090 Center Green Drive; O=The University Corporation for Atmospheric Research; OU=CISL; CN=rda.ucar.edu
* 	 start date: 2015-07-06 00:00:00 GMT
* 	 expire date: 2018-07-05 23:59:59 GMT
* 	 subjectAltName: rda.ucar.edu matched
* 	 issuer: C=US; ST=MI; L=Ann Arbor; O=Internet2; OU=InCommon; CN=InCommon RSA Server CA
* 	 SSL certificate verify ok.
* Server auth using Basic with user '[email protected]'
> GET /thredds/dodsC/files/g/ds083.2/grib1/2000/2000.02/fnl_20000201_06_00.grib1.das HTTP/1.1
> Authorization: Basic YYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY
> User-Agent: curl/7.35.0
> Host: rda.ucar.edu
> Accept: */*
> Referer: trc-file=.netrc
> 
< HTTP/1.1 200 OK
< Date: Mon, 28 Aug 2017 17:21:15 GMT
* Server Apache/2.4.6 (CentOS) is not blacklisted
< Server: Apache/2.4.6 (CentOS)
< Strict-Transport-Security: max-age=63072000; includeSubdomains; preload
< Cache-Control: private
< Expires: Wed, 31 Dec 1969 17:00:00 MST
< XDODS-Server: opendap/3.7
< Content-Description: dods-das
< Transfer-Encoding: chunked
< Content-Type: text/plain; charset=UTF-8
< 
Attributes {
    LatLon_Projection {
        String grid_mapping_name "latitude_longitude";
        Float64 earth_radius 6367470.0;
    }
    lat {
        String units "degrees_north";
[REST OF OUTPUT REMOVED]

@DennisHeimbigner
Copy link
Collaborator Author

  1. What github branch of netcdf are you using?
  2. where in the file system are .dodsrc and .netrc located?
  3. Show the contents of .dodsrc
  4. show the contents of .netrc (with password changed to XXXX)

@cofinoa
Copy link

cofinoa commented Aug 28, 2017

  1. What github branch of netcdf are you using?

netrc.dmh

  1. Where in the file system are .dodsrc and .netrc located?

in the working directory
After looking around my working directory I have discovered another RC file named .daprc that it was been read by the library instead the .dodsrc.

The credentials been provided by a .netrc or the HTTP.CREDENTIALS.USERPASSWORD keyword it's working.

For the rda.ucar.edu opendap service, username been provided includes @ character. But it is not working if username is provided with no encoding in the URI endpoint.

With respect to providing credentials by .daprc or .netrc works if they are encoded or decoded

@DennisHeimbigner
Copy link
Collaborator Author

After looking around my working directory I have discovered another RC file named .daprc
that it was been read by the library instead the .dodsrc.
The documenation says that .daprc will take precedence over the .dodsrc; you should remove
the .daprc.

The credentials been provided by a .netrc or the HTTP.CREDENTIALS.USERPASSWORD
keyword it's working.
I do not understand what you mean. You can use the USERPASSWORD, but it is insecure
because it gets passed to all the servers in the redirect chain.

With respect to providing credentials by .daprc or .netrc works if they are encoded or decoded
Again, I do not understand what this means. Are you saying that.

So, at this point, and with .daprc deleted, what is the state of things? What works and what
does not work.

@cofinoa
Copy link

cofinoa commented Aug 28, 2017

  • providing credentials in the .daprc/.dodsrc file: works ( in both cases encoded and decoded)
  • providing credentials in the a .netrc file (referenced by the .daprc/.dodsrc): works (in both cases encoded and decoded)
  • providing credentials in the URI: doesn't work (I'm only tested the encoded version)

@DennisHeimbigner
Copy link
Collaborator Author

providing credentials in the URI: doesn't work (I'm only tested the encoded version)
And this is because the @ is not being properly encoded?

@cofinoa
Copy link

cofinoa commented Aug 28, 2017

providing credentials in the URI: doesn't work (I'm only tested the encoded version)

And this is because the @ is not being properly encoded?

Credentials have to be provided with the @ character encoded as %40. If it is provided as @ then the uri parser it's not able to extract the credentials from the URI.

With version 4.4.1.1 encoded creadentials at URI works. In the the compiled branch netrc.dmh doesn't work the encoded credentials.

Note: I haven't tested with credentials that doesn't have special characters which need to be encoded

@DennisHeimbigner
Copy link
Collaborator Author

Ok, let me see if I can add a fix for using escaped characters in the user+pwd in url.

@cofinoa
Copy link

cofinoa commented Aug 29, 2017

@DennisHeimbigner
the problem it's that the last character in the password provided in the URI it's been truncated.

Then the problem is not related with encoding characters.

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.
@cofinoa
Copy link

cofinoa commented Aug 30, 2017

@DennisHeimbigner

The current commit on netrc.dmh works:

  1. embedding escaped credentials at URI
  2. using escaped and non-escaped credentials at .daprc. (Note: probably unescaped credentials with a : character would have some troubles. Apart from using escaped ones this could be overcome spliting them using RC keys USER and PASSWORD)
  3. using escaped and non-escaped credentials at .netrc file

with respect to the RC keys and doc it may be would be useful introduce this improvements:

  1. setence about the precedence of DAPRCFILE env variable should be introduced.
  2. add HTTP.CREDENTIALS.USER and HTTP.CREDENTIALS.PASSWORD to the RC keys
  3. add HTTP.NETRC the curl option affected (i.e. CURLOPT_NETRC_FILE)
  4. add a clarification about VALIDATE and VERIFYPEER RC keys
  5. Fix wrong names for some curl options: CURLOPT_SSLCAPATH -> CURLOPT_CAPATH, CURLOPT_SSLCAINFO -> CURLOPT_CAINFO
  6. make PROXY RC options it's own namespace to be more coherent with SSL. HTTP.PROXY_SERVER -> HTTP.PROXY.SERVER .
  7. add reference to https://curl.haxx.se/libcurl/c/curl_easy_setopt.html
  8. adding reference to SSL engines could clarify CAINFO or CAPATH usage, or other special SSL engines cases: https://curl.haxx.se/docs/ssl-compared.html

@DennisHeimbigner
Copy link
Collaborator Author

  1. Added support for usr:pwd escaping
  2. Updated documentation as suggested, but only partly. Some will have to wait
    for unification of the rc processing.
    I think this is now ready to be merged.

DennisHeimbigner added a commit that referenced this pull request Sep 3, 2017
Primary change is to cleanup code and remove duplicated code.

1. Unify the rc file reading into libdispatch/drc.c. Eventually extend
   if we need rc file for netcdf itself as opposed to the dap code.
2. Unify the extraction from the rc file of DAP authorization info.
3. Misc. other small unifications: make temp file, read file.
4. Avoid use of libcurl when reading file:// because
   there is some kind of problem with the Visual Studio version.
   Might be related to the winpath problem.
   In any case, do direct read instead.
5. Add new error code NC_ERCFILE for errors in reading RC file.
6. Complete documentation cleanup as indicated in this comment
   #472 (comment)
7. Convert some occurrences of #ifdef _WIN32 to #ifdef _MSC_VER
@DennisHeimbigner
Copy link
Collaborator Author

Implemented the remainder of the suggested changes in
pull request #477

@WardF WardF merged commit 8416b11 into v4.5.0-release-branch Sep 25, 2017
@WardF WardF deleted the netrc.dmh branch September 25, 2017 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants