Query string authentication does not work in some cases

Bug #477776 reported by Neil Soman on 2009-11-07
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Eucalyptus
Fix Released
High
Unassigned
eucalyptus (Ubuntu)
Medium
Dustin Kirkland 

Bug Description

Walrus responds with a 403 in cases where the signature for query string authentication contains "+" characters.

The issue is with the URL decoder used in Walrus which will convert "+" to " ".

Neil Soman (neilsoman) wrote :

http://forum.eucalyptus.com/forum/viewtopic.php?f=5&t=1254

Thanks "sulicny" for reporting this issue.

Changed in eucalyptus:
status: New → Confirmed
Neil Soman (neilsoman) wrote :

------------------------------------------------------------
revno: 1067
committer: root
branch nick: 1.6
timestamp: Sat 2009-11-07 12:30:31 -0800
message:
  fixes #477776
------------------------------------------------------------

Changed in eucalyptus:
status: Confirmed → Fix Committed
importance: Undecided → High
Thierry Carrez (ttx) wrote :

Neil, any chance we could get access to that rev1067 diff ?
Apparently it was not committed to the public branch (lp:eucalyptus/1.6).

Neil Soman (neilsoman) wrote :

Thierry, please see,

http://bazaar.launchpad.net/~eucalyptus-maintainers/eucalyptus/1.6/revision/1067

(Apparently our launchpad sync script was failing. I have pushed the latest manually).

Thierry Carrez (ttx) wrote :

OK thanks, so it seems like a separate issue from bug 461156 (which also is about "+" escaping, but in a separate setting)

Chuck Short (zulcss) on 2009-11-10
Changed in eucalyptus (Ubuntu):
status: New → Triaged
importance: Undecided → High
Thierry Carrez (ttx) on 2009-11-13
Changed in eucalyptus (Ubuntu):
importance: High → Medium
sulicny (steve-ulicny) wrote :

Neil, we tried your patch, but experienced the same intermittent results and now confirm that it is always when the signature is generated with a "+". Ran across this sample code at Amazon and reversed it for the decode and it seems to have solved our problem.

http://docs.amazonwebservices.com/AWSECommerceService/latest/DG/index.html?Query_QueryAuth.html (Java example)

Also at:
http://pastebin.com/mb5e64d1

*** WalrusAuthenticationHandler.java 2009-11-05 05:25:20.000000000 -0500
--- WalrusAuthenticationHandler.java.ulicny 2009-11-17 15:10:30.000000000 -0500
***************
*** 252,258 ****
                                //query string authentication
                                String accesskeyid = parameters.remove(SecurityParameter.AWSAccessKeyId.toString());
                                try {
! String signature = URLDecoder.decode(parameters.remove(SecurityParameter.Signature.toString()), "UTF-8");
                                        if(signature == null) {
                                                throw new AuthenticationException("User authentication failed. Null signature.");
                                        }
--- 252,261 ----
                                //query string authentication
                                String accesskeyid = parameters.remove(SecurityParameter.AWSAccessKeyId.toString());
                                try {
! String signature = URLDecoder.decode(parameters.remove(SecurityParameter.Signature.toString()), "UTF-8")
! .replace("%20", "+")
! .replace("%2A", "*")
! .replace("~", "%7E");
                                        if(signature == null) {
                                                throw new AuthenticationException("User authentication failed. Null signature.");
                                        }

Neil Soman (neilsoman) wrote :

sulicny, you are right. The fix I committed handles just one case. Thanks for the link to the Amazon documentation.

I've applied the correct fix in revno 1076.

thanks!
neil

Neil Soman (neilsoman) wrote :

Oops revno 1077. thanks.

Neil Soman (neilsoman) wrote :

Further testing reveals that the committed fix handles most but not all cases. We still need the replace(" ", "+").

Revno 1082 adds this.

Changed in eucalyptus (Ubuntu):
status: Triaged → In Progress
assignee: nobody → Dustin Kirkland (kirkland)
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package eucalyptus - 1.6.1~bzr1083-0ubuntu1

---------------
eucalyptus (1.6.1~bzr1083-0ubuntu1) lucid; urgency=low

  [ Dustin Kirkland ]
  * Merge upstream bzr revision 1082; the following bugs have been fixed
    upstream since the last merge:
    - LP: #378969 - private bug
    - LP: #404842 - init script fix
    - LP: #434283 - existing keys should be overwritten unconditionally
    - LP: #445990 - run instance will fail if no kernel or ramdisk specified
    - LP: #447457 - euca_conf --register-sc ... check the number of parameters
    - LP: #449874 - fix incorrect help text (--delete-nodes doesn't exist)
    - LP: #451795 - show registered images in elastic fox
    - LP: #454405 - return correct networkIndex values on describeInstances
    - LP: #456877 - init script fix
    - LP: #456878 - fix for libvirt xen driver
    - LP: #460085 - fix rampart memory leak
    - LP: #461156 - fix authentication problem w/ userdata
    - LP: #461394 - fix multiple concurrent snapshots on the same volume
    - LP: #461444 - fix memory leaks in NC getConsoleOutput and startup_thread
    - LP: #469984 - fix iptables rules issue
    - LP: #477776 - fix query string authentication
    - LP: #480783 - allow api connection over https
    - LP: #482249 - fix "Describe Regions"
    - LP: #484217 - create keypair should return an error if key exists
    - LP: #490623 - parse RFC 1123 formatted datetime
  * debian/control:
    - make all package lists one-per-line (makes changes henceforth more
      readable), sort lists
    - depend on rampart >= 1.3.0-0ubuntu6, which fixes some shared library
      installation issues
  * debian/patches/04-axis2c-1.6.0-rampart-1.3.0.patch: drop this patch,
    since Eucalyptus 1.6.1 natively supports axis2c 1.6.0 now
  * debian/eucalyptus-cloud.install,
    debian/eucalyptus-common.eucalyptus.upstart,
    debian/eucalyptus-java-common.install, debian/eucalyptus-sc.install,
    debian/eucalyptus-walrus.install: update static version number strings
    from "1.6-devel" to "1.6.1"; (we should really find a better way to do
    this)
  * debian/patches/03-DESTDIR.patch: ported forward for merge
 -- Dustin Kirkland <email address hidden> Tue, 01 Dec 2009 21:09:28 -0600

Changed in eucalyptus (Ubuntu):
status: In Progress → Fix Released
Scott Moser (smoser) wrote :

I believe this should be fix-released in Eucalyptus? Right? Can someone do that?

chris grzegorczyk (chris-grze) wrote :

In this case, Fix-Committed should be interpreted as Fix-Released.

Changed in eucalyptus:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers