checkbox should send a referer header when it POSTs data to Launchpad.

Bug #550973 reported by Abel Deuring on 2010-03-29
20
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Checkbox
Undecided
Marc Tardif
Launchpad itself
Undecided
Abel Deuring
checkbox (Ubuntu)
Undecided
Unassigned
Lucid
Undecided
Unassigned

Bug Description

The impact of this bug is adheres to an inconsistent interface in Launchpad. The proposed fix adds the REFERER information to the HTTP header which is consistent with all the other Launchpad interfaces. A workaround is currently provided in Launchpad but it would be nice to have this bug fixed in an LTS release so that Launchpad can eventually remove this workaround safely.

Since ca. 2010-3-24, Launchpad requires a referer header for all POST requests, see bug 529348 . We will exempt the /+hwdb/+submit URL for now from this requirement, but in order to prevent future CRSF problems, checkbox should send a referer header.

From #launchpad-dev, 2010-03-29:

(17:39:38) gary_poster: adeuring: so...actually, I also suggest that they change their script now to include a REFERER. That way eventually legacy clients will "just work," and sooner than if they wait to be able to do whatever it is they need to do through the webservice API
(17:40:53) adeuring: gary_poster: yes, checkbox should do that. But it is installed by default on every Ubuntu system, and getting rid of old version will ned quite some time...
(17:41:38) gary_poster: adeuring: ack, so let's get started ;-) getting the change into lucid would be a *big* win in that regard

Related branches

Gary Poster (gary) wrote :

I talked with Marc and remembered what the hwdb app actually is--that is, a completely separate application that basically happens to co-habitate with the Launchpad codebase and database, but is not exposed through the Launchpad browser interface or launchpadlib.

In that light, whether a REFERER header is required is more of a question for the specs, if they exist, of what the hwdb API is. It's probably a reasonable assertion that a REFERER header doesn't belong in them.

My new, new, new recommendation is that we make sure that the specs for the hwdb are clearly stated and well-tested in Launchpad, whatever they are. They probably are already tested, Launchpad generally being pretty well tested; perhaps Zope bug 98437 (which we work around in the new tests) caused the test to appear falsely sufficient in this regard.

If the hwdb specs indicate that the REFERER header should not be required, then we should also add that comment to the pertinent code (lib/canonical/launchpad/webapp/publication.py LaunchpadBrowserPublication.maybeBlockOffsiteFormPost).

Marc Tardif (cr3) on 2010-04-01
Changed in launchpad:
assignee: nobody → Abel Deuring (adeuring)
Changed in checkbox:
assignee: nobody → Marc Tardif (cr3)
status: New → In Progress
affects: launchpad → launchpad-foundations
Gary Poster (gary) on 2010-04-06
Changed in launchpad-foundations:
status: New → Invalid
Changed in malone:
assignee: nobody → Abel Deuring (adeuring)
Abel Deuring (adeuring) wrote :

another option would be to allow HWDB submissions via the webservice API. That's quite well tested and documented, and the fact that submissions to the HWDB are present only possible via a regular web form is mostly due to the fact that the HWDB is a bit older than the webservice API and that I never got around to implement an API method that hanldes HWDB submissions...

Gary Poster (gary) wrote :

My suggestion in comment #1 was not clear. As of this writing, the following is the pertinent comment in lib/canonical/launchpad/webapp/publication.py LaunchpadBrowserPublication.maybeBlockOffsiteFormPost:

            # We only want to check for the referrer header if we are
            # in the middle of a request initiated by a web browser. A
            # request to the web service (which is necessarily
            # OAuth-signed) or a request that does not implement
            # IBrowserRequest (such as an XML-RPC request) can do
            # without a Referer.
            #
            # XXX gary 2010-03-09 bug=535122,538097
            # The one-off exceptions are necessary because existing
            # non-browser applications make requests to these URLs
            # without providing a Referer. Apport makes POST requests
            # to +storeblob without providing a Referer (bug 538097),
            # and launchpadlib used to make POST requests to
            # +request-token and +access-token without providing a
            # Referer.
            #
            # We'll have to keep an application's one-off exception
            # until the application has been changed to send a
            # Referer, and until we have no legacy versions of that
            # application to support. For instance, we can't get rid
            # of the apport exception until after Lucid's end-of-life
            # date. We should be able to get rid of the launchpadlib
            # exception after Karmic's end-of-life date.

