Bug 760592 - 401 Error during post request
Summary: 401 Error during post request
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: CloudForms Cloud Engine
Classification: Retired
Component: aeolus-conductor
Version: 1.0.0
Hardware: Unspecified
OS: Unspecified
unspecified
medium
Target Milestone: 1.0.2
Assignee: Greg Blomquist
QA Contact: dgao
URL:
Whiteboard:
Depends On: 760377
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-12-06 15:11 UTC by dgao
Modified: 2012-12-04 14:53 UTC (History)
15 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
RestClient caused part of the request to drop, which caused OAuth_body_hash to appear invalid. A POST or PUT request returned a 401 Unauthorized even when using the same consumer key/secret. This bug fix updates Conductor, and uses Ruby OAuth client instead of RestClient, so Conductor connects to the config-server without unauthorized errors.
Clone Of: 760377
: 782899 (view as bug list)
Environment:
Last Closed: 2012-12-04 14:53:26 UTC


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHEA-2012:1516 0 normal SHIPPED_LIVE CloudForms Cloud Engine 1.1 update 2012-12-04 19:51:45 UTC

Description dgao 2011-12-06 15:11:14 UTC
+++ This bug was initially created as a clone of Bug #760377 +++

Automation uses httpclient in conjuncture of signpost (oauth lib) to make requests to configserver. For regular GET request, the server was able to authorized and return the proper information. For POST or PUT, it returns a 401 Unauthorized error while using the same consumer key/secret.

After speaking w/ the dev, it sounds like conductor does not take the entire header and generate the hash and signature. Instead it omits the "data=..." section. 

As a result, because the automation includes the data section when it generate the hash and signature, it fails verification on configserver side.

--- Additional comment from gblomqui on 2011-12-06 09:52:56 EST ---

The correct behavior according to oauth standards is to include the entire post body payload when generating the signature in the HTTP request.  However, Conductor is using the RestClient ruby library with a request preprocessor that signs the request via oauth prior to sending the request.  Either the way Conductor is using the RestClient library, or the library itself, is not including the entire post body when signing the request.  Namely, it's not including the "data=..." part of the post body payload when generating the oauth signature.

The workaround on the server side (i.e., inside of configserver) is to omit the "data=..." part of the post body when validating the signature.  The security implications here are minimal, since there is a timestamp as well as other oauth headers included in the signed value.  

The major downside to this approach is that it is non-standard.  Which leads right back to David's comment about test automation failing.

Comment 1 wes hayutin 2012-01-03 17:41:52 UTC
adding ce-sprint-next bugs to ce-sprint

Comment 2 Matt Wagner 2012-01-04 18:29:37 UTC
In #760377, Greg got around this by <a href="https://fedorahosted.org/pipermail/aeolus-devel/2011-December/007547.html">using the oauth gem directly</a> instead of using restclient at all. That's probably a good path to take here as well.

Comment 3 Matt Wagner 2012-01-04 20:43:28 UTC
Where this actually matters for Conductor (besides the config server code, which was fixed in 760377) is in our iwhd client in rubygem-aeolus-image. Fixing it there would mean that we want to implement https://www.aeolusproject.org/redmine/issues/2606 to have iwhd verify the full hash.

Comment 4 Matt Wagner 2012-01-04 22:33:00 UTC
Patch sent for feedback: http://lists.fedorahosted.org/pipermail/aeolus-devel/2012-January/007838.html

Comment 5 Matt Wagner 2012-01-10 18:36:00 UTC
Pushed to aeolus-image-rubygem's master:

commit 67fdf6ff1a395624c81df8b16832248b45185505
Author: Matt Wagner <matt.wagner>
Date:   Wed Jan 4 17:04:10 2012 -0500

    Use oauth gem directly for signing OAuth requests
    
    Using restclient appears to cause part of the request to get dropped
    on the floor, causing oauth_body_hash to be invalid.
    
    This appears backwards compatible with existing versions of iwhd.
    
    Resolves https://bugzilla.redhat.com/show_bug.cgi?id=760592

Comment 6 Matt Wagner 2012-01-10 18:39:25 UTC
I've pushed this, but note that the code to validate this on the iwhd side isn't yet committed. It's possible that it won't be committed: http://lists.fedorahosted.org/pipermail/aeolus-devel/2012-January/007896.html

This patch is still a good thing in that we're sending a properly-signed request to iwhd now, but it will have no real-world benefit as iwhd isn't checking whether the oauth_body_hash is correct or not unless the patch linked in the previous paragraph is committed to iwhd.

Comment 7 Steve Linabery 2012-01-10 22:02:46 UTC
Blindly setting this to ON_QA in the belief that a fix is included in the latest aeolus packages.

Comment 8 wes hayutin 2012-01-12 16:14:20 UTC
bugs in verified or on_qa moving off tracker

Comment 9 dgao 2012-01-12 21:23:13 UTC
Fixed in the latest build of audrey client and configserver:

aeolus-audrey-agent-0.4.3-1.el6.noarch.rpm                                  
aeolus-configserver-0.4.5-1.el6.noarch.rpm

Comment 10 dgao 2012-01-12 21:25:10 UTC
oops mistakenly commented the wrong bug, disregard the last comment.

Comment 11 Greg Blomquist 2012-01-18 19:46:54 UTC
Setting this back to POST because https://fedorahosted.org/pipermail/aeolus-devel/2011-December/007547.html has never been applied.

I'll work with someone from conductor to get this in.

Comment 12 Greg Blomquist 2012-01-18 20:07:20 UTC
This bug is to track the changes on the conductor side of the conductor -> config server communication.

It looks like Matt's comments relate to the same type of communication between conductor -> iwhd.  This should be opened and tracked in another bug.

Comment 13 Steve Linabery 2012-01-18 21:10:30 UTC
rubygem-aeolus-image-0.3.0-3.el6.src.rpm
67fdf6ff1a395624c81df8b16832248b45185505

Comment 14 Greg Blomquist 2012-01-27 14:37:30 UTC
Ugh, according to dgao, this bug is actually to track the conductor side of the conductor -> configserver oauth relationship.  All of the comments related to the communication with iwhd should be associated with https://bugzilla.redhat.com/show_bug.cgi?id=782899.

I submitted a patch for this on Dec.16,2011, but didn't get it reviewed.  So, I'm rebasing the patch, retesting and re-posting.

Moving this back to assigned.

Comment 15 Hugh Brock 2012-01-27 18:00:26 UTC
Moving this to 1.1. Dev agrees this is too invasive to fix this late in the game.

Comment 16 Greg Blomquist 2012-09-10 15:17:08 UTC
https://github.com/aeolusproject/conductor/pull/46

Comment 17 Greg Blomquist 2012-09-10 15:47:03 UTC
Validation notes:

What this patch changes:

This patch changes the way that conductor authenticates with config server.  The latest config server (aeolus-configserver-0.4.10-2) no longer accepts the way that conductor was authenticating--which was not fully signing the request body.

Now, config server requires that the request POST body be fully and correctly signed.

This patch removes RestClient as the mechanism for building the oauth request to config server.  Instead, it now uses the Ruby OAuth client directly to build the oauth request.

How to validate this change actually works:

Prereqs:
  - an unpatched conductor
  - a provider account
  - a config server running version 0.4.10-2

Setup:

a) associate the config server with the provider account
  - in conductor, navigate to the provider account, and click "Add" next to config server
  - enter the config server's endpoint (https://fqdn)
  - enter the oauth key
  - enter the oauth secret
  - save

b) launch a deployable with at least one service (this doesn't need to be a complex service ... any service will do)

c)  witness the 404 in the config server logs and conductor fails to launch the deployment

Validation:

a) Patch conductor

b) launch the same deployable

c) see the 200 in the config server log and conductor successfully launches the deployment.


Getting a config server:

The easiest way to get access to a config server is to work with QE.  Talk to dgao at redhat dot com.  He usually has one running and can provide the connection details as well as access to the config server logs.

Comment 18 Tzu-Mainn Chen 2012-09-10 18:21:28 UTC
Thanks for the detailed instructions!  ACK'd and pushed.

Comment 19 Tzu-Mainn Chen 2012-09-11 19:42:13 UTC
The initial patch had a minor flaw that I failed to detect.  Here's the reworked patch, which is now in master:

https://github.com/aeolusproject/conductor/pull/55

Comment 20 Tzu-Mainn Chen 2012-09-11 20:50:53 UTC
Pushed to 1.1.  Just to be clear, https://github.com/aeolusproject/conductor/pull/46 was reverted; https://github.com/aeolusproject/conductor/pull/55 is what's been pushed.

Comment 22 dgao 2012-09-21 19:42:28 UTC
Patch is good for aeolus-conductor-0.13.7-1.el6cf.noarch

Comment 23 Julie 2012-10-23 07:14:37 UTC
technical notes added. Feedback appreciated.

Comment 25 errata-xmlrpc 2012-12-04 14:53:26 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHEA-2012-1516.html


Note You need to log in before you can comment on or make changes to this bug.