Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:create-mp-refs into launchpad:master

2020-09-11 Thread William Grant 
On 12/9/20 12:38 am, Thiago F. Pappacena wrote:
> wgrant, good point.
> 
> When a user opens a MP#1 targeting RepositoryX's master, for example, 
> RepositoryX itself will have a read-only ref called `refs/merge/1/head`. The 
> idea is that whomever is responsible for RepositoryX will have an easier way 
> to pull locally the changes introduced by MP#1.
> 
> Let's assume a RepositoryX is private. In theory, nothing changes for the 
> user opening the MP#1: the privacy checks and requirements to actually open a 
> new MP targeting RepositoryX are still the same. 
> 
> The only extra security check introduced on RepositoryX will be on Turnip 
> side, to block pushes to `refs/merge/...` namespace: 
> https://code.launchpad.net/~pappacena/turnip/+git/turnip/+merge/390620.
> 
> Do you see any specific privacy problem with this scenario?

The problem arises when the *source* repository is private. Consider,
for example, a security fix MP: a user can only view an MP if they can
see both the source and target branches. But this will let anyone who
can see the target repository examine the code in the MP from a
potentially invisible private branch.

-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/390581
Your team Launchpad code reviewers is requested to review the proposed merge of 
~pappacena/launchpad:create-mp-refs into launchpad:master.

___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~pappacena/turnip:block-readonly-refs into turnip:master

2020-09-11 Thread Thiago F. Pappacena 
Although there is a chain of MPs to make the `refs/merge/xxx` virtual refs 
work, this MP is independent and can be merged to master once it's reviewed.
-- 
https://code.launchpad.net/~pappacena/turnip/+git/turnip/+merge/390620
Your team Launchpad code reviewers is requested to review the proposed merge of 
~pappacena/turnip:block-readonly-refs into turnip:master.

___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:create-mp-refs into launchpad:master

2020-09-11 Thread Thiago F. Pappacena 
wgrant, good point.

When a user opens a MP#1 targeting RepositoryX's master, for example, 
RepositoryX itself will have a read-only ref called `refs/merge/1/head`. The 
idea is that whomever is responsible for RepositoryX will have an easier way to 
pull locally the changes introduced by MP#1.

Let's assume a RepositoryX is private. In theory, nothing changes for the user 
opening the MP#1: the privacy checks and requirements to actually open a new MP 
targeting RepositoryX are still the same. 

The only extra security check introduced on RepositoryX will be on Turnip side, 
to block pushes to `refs/merge/...` namespace: 
https://code.launchpad.net/~pappacena/turnip/+git/turnip/+merge/390620.

Do you see any specific privacy problem with this scenario?
-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/390581
Your team Launchpad code reviewers is requested to review the proposed merge of 
~pappacena/launchpad:create-mp-refs into launchpad:master.

___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~twom/launchpad:loggerhead-default-interface into launchpad:master

2020-09-11 Thread Tom Wardill 
Tom Wardill has proposed merging ~twom/launchpad:loggerhead-default-interface 
into launchpad:master.

Commit message:
Change developer codehosting interface to 0.0.0.0

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~twom/launchpad/+git/launchpad/+merge/390621

We recommend running launchpad in a container now, so it's useful to be able to 
push bazaar branches from outside the container.
Make the default interface be 0.0.0.0 as it's one less step to making it work 
as a developer and one less diff that everyone has to carry.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~twom/launchpad:loggerhead-default-interface into launchpad:master.
diff --git a/configs/development/launchpad-lazr.conf b/configs/development/launchpad-lazr.conf
index b8f2c55..a4956f3 100644
--- a/configs/development/launchpad-lazr.conf
+++ b/configs/development/launchpad-lazr.conf
@@ -42,7 +42,7 @@ internal_branch_by_id_root: http://bazaar-internal.launchpad.test/
 internal_codebrowse_root: http://localhost:8080/
 rewrite_script_log_file: /var/tmp/bazaar.launchpad.test/rewrite.log
 host_key_pair_path: lib/lp/codehosting/sshserver/tests/keys