That comment should be expanded to explain why /+hwdb/+submit is an exception, in the same way that it explains +storeblob, +request-token, and +access-token. If the exception is temporary, I'd like it to be associated with an XXX and a bug. Contrariwise, if the exception is permanent, I'd like to have a mention of this in the comment.

Thanks

Gary

Deryck Hodge (deryck) on 2010-04-06
Changed in malone:
status: New → In Progress
importance: Undecided → High
Marc Tardif (cr3) on 2010-04-06
Changed in checkbox:
status: In Progress → Fix Committed
Abel Deuring (adeuring) wrote :

r10699

Changed in malone:
status: In Progress → Fix Committed
Marc Tardif (cr3) on 2010-06-10
Changed in checkbox (Ubuntu):
milestone: none → lucid-updates
status: New → In Progress
milestone: lucid-updates → none
Changed in checkbox (Ubuntu Lucid):
status: New → In Progress
Marc Tardif (cr3) on 2010-06-10
Changed in checkbox (Ubuntu Lucid):
milestone: none → lucid-updates
Changed in checkbox:
status: Fix Committed → Fix Released
Changed in checkbox (Ubuntu):
status: In Progress → Fix Released
Marc Tardif (cr3) on 2010-06-10
description: updated
Marc Tardif (cr3) on 2010-06-10
Changed in malone:
status: Fix Committed → Fix Released
Daniel Manrique (roadmr) wrote :

Hello, the following information should complete this bug as a valid SRU report, to hopefully upload this fix to Lucid.

Solution:

Checkbox revision 782 adds a REFERER header to each request.

TEST CASE:
1- Perform this test case on a freshly-installed 10.04 system.
2- Install netcat (sudo apt-get install netcat)
3- Open a terminal and launch an instance of nc to intercept checkbox traffic and log to a file:
   nc -l 8080 >/tmp/dump.dat
3- Open another terminal and launch checkbox-gtk as follows, to make it submit to the locally-listening nc:
   checkbox-gtk --config='checkbox/plugins/launchpad_exchange/transport_url=http://localhost:8080'
4- Perform a test run (deselect all tests so it takes less time).
5- On the report screen, where it asks for an e-mail address to submit to launchpad, put any e-mail address (a@b.com works).
6- Checkbox will seemingly stall in "Exchanging information with server..."
7- Press "Enter" on the terminal where nc is running so checkbox terminates the sending process. nc will also exit. Checkbox can be closed at this point.
8- Open the dump.dat file and confirm (at the very beginning) that there's no Referer: header in the HTTP request.

Regression potential:
Unless Launchpad decides to outright start rejecting submissions that DO contain a referrer, regression potential on this one is basically none.

Ara Pulido (ara) wrote :

This bug is awaiting verification that the checkbox version in lucid-proposed solves the problem. Please test checkbox and update this bug with the results. If the problem is solved, change the tag 'verification-needed' to 'verification-done'.

See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you!

tags: added: verification-needed
Chris Halse Rogers (raof) wrote :

ubuntu-sru approved and reviewed. Please accept into lucid-proposed.

Hello Abel, or anyone else affected,

Accepted checkbox into lucid-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

Changed in checkbox (Ubuntu Lucid):
status: In Progress → Fix Committed
Chad A. Davis (chadadavis) wrote :

Verified fix from checkbox 0.9.2 on Lucid

tags: added: verification-done
removed: verification-needed
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package checkbox - 0.9.2

---------------
checkbox (0.9.2) lucid-proposed; urgency=low

  New upstream release (LP: #567568):
  * Added referer when sending submissions to Launchpad (LP: #550973)
  * Added suggests to checkbox package in debian/control file (LP: #352740)
  * Fixed udev_resource script to be more resilient (LP: #556824)
  * Fixed cdimage_resource script to read casper.log (LP: #558728)
  * Fixed reporting all resources found for a job (LP: #560948)
  * Fixed stalling when using kdesudo to start backend (LP: #557443)
  * Fixed starting the appropriate default browser on UNR (LP: #563050)
  * Fixed opening the report with the gconf preferred browser (LP: #562580)
  * Fixed suspend_test to use relative time for wakealarm (LP: #349768)
  * Fixed backend not getting terminated upon closing (LP: #553328)
 -- Daniel Manrique <email address hidden> Wed, 22 Jun 2011 14:18:08 -0400

Changed in checkbox (Ubuntu Lucid):
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