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

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).

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...

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

r10699

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.

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!

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!

Verified fix from checkbox 0.9.2 on Lucid

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