Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes to Enable SSH for Repository hosting #4

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion .kitchen.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,16 @@ suites:
- recipe[phabricator::default]
attributes: {
"phabricator": {
"domain": "phabricator.example.com"
"domain": "phabricator.example.com",
"vcs_ssh": {
"hosting_enabled": true
}
},
"authorization": {
"sudo": {
"users": ["vagrant"],
"passwordless": true
}
}
}
- name: arcanist
Expand Down
36 changes: 34 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@ This cookbook has been tested on Ubuntu 12.04 and 14.04.
- `nginx ~> 2.7`
- `mysql ~> 6.0`
- `database ~> 3.1`
- `openssl ~> 4.4`

Attributes
----------
See `attributes/default.rb`.
See `attributes/default.rb` and `attributes/repo_hosting.rb`.

Usage
-----
Expand All @@ -38,11 +39,42 @@ Just include `phabricator` in your node's `run_list`:
MySQL Installation
------------------

If `node['phabricator]['mysql_host']` is set to `localhost`, the cookbook will
If `node['phabricator']['mysql_host']` is set to `localhost`, the cookbook will
install and configure the MySQL server appropriately. Otherwise, it will
configure Phabricator to connect to an external database. In the latter case, a
MySQL database user will //not// be managed by this cookbook.

Repository Hosting
------------------

If running Ubuntu 14.04, this recipe can also setup Phabricator for hosting repositories
over SSH. (It requires functionality in OpenSSH 6.2 or newer, which is not available for
Ubuntu 12.04).

If `node['phabricator']['vcs_ssh']['hosting_enabled']` is set to `true`, the cookbook will
set up the server for serving up VCS over SSH.
Defaults to `false` to preserve behaviour with previous versions.

It will setup a seperate daemon called `ssh-vcs`, which is configured to listen on a
different port to the standard system sshd, this is controlled via
`default['phabricator']['vcs_ssh']['port']` It defaults to `617`.

