Skip to content

Commit f2938ba

Browse files
author
epriestley
committed
Generalize SSH passthru for repository hosting
Summary: Ref T2230. In Git, we can determine if a command is read-only or read/write from the command itself, but this isn't the case in Mercurial or SVN. For Mercurial and SVN, we need to proxy the protocol that's coming over the wire, look at each request from the client, and then check if it's a read or a write. To support this, provide a more flexible version of `passthruIO`. The way this will work is: - The SSH IO channel is wrapped in a `ProtocolChannel` which can parse the the incoming stream into message objects. - The `willWriteCallback` will look at those messages and determine if they're reads or writes. - If they're writes, it will check for write permission. - If we're good to go, the message object is converted back into a byte stream and handed to the underlying command. Test Plan: Executed `git clone`, `git clone --depth 3`, `git push` (against no-write repo, got error), `git push` (against valid repo). Reviewers: btrahan Reviewed By: btrahan CC: hach-que, asherkin, aran Maniphest Tasks: T2230 Differential Revision: https://secure.phabricator.com/D7551
1 parent ae5fbe0 commit f2938ba

6 files changed

+221
-72
lines changed

src/__phutil_library_map__.php

+2
Original file line numberDiff line numberDiff line change
@@ -1746,6 +1746,7 @@
17461746
'PhabricatorRepositoryVCSPassword' => 'applications/repository/storage/PhabricatorRepositoryVCSPassword.php',
17471747
'PhabricatorS3FileStorageEngine' => 'applications/files/engine/PhabricatorS3FileStorageEngine.php',
17481748
'PhabricatorSQLPatchList' => 'infrastructure/storage/patch/PhabricatorSQLPatchList.php',
1749+
'PhabricatorSSHPassthruCommand' => 'infrastructure/ssh/PhabricatorSSHPassthruCommand.php',
17491750
'PhabricatorSSHWorkflow' => 'infrastructure/ssh/PhabricatorSSHWorkflow.php',
17501751
'PhabricatorSavedQuery' => 'applications/search/storage/PhabricatorSavedQuery.php',
17511752
'PhabricatorSavedQueryQuery' => 'applications/search/query/PhabricatorSavedQueryQuery.php',
@@ -4175,6 +4176,7 @@
41754176
'PhabricatorRepositoryTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
41764177
'PhabricatorRepositoryVCSPassword' => 'PhabricatorRepositoryDAO',
41774178
'PhabricatorS3FileStorageEngine' => 'PhabricatorFileStorageEngine',
4179+
'PhabricatorSSHPassthruCommand' => 'Phobject',
41784180
'PhabricatorSSHWorkflow' => 'PhutilArgumentWorkflow',
41794181
'PhabricatorSavedQuery' =>
41804182
array(

src/applications/diffusion/ssh/DiffusionSSHGitReceivePackWorkflow.php

+8-5
Original file line numberDiff line numberDiff line change
@@ -14,21 +14,24 @@ public function didConstruct() {
1414
));
1515
}
1616

17-
public function isReadOnly() {
18-
return false;
19-
}
20-
2117
public function getRequestPath() {
2218
$args = $this->getArgs();
2319
return head($args->getArg('dir'));
2420
}
2521

