launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-822640 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #822640 in Launchpad itself: "Privilege violation in publish-ftpmaster"
  https://bugs.launchpad.net/launchpad/+bug/822640

For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-822640/+merge/70730

= Summary =

The new publish-ftpmaster script runs process-accepted and publish-distro in-process now, so needs the same privileges.  Some of them are fairly obscure, hidden away in massive complexity and deep code paths so testing for them in advance was not a realistic proposition.


== Proposed fix ==

Four tables needed extra access privileges for the archivepublisher user.


== Pre-implementation notes ==

William was kind enough to run through the database roles and figure out what the differences amounted to.  This was not easy because of structural differences in how their security configs were arranged.

Managing this kind of thing has gotten far out of hand.


== Tests ==

It should be possible to reconstruct test scenarios where these privileges are used, but to do so would probably incur significant costs in both development and in test run times.  I don't believe such relatively small configuration changes are worth it; the worry is not that this might regress, but that we may have forgotten about another obscure code path that reads from another table.


== Demo and Q/A ==

Observe the publish-ftpmaster script runnin in production, at three minutes past every hour.  Dogfood testing was not enough to turn up the missing SELECT privilege on TranslationGroup that we observed.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  database/schema/security.cfg
-- 
https://code.launchpad.net/~jtv/launchpad/bug-822640/+merge/70730
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-822640 into lp:launchpad.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2011-08-02 09:57:41 +0000
+++ database/schema/security.cfg	2011-08-08 12:48:34 +0000
@@ -857,6 +857,7 @@
 public.cve                              = SELECT, INSERT
 public.distributionjob                  = SELECT, INSERT, DELETE
 public.distributionsourcepackage        = SELECT, INSERT, UPDATE
+public.distroseriesdifference           = SELECT
 public.distroseriesparent               = SELECT, INSERT, UPDATE
 public.flatpackagesetinclusion          = SELECT, INSERT, UPDATE, DELETE
 public.gpgkey                           = SELECT, INSERT, UPDATE
@@ -885,7 +886,10 @@
 public.questionjob                      = SELECT, INSERT
 public.questionsubscription             = SELECT
 public.sourcepackagepublishinghistory   = SELECT, INSERT, UPDATE, DELETE
+public.sourcepackagerecipebuild         = SELECT
+public.sourcepackagerecipebuildjob      = SELECT, INSERT, UPDATE
 public.structuralsubscription           = SELECT
+public.translationgroup                 = SELECT
 public.validpersoncache                 = SELECT
 public.validpersonorteamcache           = SELECT
 type=user