20after4 has uploaded a new change for review.
https://gerrit.wikimedia.org/r/289236
Change subject: WIP: keyholder key cleanup
......................................................................
WIP: keyholder key cleanup
Bug: T132747
Change-Id: Id298a3e0f12e31afd7ea83970e192330ffbd4243
---
M hieradata/common/scap/server.yaml
M hieradata/labs/deployment-prep/common.yaml
M modules/eventlogging/manifests/deployment/target.pp
M modules/keyholder/files/keyholder
M modules/keyholder/files/ssh-agent-proxy
M modules/keyholder/manifests/agent.pp
D modules/keyholder/manifests/private_key.pp
D modules/phabricator/manifests/deployment/target.pp
M modules/phabricator/manifests/init.pp
M modules/role/manifests/deployment/mediawiki.pp
M modules/role/manifests/snapshot/deployment.pp
M modules/scap/manifests/target.pp
M modules/service/manifests/node.pp
M modules/zotero/manifests/init.pp
14 files changed, 103 insertions(+), 157 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/operations/puppet
refs/changes/36/289236/1
diff --git a/hieradata/common/scap/server.yaml
b/hieradata/common/scap/server.yaml
index 4568ed2..cf17508 100644
--- a/hieradata/common/scap/server.yaml
+++ b/hieradata/common/scap/server.yaml
@@ -12,24 +12,18 @@
phabricator:
trusted_group: deploy-phabricator
- key_fingerprint: 39:b3:2c:a7:b2:80:65:ff:0c:97:e1:22:88:6c:59:10
- key_secret: phabricator/phab_deploy_private_key
eventlogging:
trusted_group: eventlogging-admins
- key_fingerprint: b6:4e:1a:1b:4b:70:ef:91:31:cd:a3:18:9a:ca:41:44
deploy-service:
trusted_group:
- deploy-service
- aqs-admins
- key_fingerprint: 6d:54:92:8b:39:10:f5:9b:84:40:36:ef:3c:9a:6d:d8
- key_file: servicedeploy_rsa
dumpsdeploy:
trusted_group: ops
- key_fingerprint: 86:c9:17:ab:b7:00:79:b5:8a:c5:b5:ee:29:24:c9:2f
-
+ key_name: dumps
# scap::source declarations. These are created
# by the scap::server class. Each source listed here
@@ -61,4 +55,3 @@
# cxserver is the ContentTranslation server.
cxserver/deploy:
repository: cxserver/deploy
-
diff --git a/hieradata/labs/deployment-prep/common.yaml
b/hieradata/labs/deployment-prep/common.yaml
index 6a1ae68..f5c4d3d 100644
--- a/hieradata/labs/deployment-prep/common.yaml
+++ b/hieradata/labs/deployment-prep/common.yaml
@@ -205,18 +205,13 @@
phabricator:
trusted_group: project-%{::labsproject}
- key_fingerprint: 39:b3:2c:a7:b2:80:65:ff:0c:97:e1:22:88:6c:59:10
- key_secret: phabricator/phab_deploy_private_key
eventlogging:
trusted_group: project-%{::labsproject}
- key_fingerprint: 02:9b:99:e2:f0:16:70:a3:d2:5a:e6:02:a3:73:0e:b0
deploy-service:
trusted_group: deploy-service
- key_fingerprint: 6d:54:92:8b:39:10:f5:9b:84:40:36:ef:3c:9a:6d:d8
- key_file: servicedeploy_rsa
-
+ key_name: servicedeploy
# deployment-prep scap::source declarations. These are created
# by the scap::server class. Each source listed here
diff --git a/modules/eventlogging/manifests/deployment/target.pp
b/modules/eventlogging/manifests/deployment/target.pp
index 07a46ea..6ecd1ae 100644
--- a/modules/eventlogging/manifests/deployment/target.pp
+++ b/modules/eventlogging/manifests/deployment/target.pp
@@ -41,11 +41,10 @@
include eventlogging::dependencies
scap::target { "eventlogging/${title}":
- package_name => $package_name,
- deploy_user => 'eventlogging',
- public_key_source =>
"puppet:///modules/eventlogging/deployment/eventlogging_rsa.pub.${::realm}",
- service_name => $service_name,
- sudo_rules => $sudo_rules,
- manage_user => false,
+ package_name => $package_name,
+ deploy_user => 'eventlogging',
+ service_name => $service_name,
+ sudo_rules => $sudo_rules,
+ manage_user => false,
}
}
diff --git a/modules/keyholder/files/keyholder
b/modules/keyholder/files/keyholder
index 942bbc1..77c39a1 100755
--- a/modules/keyholder/files/keyholder
+++ b/modules/keyholder/files/keyholder
@@ -25,8 +25,15 @@
exit 1
}
+#is_private_key() {
+# [ -f "$1" ] && /usr/bin/openssl rsa -in "$1" -text -noout | \
+# /bin/grep -o "[0-9]* bit" 1>/dev/null 2>&1
+#}
is_valid_private_key() {
- # Check that $1 is a passphrase-protected private key file.
+ # on deployment-prep we allow unencrypted keys:
+ [ -f "$1" ] && [ -f "/etc/wmflabs-project" ] && return 0;
+
+ # on production, check that $1 is a password-protected rsa private key file.
[ -f "$1" ] && /usr/bin/openssl rsa -in "$1" -check -pubout -passin pass:
2>&1 | \
/bin/grep -q "bad password"
}
@@ -51,10 +58,12 @@
;;
arm)
requires_root
+ KEYS=""
for key in /etc/keyholder.d/*; do
is_valid_private_key "$key" || ( echo "$key is not an acceptable key.
Does it have a passphrase?"; continue )
- $0 add "$key"
+ KEYS="$KEYS $key"
done
+ $0 add $KEYS
;;
disarm)
requires_root
diff --git a/modules/keyholder/files/ssh-agent-proxy
b/modules/keyholder/files/ssh-agent-proxy
index 134a8eb..5bd110c 100644
--- a/modules/keyholder/files/ssh-agent-proxy
+++ b/modules/keyholder/files/ssh-agent-proxy
@@ -88,15 +88,30 @@
return string, offset + struct.calcsize(fmt)
+def get_key_fingerprints():
+ """Look up the key fingerprints for all keys held by keyholder"""
+ keymap = {}
+ lines = subprocess.check_output(['/usr/local/sbin/keyholder', 'status'])
+ lines = lines.splitlines()[2:]
+ for line in lines:
+ line = line.split(' ')
+ fingerprint = line[2]
+ path = line[3].split('/')
+ keyfile = path[-1]
+ keymap[keyfile] = fingerprint
+ return keymap
+
def get_key_perms(path):
"""Recursively walk `path`, loading YAML configuration files."""
key_perms = {}
+ fingerprints = get_key_fingerprints()
for fname in glob.glob(os.path.join(path, '*.y*ml')):
with open(fname) as yml:
for group, keys in yaml.safe_load(yml).items():
for key in keys:
- key = key.replace(':', '')
- key_perms.setdefault(key, set()).add(group)
+ fingerprint = fingerprints[key]
+ fingerprint.replace(':', '')
+ key_perms.setdefault(fingerprint, set()).add(group)
return key_perms
diff --git a/modules/keyholder/manifests/agent.pp
b/modules/keyholder/manifests/agent.pp
index 8cdc9de..7733d0a 100644
--- a/modules/keyholder/manifests/agent.pp
+++ b/modules/keyholder/manifests/agent.pp
@@ -7,59 +7,49 @@
# [*name*]
# Used for service names, socket names, and default key name
#
-# [*key_file*]
-# The name of the key file stored in puppet private
-# Should exist prior to running a defined resource
-#
# [*trusted_group*]
# The name or GID of the trusted user group with which the agent
# should be shared. It is the caller's responsibility to ensure
# the group exists. An array of group identifiers can also be provided
# to allow access by multiple groups.
#
-# [*key_fingerprint*]
-# Fingerprint of the public half of the private keyfile specified
-# by $key_file
+# [*ensure*]
+# If 'present', config will be enabled; if 'absent', disabled.
+# The default is 'present'.
#
# === Examples
#
# keyholder::agent { 'mwdeploy':
-# key_file => 'mwdeploy_key_rsa',
# trusted_group => ['wikidev', 'mwdeploy'],
-# key_fingerprint => '00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00'
-# require => Group['wikidev'],
# }
#
define keyholder::agent(
$trusted_group,
- $key_fingerprint,
- $key_file = "${name}_rsa",
- $key_content = undef,
- $key_secret = undef,
+ $ensure = 'present',
+ $key_name = $title
) {
+ validate_ensure($ensure)
+
require ::keyholder
require ::keyholder::monitoring
- file { "/etc/keyholder-auth.d/${name}.yml":
- content => inline_template("---\n<% [*@trusted_group].each do |g|
%><%= g %>: ['<%= @key_fingerprint %>']\n<% end %>"),
+ $key_name_safe = regsubst($key_name, '\W', '_', 'G')
+ $key_content = secret("keyholder/${key_name_safe}")
+
+
+ file { "/etc/keyholder.d/${key_name_safe}":
+ ensure => $ensure,
+ content => $key_content,
owner => 'root',
group => 'keyholder',
mode => '0440',
}
- # lint:ignore:puppet_url_without_modules
- if $key_content {
- keyholder::private_key { $key_file:
- content => $key_content,
- }
- } elsif $key_secret {
- keyholder::private_key { $key_file:
- content => secret($key_secret)
- }
- } else {
- keyholder::private_key { $key_file:
- source => "puppet:///private/ssh/tin/${key_file}",
- }
+ file { "/etc/keyholder-auth.d/${key_name_safe}.yml":
+ ensure => $ensure,
+ content => inline_template("---\n<%= [*@trusted_group].map { |g|
\"#{g}: [#{@key_name_safe}]\" }.join(\"\\n\") %>\n"),
+ owner => 'root',
+ group => 'keyholder',
+ mode => '0440',
}
- # lint:endignore
}
diff --git a/modules/keyholder/manifests/private_key.pp
b/modules/keyholder/manifests/private_key.pp
deleted file mode 100644
index 5c06ebf..0000000
--- a/modules/keyholder/manifests/private_key.pp
+++ /dev/null
@@ -1,50 +0,0 @@
-# == Define: keyholder::private_key
-#
-# Provisions a private key file in /etc/keyholder.d.
-#
-# === Parameters
-#
-# [*ensure*]
-# If 'present', config will be enabled; if 'absent', disabled.
-# The default is 'present'.
-#
-# [*content*]
-# If defined, will be used as the content of the key file.
-# Undefined by default. Mutually exclusive with 'source'.
-#
-# [*source*]
-# Path to key file. Undefined by default.
-# Mutually exclusive with 'content'.
-#
-# === Examples
-#
-# keyholder::private_key { 'mwdeploy_rsa':
-# ensure => present,
-# content => secret('ssh/tin/mwdeploy_rsa'),
-# }
-#
-define keyholder::private_key(
- $ensure = present,
- $content = undef,
- $source = undef,
-) {
- validate_ensure($ensure)
-
- if $source == undef and $content == undef {
- fail('you must provide either "source" or "content"')
- }
- if $source != undef and $content != undef {
- fail('"source" and "content" are mutually exclusive')
- }
-
- $title_safe = regsubst($title, '\W', '_', 'G')
-
- file { "/etc/keyholder.d/${title_safe}":
- ensure => $ensure,
- content => $content,
- source => $source,
- owner => 'root',
- group => 'keyholder',
- mode => '0440',
- }
-}
diff --git a/modules/phabricator/manifests/deployment/target.pp
b/modules/phabricator/manifests/deployment/target.pp
deleted file mode 100644
index 0cf7ce4..0000000
--- a/modules/phabricator/manifests/deployment/target.pp
+++ /dev/null
@@ -1,18 +0,0 @@
-# == Class phabricator::deployment::target
-# This sets up sudo rules and the deploy_user for phabricator deployments with
-# scap3.
-
-class phabricator::deployment::target(
- $deploy_user,
- $deploy_key,
- $deploy_target= 'phabricator/deployment',
-) {
- scap::target { $deploy_target:
- deploy_user => $deploy_user,
- public_key_source => $deploy_key,
- sudo_rules => [
- 'ALL=(root) NOPASSWD: /usr/sbin/service phd *',
- 'ALL=(root) NOPASSWD: /usr/sbin/service apache2 *',
- ]
- }
-}
diff --git a/modules/phabricator/manifests/init.pp
b/modules/phabricator/manifests/init.pp
index 0d537fd..7e8f4d1 100644
--- a/modules/phabricator/manifests/init.pp
+++ b/modules/phabricator/manifests/init.pp
@@ -65,7 +65,7 @@
$serveralias = '',
$deploy_user = 'phab-deploy',
$deploy_target = 'phabricator/deployment',
- $deploy_key =
"puppet:///modules/phabricator/phab-deploy-key.${::realm}",
+ $deploy_key = 'phabricator',
) {
$deploy_root = "/srv/deployment/${deploy_target}"
@@ -134,10 +134,13 @@
}
}
- class { '::phabricator::deployment::target':
- deploy_user => $deploy_user,
- deploy_key => $deploy_key,
- deploy_target => $deploy_target,
+ scap::target { $deploy_target:
+ deploy_user => $deploy_user,
+ deploy_key => $deploy_key,
+ sudo_rules => [
+ 'ALL=(root) NOPASSWD: /usr/sbin/service phd *',
+ 'ALL=(root) NOPASSWD: /usr/sbin/service apache2 *',
+ ]
}
file { $phabdir:
diff --git a/modules/role/manifests/deployment/mediawiki.pp
b/modules/role/manifests/deployment/mediawiki.pp
index 18d34bd..f203d1b 100644
--- a/modules/role/manifests/deployment/mediawiki.pp
+++ b/modules/role/manifests/deployment/mediawiki.pp
@@ -3,7 +3,6 @@
class role::deployment::mediawiki(
$keyholder_user = 'mwdeploy',
$keyholder_group = ['wikidev', 'mwdeploy'],
- $key_fingerprint = 'f5:18:a3:44:77:a2:31:23:cb:7b:44:e1:4b:45:27:11',
) {
# All needed classes for deploying mediawiki
@@ -18,7 +17,6 @@
keyholder::agent { $keyholder_user:
trusted_group => $keyholder_group,
- key_fingerprint => $key_fingerprint,
}
# Wikitech credentials file
diff --git a/modules/role/manifests/snapshot/deployment.pp
b/modules/role/manifests/snapshot/deployment.pp
index c504ae0..19391db 100644
--- a/modules/role/manifests/snapshot/deployment.pp
+++ b/modules/role/manifests/snapshot/deployment.pp
@@ -1,7 +1,8 @@
+# Defines snapshot data dump deployment target(s)
class role::snapshot::deployment {
scap::target { 'dumps/dumps':
- deploy_user => 'datasets',
- public_key_source =>
'puppet:///modules/snapshots/deployment/dumpsdeploy_rsa.pub',
- manage_user => false,
+ deploy_user => 'datasets',
+ manage_user => false,
+ key_name => 'dumps',
}
}
diff --git a/modules/scap/manifests/target.pp b/modules/scap/manifests/target.pp
index ff3f67c..7f7f9f6 100644
--- a/modules/scap/manifests/target.pp
+++ b/modules/scap/manifests/target.pp
@@ -10,9 +10,13 @@
# [*deploy_user*]
# user that will be used for deployments
#
-# [*public_key_source*]
-# puppet source argument to pass to ssh::userkey for installing
-# $deploy_user's public ssh key.
+# [*key_name*]
+# The name of a keyholder ssh key used to access this deployment target.
+# This should correspond to a key which is defined in keyholder::agent
+# (which are defined by hiera data in common::scap::server)
+# Warning: If key_name is left undefined, then you must define the correct
+# ssh::userkey for your deploy_user so that scap can connect to the target
+# from the deployment server with a corresponding key in keyholder.
#
# [*service_name*]
# service name that should be allowed to be restarted via sudo by
@@ -34,18 +38,17 @@
#
# scap::target { 'mockbase':
# deploy_user => 'deploy-mockbase',
-# public_key_source => 'puppet://modules/mockbase/deploy-test_rsa.pub'
+# key_name => 'deploy-mockbase',
# }
#
# scap::target { 'eventlogging/eventlogging':
# deploy_user => 'eventlogging',
-# public_key_source =>
"puppet:///modules/eventlogging/deployment/eventlogging_rsa.pub.${::realm}",
# manage_user => false,
# }
#
define scap::target(
$deploy_user,
- $public_key_source,
+ $key_name = undef,
$service_name = undef,
$package_name = $title,
$manage_user = true,
@@ -65,7 +68,7 @@
Group[$deploy_user] -> Scap::Target[$title]
}
- if $manage_user and !defined(User[$deploy_user]) {
+ if $manage_user {
user { $deploy_user:
ensure => present,
shell => '/bin/bash',
@@ -75,6 +78,20 @@
}
} else {
User[$deploy_user] -> Scap::Target[$title]
+ }
+
+ if $key_name and !defined(Ssh::Userkey[$deploy_user]){
+ ssh::userkey { $deploy_user:
+ ensure => 'present',
+ content => secret("keyholder/${key_name}.pub"),
+ }
+ } else {
+ if !defined(Ssh::Userkey[$deploy_user]) {
+ # warning for services which manage their own user but forget to
+ # define the key.
+ info("Scap will not be able to deploy this target until you define
\
+ the ssh::userkey for user '${deploy_user}'")
+ }
}
if $::realm == 'labs' {
@@ -96,12 +113,6 @@
owner => $deploy_user}],
provider => 'scap3',
require => [Package['scap'], User[$deploy_user]],
- }
-
- if !defined(Ssh::Userkey[$deploy_user]) {
- ssh::userkey { $deploy_user:
- source => $public_key_source,
- }
}
# XXX: Temporary work-around for switching services from Trebuchet to Scap3
diff --git a/modules/service/manifests/node.pp
b/modules/service/manifests/node.pp
index 0fa5104..b40a777 100644
--- a/modules/service/manifests/node.pp
+++ b/modules/service/manifests/node.pp
@@ -75,7 +75,8 @@
# crashes. Default: true
#
# [*deployment*]
-# If this value is set to 'scap3' then deploy via scap3, otherwise, use
trebuchet
+# If this value is set to 'scap3' then deploy via scap3, otherwise,
+# use trebuchet
# Default: undef
#
# [*deployment_user*]
@@ -130,11 +131,12 @@
case $deployment {
'scap3': {
if ! defined(Service::Deploy::Trebuchet[$repo]) {
- service::deploy::scap{ $repo:
+ scap::target { $repo:
service_name => $title,
- user => $deployment_user,
- before => Base::Service_unit[$title],
+ key_name => 'servicedeploy',
+ deploy_user => $deployment_user,
manage_user => $deployment_manage_user,
+ before => Base::Service_unit[$title],
}
}
}
diff --git a/modules/zotero/manifests/init.pp b/modules/zotero/manifests/init.pp
index 85537c5..d7541b9 100644
--- a/modules/zotero/manifests/init.pp
+++ b/modules/zotero/manifests/init.pp
@@ -34,17 +34,15 @@
}
scap::target { 'zotero/translators':
- deploy_user => 'deploy-service',
- public_key_source => 'puppet:///modules/service/servicedeploy_rsa.pub',
- service_name => 'zotero',
- before => Service['zotero'],
+ deploy_user => 'deploy-service',
+ service_name => 'zotero',
+ before => Service['zotero'],
}
scap::target { 'zotero/translation-server':
- deploy_user => 'deploy-service',
- public_key_source => 'puppet:///modules/service/servicedeploy_rsa.pub',
- service_name => 'zotero',
- before => Service['zotero'],
+ deploy_user => 'deploy-service',
+ service_name => 'zotero',
+ before => Service['zotero'],
}
#
/srv/deployment/zotero/translation-server/defaults/preferences/defaults.js
--
To view, visit https://gerrit.wikimedia.org/r/289236
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id298a3e0f12e31afd7ea83970e192330ffbd4243
Gerrit-PatchSet: 1
Gerrit-Project: operations/puppet
Gerrit-Branch: production
Gerrit-Owner: 20after4 <mmod...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