2622
protected function executeRepositoryOperations(
2723
PhabricatorRepository $repository) {
24+
25+
// This is a write, and must have write access.
26+
$this->requireWriteAccess();
27+
2828
$future = new ExecFuture(
2929
'git-receive-pack %s',
3030
$repository->getLocalPath());
31-
$err = $this->passthruIO($future);
31+
$err = $this->newPassthruCommand()
32+
->setIOChannel($this->getIOChannel())
33+
->setCommandChannelFromExecFuture($future)
34+
->execute();
3235

3336
if (!$err) {
3437
$repository->writeStatusMessage(

src/applications/diffusion/ssh/DiffusionSSHGitUploadPackWorkflow.php

+4-5
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,6 @@ public function didConstruct() {
1414
));
1515
}
1616

17-
public function isReadOnly() {
18-
return true;
19-
}
20-
2117
public function getRequestPath() {
2218
$args = $this->getArgs();
2319
return head($args->getArg('dir'));
@@ -28,7 +24,10 @@ protected function executeRepositoryOperations(
2824

2925
$future = new ExecFuture('git-upload-pack %s', $repository->getLocalPath());
3026

31-
return $this->passthruIO($future);
27+
return $this->newPassthruCommand()
28+
->setIOChannel($this->getIOChannel())
29+
->setCommandChannelFromExecFuture($future)
30+
->execute();
3231
}
3332

3433
}

src/applications/diffusion/ssh/DiffusionSSHWorkflow.php

+43-16
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,17 @@
33
abstract class DiffusionSSHWorkflow extends PhabricatorSSHWorkflow {
44

55
private $args;
6+
private $repository;
7+
private $hasWriteAccess;
8+
9+
public function getRepository() {
10+
return $this->repository;
11+
}
612

713
public function getArgs() {
814
return $this->args;
915
}
1016

11-
abstract protected function isReadOnly();
1217
abstract protected function getRequestPath();
1318
abstract protected function executeRepositoryOperations(
1419
PhabricatorRepository $repository);
@@ -23,6 +28,7 @@ final public function execute(PhutilArgumentParser $args) {
2328

2429
try {
2530
$repository = $this->loadRepository();
31+
$this->repository = $repository;
2632
return $this->executeRepositoryOperations($repository);
2733
} catch (Exception $ex) {
2834
$this->writeError(get_class($ex).': '.$ex->getMessage());
@@ -56,34 +62,55 @@ private function loadRepository() {
5662
pht('No repository "%s" exists!', $callsign));
5763
}
5864

59-
$is_push = !$this->isReadOnly();
65+
switch ($repository->getServeOverSSH()) {
66+
case PhabricatorRepository::SERVE_READONLY:
67+
case PhabricatorRepository::SERVE_READWRITE:
68+
// If we have read or read/write access, proceed for now. We will
69+
// check write access when the user actually issues a write command.
70+
break;
71+
case PhabricatorRepository::SERVE_OFF:
72+
default:
73+
throw new Exception(
74+
pht('This repository is not available over SSH.'));
75+
}
76+
77+
return $repository;
78+
}
79+
80+
protected function requireWriteAccess() {
81+
if ($this->hasWriteAccess === true) {
82+
return;
83+
}
84+
85+
$repository = $this->getRepository();
86+
$viewer = $this->getUser();
6087

6188
switch ($repository->getServeOverSSH()) {
6289
case PhabricatorRepository::SERVE_READONLY:
63-
if ($is_push) {
64-
throw new Exception(
65-
pht('This repository is read-only over SSH.'));
66-
}
90+
throw new Exception(
91+
pht('This repository is read-only over SSH.'));
6792
break;
6893
case PhabricatorRepository::SERVE_READWRITE:
69-
if ($is_push) {
70-
$can_push = PhabricatorPolicyFilter::hasCapability(
71-
$viewer,
72-
$repository,
73-
DiffusionCapabilityPush::CAPABILITY);
74-
if (!$can_push) {
75-
throw new Exception(
76-
pht('You do not have permission to push to this repository.'));
77-
}
94+
$can_push = PhabricatorPolicyFilter::hasCapability(
95+
$viewer,
96+
$repository,
97+
DiffusionCapabilityPush::CAPABILITY);
98+
if (!$can_push) {
99+
throw new Exception(
100+
pht('You do not have permission to push to this repository.'));
78101
}
79102
break;
80103
case PhabricatorRepository::SERVE_OFF:
81104
default:
105+
// This shouldn't be reachable because we don't get this far if the
106+
// repository isn't enabled, but kick them out anyway.
82107
throw new Exception(
83108
pht('This repository is not available over SSH.'));
84109
}
85110

86-
return $repository;
111+
$this->hasWriteAccess = true;
112+
return $this->hasWriteAccess;
87113
}
88114

115+
89116
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
<?php
2+
3+
/**
4+
* Proxy an IO channel to an underlying command, with optional callbacks. This
5+
* is a mostly a more general version of @{class:PhutilExecPassthru}. This
6+
* class is used to proxy Git, SVN and Mercurial traffic to the commands which
7+
* can actually serve it.
8+
*
9+
* Largely, this just reads an IO channel (like stdin from SSH) and writes
10+
* the results into a command channel (like a command's stdin). Then it reads
11+
* the command channel (like the command's stdout) and writes it into the IO
12+
* channel (like stdout from SSH):
13+
*
14+
* IO Channel Command Channel
15+
* stdin -> stdin
16+
* stdout <- stdout
17+
* stderr <- stderr
18+
*
19+
* You can provide **read and write callbacks** which are invoked as data
20+
* is passed through this class. They allow you to inspect and modify traffic.
21+
*
22+
* IO Channel Passthru Command Channel
23+
* stdout -> willWrite -> stdin
24+
* stdin <- willRead <- stdout
25+
* stderr <- (identity) <- stderr
26+
*
27+
* Primarily, this means:
28+
*
29+
* - the **IO Channel** can be a @{class:PhutilProtocolChannel} if the
30+
* **write callback** can convert protocol messages into strings; and
31+
* - the **write callback** can inspect and reject requests over the channel,
32+
* e.g. to enforce policies.
33+
*
34+
* In practice, this is used when serving repositories to check each command
35+
* issued over SSH and determine if it is a read command or a write command.
36+
* Writes can then be checked for appropriate permissions.
37+
*/
38+
final class PhabricatorSSHPassthruCommand extends Phobject {
39+
40+
private $commandChannel;
41+
private $ioChannel;
42+
private $errorChannel;
43+
private $execFuture;
44+
private $willWriteCallback;
45+
private $willReadCallback;
46+
47+
public function setCommandChannelFromExecFuture(ExecFuture $exec_future) {
48+
$exec_channel = new PhutilExecChannel($exec_future);
49+
$exec_channel->setStderrHandler(array($this, 'writeErrorIOCallback'));
50+
51+
$this->execFuture = $exec_future;
52+
$this->commandChannel = $exec_channel;
53+
54+
return $this;
55+
}
56+
57+
public function setIOChannel(PhutilChannel $io_channel) {
58+
$this->ioChannel = $io_channel;
59+
return $this;
60+
}
61+
62+
public function setErrorChannel(PhutilChannel $error_channel) {
63+
$this->errorChannel = $error_channel;
64+
return $this;
65+
}
66+
67+
public function setWillReadCallback($will_read_callback) {
68+
$this->willReadCallback = $will_read_callback;
69+
return $this;
70+
}
71+
72+
public function setWillWriteCallback($will_write_callback) {
73+
$this->willWriteCallback = $will_write_callback;
74+
return $this;
75+
}
76+
77+
public function writeErrorIOCallback(PhutilChannel $channel, $data) {
78+
$this->errorChannel->write($data);
79+
}
80+
81+
public function execute() {
82+
$command_channel = $this->commandChannel;
83+
$io_channel = $this->ioChannel;
84+
$error_channel = $this->errorChannel;
85+
86+
if (!$command_channel) {
87+
throw new Exception("Set a command channel before calling execute()!");
88+
}
89+
90+
if (!$io_channel) {
91+
throw new Exception("Set an IO channel before calling execute()!");
92+
}
93+
94+
if (!$error_channel) {
95+
throw new Exception("Set an error channel before calling execute()!");
96+
}
97+
98+
$channels = array($command_channel, $io_channel, $error_channel);
99+
100+
while (true) {
101+
PhutilChannel::waitForAny($channels);
102+
103+
$io_channel->update();
104+
$command_channel->update();
105+
$error_channel->update();
106+
107+
$done = !$command_channel->isOpen();
108+
109+
$in_message = $io_channel->read();
110+
$in_message = $this->willWriteData($in_message);
111+
if ($in_message !== null) {
112+
$command_channel->write($in_message);
113+
}
114+
115+
$out_message = $command_channel->read();
116+
$out_message = $this->willReadData($out_message);
117+
if ($out_message !== null) {
118+
$io_channel->write($out_message);
119+
}
120+
121+
// If we have nothing left on stdin, close stdin on the subprocess.
122+
if (!$io_channel->isOpenForReading()) {
123+
// TODO: This should probably be part of PhutilExecChannel?
124+
$this->execFuture->write('');
125+
}
126+
127+
if ($done) {
128+
break;
129+
}
130+
}
131+
132+
list($err) = $this->execFuture->resolve();
133+
134+
return $err;
135+
}
136+
137+
public function willWriteData($message) {
138+
if ($this->willWriteCallback) {
139+
return call_user_func($this->willWriteCallback, $this, $message);
140+
} else {
141+
if (strlen($message)) {
142+
return $message;
143+
} else {
144+
return null;
145+
}
146+
}
147+
}
148+
149+
public function willReadData($message) {
150+
if ($this->willReadCallback) {
151+
return call_user_func($this->willReadCallback, $this, $message);
152+
} else {
153+
if (strlen($message)) {
154+
return $message;
155+
} else {
156+
return null;
157+
}
158+
}
159+
}
160+
161+
}

src/infrastructure/ssh/PhabricatorSSHWorkflow.php

+3-46
Original file line numberDiff line numberDiff line change
@@ -37,50 +37,6 @@ public function getIOChannel() {
3737
return $this->iochannel;
3838
}
3939

40-
public function passthruIO(ExecFuture $future) {
41-
$exec_channel = new PhutilExecChannel($future);
42-
$exec_channel->setStderrHandler(array($this, 'writeErrorIOCallback'));
43-
44-
$io_channel = $this->getIOChannel();
45-
$error_channel = $this->getErrorChannel();
46-
47-
$channels = array($exec_channel, $io_channel, $error_channel);
48-
49-
while (true) {
50-
PhutilChannel::waitForAny($channels);
51-
52-
$io_channel->update();
53-
$exec_channel->update();
54-
$error_channel->update();
55-
56-
$done = !$exec_channel->isOpen();
57-
58-
$data = $io_channel->read();
59-
if (strlen($data)) {
60-
$exec_channel->write($data);
61-
}
62-
63-
$data = $exec_channel->read();
64-
if (strlen($data)) {
65-
$io_channel->write($data);
66-
}
67-
68-
// If we have nothing left on stdin, close stdin on the subprocess.
69-
if (!$io_channel->isOpenForReading()) {
70-
// TODO: This should probably be part of PhutilExecChannel?
71-
$future->write('');
72-
}
73-
74-
if ($done) {
75-
break;
76-
}
77-
}
78-
79-
list($err) = $future->resolve();
80-
81-
return $err;
82-
}
83-
8440
public function readAllInput() {
8541
$channel = $this->getIOChannel();
8642
while ($channel->update()) {
@@ -102,8 +58,9 @@ public function writeErrorIO($data) {
10258
return $this;
10359
}
10460

105-
public function writeErrorIOCallback(PhutilChannel $channel, $data) {
106-
$this->writeErrorIO($data);
61+
protected function newPassthruCommand() {
62+
return id(new PhabricatorSSHPassthruCommand())
63+
->setErrorChannel($this->getErrorChannel());
10764
}
10865

10966
}

0 commit comments

Comments
 (0)