This is a highly locked down SSH process following recommendations from upstream:
[https://secure.phabricator.com/book/phabricator/article/diffusion_hosting/]

The user used for VCS is controlled via `default['phabricator']['vcs_ssh']['user']` and defaults
to `git`.

When interacting with Diffusion over ssh the following is recommended in your `~/.ssh/config`:

```
Host phabricator.example.com
User git
Port 617
Compression yes
```


Contributing
------------
1. Fork the repository on Github
Expand Down
28 changes: 28 additions & 0 deletions attributes/vcs_ssh_hosting.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Default to previous behaviour of not setting up SSH for VCS Hosting.
# NB this setting has no effect on Ubuntu 12.04 as its version of OpenSSH
# is too old to work.
# See:
# [https://secure.phabricator.com/book/phabricator/article/diffusion_hosting/#configuring-ssh]
default['phabricator']['vcs_ssh']['hosting_enabled'] = false

# Port that VCS SSH will listen on (seperate daemon)
default['phabricator']['vcs_ssh']['port'] = '617'

# Username for SSH for VCS
default['phabricator']['vcs_ssh']['user'] = 'git'

# Homedir for VCS user. NB this is not the repo path. See
# `node['phabricator']['repository_path']` in the default attributes
# file for this.`
home_dir_prefix = '/home'
home_dir = File.join(home_dir_prefix, node['phabricator']['vcs_ssh']['user'])
default['phabricator']['vcs_ssh']['home_dir'] = home_dir

append_paths = []
append_paths << File.join(node['phabricator']['path'], '/phabricator/support/bin')
append_paths << '/bin'
append_paths << '/usr/bin'
append_paths << '/usr/local/bin'
append_paths << '/usr/lib/git-core'

default['phabricator']['config']['environment.append-paths'] = %('#{append_paths}')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious to why this attribute is conofigured in vcs_ssh_hosting.rb and not default.rb? It looks like it belongs in the latter file.

2 changes: 2 additions & 0 deletions metadata.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,5 @@
depends 'nginx', '~> 2.7'
depends 'mysql', '~> 6.0'
depends 'database', '~> 3.1'
depends 'sudo', '~> 2.7.2'
depends 'openssl', '~> 4.4.0'
13 changes: 11 additions & 2 deletions recipes/default.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
action :create
user node['phabricator']['user']
group node['phabricator']['group']
mode "0750"
mode "0755"
end

%w{ phabricator libphutil arcanist }.each do |repo|
Expand All @@ -83,7 +83,7 @@
action :create
user node['phabricator']['user']
group node['phabricator']['group']
mode "0750"
mode "0755"
end

# Set up file storage
Expand Down Expand Up @@ -187,3 +187,12 @@
node.set['phabricator']['storage_upgrade_done'] = true
end
end

# Setup ssh repo hosting if we want it!
# Only applicable on 14.04 or newer, due to need for AuthorizedKeysCommand which came in
# OpenSSH 6.2.
if node['platform_version'] >= '14.04'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting ['phabricator']['vcs_ssh']['hosting_enabled'] to true on a system other than (Ubuntu) 14.04 will fail silently. It will be better to fail fast, throwing an exception in the vcs_ssh_hosting recipe if the system is unsupported.

Furthermore, what do you think about actually not including the vcs_ssh_hosting recipe here, but instead including it explicitly in the run list if wanted? Seems easier to grasp what is actually happening.

if node['phabricator']['vcs_ssh']['hosting_enabled']
include_recipe 'phabricator::vcs_ssh_hosting'
end
end
110 changes: 110 additions & 0 deletions recipes/vcs_ssh_hosting.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
#
# Cookbook Name:: phabricator
# Recipe:: vcs_ssh_hosting
#
# Copyright 2014, MET Norway
#
# Authors: Kim Tore Jensen <[email protected]>,
# Andrew Mulholland <[email protected]>
#
# Sets up phabricator for hosting repositories over SSH

# To prevent the `node['phabricator']['vcs_ssh']['user']` (i.e. git ) user from being locked, a
# password needs to be provided with account creation. Using OpenSSLCookbook's RandomPassword
# function to generate a secure password.
unless node['phabricator']['vcs_ssh']['password']
Chef::Recipe.send(:include, OpenSSLCookbook::RandomPassword)
require 'digest/sha2'
password = Digest::SHA512.hexdigest(random_password(length: 50))
node.set['phabricator']['vcs_ssh']['password'] = password
end

user node['phabricator']['vcs_ssh']['user'] do
comment 'VCS User'
home node['phabricator']['vcs_ssh']['home_dir']
shell '/bin/sh'
system true
# This password is never used!
password node['phabricator']['vcs_ssh']['password']
action [:create, :unlock]
end

directory node['phabricator']['vcs_ssh']['home_dir'] do
action :create
owner node['phabricator']['vcs_ssh']['user']
mode '0755'
end

directory '/etc/ssh-phabricator' do
action :create
end

# Install Upstart init script for the SSH Daemon
template '/etc/init/ssh-vcs.conf' do
source 'upstart/ssh-vcs.conf.erb'
owner 'root'
group 'root'
mode '0755'
notifies :restart, 'service[ssh-vcs]'
end

service 'ssh-vcs' do
provider Chef::Provider::Service::Upstart
supports status: true, restart: true, reload: true
action [:enable, :start]
end

template '/etc/ssh-phabricator/sshd_config' do
source 'vcs_ssh/sshd_config.erb'
owner 'root'
group 'root'
mode '0755'
notifies :restart, 'service[ssh-vcs]'
end

directory '/usr/libexec' do
owner 'root'
group 'root'
mode '0755'
end

template '/usr/libexec/ssh-phabricator-hook' do
source 'vcs_ssh/ssh-phabricator-hook.erb'
owner 'root'
group 'root'
mode '0755'
end

# enable /etc/sudoers.d directory to enable the sudoer provider to work
node.override['authorization']['sudo']['include_sudoers_d'] = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why override and not default? Recipes have precedence over attribute files.


# We use template here, because right now upstream `sudo` cookbook doesn't support
# setting setenv, which is needed because phab runs `sudo -E`.
# Have filed https://github.com/chef-cookbooks/sudo/pull/72 for this.
sudo 'vcs_user' do
template 'vcs_ssh/phab_sudo.erb'
variables commands: ['/usr/bin/git-upload-pack',
'/usr/bin/git-receive-pack',
'/usr/bin/hg',
'/usr/bin/svnserve'],
nopasswd: true,
setenv: true,
sudoer: node['phabricator']['vcs_ssh']['user'],
runas: node['phabricator']['user']
end

sudo 'www-data' do
template 'vcs_ssh/phab_sudo.erb'
variables commands: ['/usr/bin/git-http-backend'],
nopasswd: true,
setenv: true,
sudoer: 'www-data',
runas: node['phabricator']['user']
end

# Install Git, Mercurial, SVN
%w(git subversion mercurial).each do |pkg|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, but is it not the responsibility of the main recipe? IMO, in any case, it is a entirely different issue and should be committed separately.

package pkg do
action :install
end
end
27 changes: 27 additions & 0 deletions templates/default/upstart/ssh-vcs.conf.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# ssh for VCS
#
# This OpenSSH server provides ssh access to VCS for Phabricator

description "SSH-VCS server"

start on runlevel [2345]
stop on runlevel [!2345]

respawn
respawn limit 10 5
umask 022

env SSH_SIGSTOP=1
expect stop

# 'sshd -D' leaks stderr and confuses things in conjunction with 'console log'
console none

pre-start script
test -x /usr/sbin/sshd || { stop; exit 0; }

end script

# if you used to set SSHD_OPTS in /etc/default/ssh, you can change the
# 'exec' line here instead
exec /usr/sbin/sshd -D -f /etc/ssh-phabricator/sshd_config
6 changes: 6 additions & 0 deletions templates/default/vcs_ssh/phab_sudo.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# This file is managed by Chef.
# Do NOT modify this file directly.

<% @commands.each do |command| -%>
<%= @sudoer %> ALL=(<%= @runas %>) <%= 'NOPASSWD:' if @nopasswd %><%= 'SETENV:' if @setenv %><%= command %>
<% end -%>
13 changes: 13 additions & 0 deletions templates/default/vcs_ssh/ssh-phabricator-hook.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/bin/sh
#
VCSUSER="<%= node['phabricator']['vcs_ssh']['user'] %>"
#
# Path to Phabricator directory.
ROOT="<%= File.join(node['phabricator']['path'],'phabricator') %>"

if [ "$1" != "$VCSUSER" ];
then
exit 1
fi

exec "$ROOT/bin/ssh-auth" $@
12 changes: 12 additions & 0 deletions templates/default/vcs_ssh/sshd_config.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
AuthorizedKeysCommand /usr/libexec/ssh-phabricator-hook
AuthorizedKeysCommandUser <%= node['phabricator']['vcs_ssh']['user'] %>

Port <%= node['phabricator']['vcs_ssh']['port'] %>
Protocol 2
PermitRootLogin no
AllowAgentForwarding no
AllowTcpForwarding no
PrintMotd no
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to have X11Forwarding no, even though it is the default, to be explicit about it.

PrintLastLog no
PasswordAuthentication no
AuthorizedKeysFile none
18 changes: 18 additions & 0 deletions test/integration/default/bats/default.bats
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,21 @@
[ $status -eq 0 ]
}
}

@test "SSHD VCS is running" {
if [ $(lsb_release -r | awk '{print $2}') == "12.04" ]; then
skip "SSH hosting not supported on 12.04"
fi

run service ssh-vcs status
[ $status -eq 0 ]
}

@test "SSHd for VCS is listening" {
if [ $(lsb_release -r | awk '{print $2}') == "12.04" ]; then
skip "SSH hosting not supported on 12.04"
fi

run sh -c "netstat -nlp |grep :617"
[ $status -eq 0 ]
}