-port: tcp:5022:interface=127.0.0.88
+port: tcp:5022:interface=0.0.0.0
 bzr_lp_prefix: lp://dev/
 lp_url_hosts: dev
 access_log: /var/tmp/bazaar.launchpad.test/codehosting-access.log
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

[Launchpad-reviewers] [Merge] ~pappacena/turnip:block-readonly-refs into turnip:master

2020-09-11 Thread Thiago F. Pappacena 
Thiago F. Pappacena has proposed merging ~pappacena/turnip:block-readonly-refs 
into turnip:master.

Commit message:
Blocking pushes to read-only ref namespaces, like refs/merge/

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/turnip/+git/turnip/+merge/390620
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~pappacena/turnip:block-readonly-refs into turnip:master.
diff --git a/turnip/pack/hooks/hook.py b/turnip/pack/hooks/hook.py
index 0dba4dd..4e492ea 100755
--- a/turnip/pack/hooks/hook.py
+++ b/turnip/pack/hooks/hook.py
@@ -25,6 +25,12 @@ from turnip.compat.files import fd_buffer
 GIT_OID_HEX_ZERO = '0'*40
 
 
+# Users cannot update references in this list.
+READONLY_REF_NAMESPACES = [
+b'refs/merge/',
+]
+
+
 def check_ancestor(old, new):
 # This is a delete, setting the new ref.
 if new == GIT_OID_HEX_ZERO:
@@ -42,6 +48,8 @@ def is_default_branch(pushed_branch):
 
 
 def determine_permissions_outcome(old, ref, rule_lines):
+if any(ref.startswith(i) for i in READONLY_REF_NAMESPACES):
+return b"%s is in a read-only namespace." % ref
 rule = rule_lines.get(ref, [])
 if old == GIT_OID_HEX_ZERO:
 # We are creating a new ref
@@ -88,6 +96,8 @@ def match_update_rules(rule_lines, ref_line):
 the rule_lines to confirm that the user has permissions for that operation.
 """
 ref, old, new = ref_line
+if any(ref.startswith(i) for i in READONLY_REF_NAMESPACES):
+return [b"%s is in a read-only namespace." % ref]
 
 # If it's a create, the old ref doesn't exist
 if old == GIT_OID_HEX_ZERO:
diff --git a/turnip/pack/tests/test_hooks.py b/turnip/pack/tests/test_hooks.py
index 22c31c7..374097f 100644
--- a/turnip/pack/tests/test_hooks.py
+++ b/turnip/pack/tests/test_hooks.py
@@ -265,6 +265,14 @@ class TestPreReceiveHook(HookTestMixin, TestCase):
 b"You do not have permission to push to refs/heads/verboten.\n")
 
 @defer.inlineCallbacks
+def test_rejected_readonly_namespaces(self):
+# An read-only ref is rejected.
+yield self.assertRejected(
+[(b'refs/merge/123/heads', self.old_sha1, self.new_sha1)],
+{b'refs/heads/master': ['push', 'force_push', 'create']},
+b"refs/merge/123/heads is in a read-only namespace.\n")
+
+@defer.inlineCallbacks
 def test_rejected_multiple(self):
 # A combination of valid and invalid refs is still rejected.
 yield self.assertRejected(
@@ -441,6 +449,14 @@ class TestUpdateHook(TestCase):
 self.assertEqual(
 [b'You do not have permission to force-push to ref.'], output)
 
+def test_read_only_ref(self):
+# User does not have permission to force-push to a read-only namespace
+output = hook.match_update_rules(
+{'ref': ['create']},
+[b'refs/merge/123/head', 'old', 'new'])
+self.assertEqual(
+[b'refs/merge/123/head is in a read-only namespace.'], output)
+
 
 class TestDeterminePermissions(TestCase):
 
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp