Skip to content

Commit 0b69c07

Browse files
committed
Add support for configurable SSH algorithms in diego-sshd
Add configuration schema and implementation to allow operators to configure SSH algorithms (ciphers, host key algorithms, key exchanges, and MACs) for diego-sshd running in app containers. The diego.sshd configuration accepts comma-separated strings for each algorithm type. When configured, these values are passed as command-line flags to diego-sshd (using -allowedCiphers, -allowedHostKeyAlgorithms, -allowedKeyExchanges, -allowedMACs). Empty strings result in no flags being passed, allowing diego-sshd to use its defaults. Both the sshd section and individual algorithm options are optional in all schemas (api, worker, clock, and deployment_updater), allowing operators to configure only the algorithms they need to restrict.
1 parent 6d1bb45 commit 0b69c07

File tree

6 files changed

+141
-11
lines changed

6 files changed

+141
-11
lines changed

lib/cloud_controller/config_schemas/api_schema.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,13 @@ class ApiSchema < VCAP::Config
160160
insecure_docker_registry_list: [String],
161161
docker_staging_stack: String,
162162
optional(:temporary_oci_buildpack_mode) => enum('oci-phase-1', NilClass),
163-
enable_declarative_asset_downloads: bool
163+
enable_declarative_asset_downloads: bool,
164+
optional(:sshd) => {
165+
optional(:allowed_ciphers) => String,
166+
optional(:allowed_host_key_algorithms) => String,
167+
optional(:allowed_key_exchanges) => String,
168+
optional(:allowed_macs) => String
169+
}
164170
},
165171

166172
app_log_revision: bool,

lib/cloud_controller/config_schemas/clock_schema.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,13 @@ class ClockSchema < VCAP::Config
9696
use_privileged_containers_for_running: bool,
9797
use_privileged_containers_for_staging: bool,
9898
optional(:temporary_oci_buildpack_mode) => enum('oci-phase-1', NilClass),
99-
enable_declarative_asset_downloads: bool
99+
enable_declarative_asset_downloads: bool,
100+
optional(:sshd) => {
101+
optional(:allowed_ciphers) => String,
102+
optional(:allowed_host_key_algorithms) => String,
103+
optional(:allowed_key_exchanges) => String,
104+
optional(:allowed_macs) => String
105+
}
100106
},
101107

102108
app_log_revision: bool,

lib/cloud_controller/config_schemas/deployment_updater_schema.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,13 @@ class DeploymentUpdaterSchema < VCAP::Config
107107
use_privileged_containers_for_running: bool,
108108
use_privileged_containers_for_staging: bool,
109109
optional(:temporary_oci_buildpack_mode) => enum('oci-phase-1', NilClass),
110-
enable_declarative_asset_downloads: bool
110+
enable_declarative_asset_downloads: bool,
111+
optional(:sshd) => {
112+
optional(:allowed_ciphers) => String,
113+
optional(:allowed_host_key_algorithms) => String,
114+
optional(:allowed_key_exchanges) => String,
115+
optional(:allowed_macs) => String
116+
}
111117
},
112118

113119
app_log_revision: bool,

lib/cloud_controller/config_schemas/worker_schema.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,13 @@ class WorkerSchema < VCAP::Config
8888
use_privileged_containers_for_running: bool,
8989
use_privileged_containers_for_staging: bool,
9090
optional(:temporary_oci_buildpack_mode) => enum('oci-phase-1', NilClass),
91-
enable_declarative_asset_downloads: bool
91+
enable_declarative_asset_downloads: bool,
92+
optional(:sshd) => {
93+
optional(:allowed_ciphers) => String,
94+
optional(:allowed_host_key_algorithms) => String,
95+
optional(:allowed_key_exchanges) => String,
96+
optional(:allowed_macs) => String
97+
}
9298
},
9399

94100
app_log_revision: bool,

lib/cloud_controller/diego/main_lrp_action_builder.rb

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -100,21 +100,34 @@ def generate_sidecar_actions(user)
100100
end
101101

102102
def generate_ssh_action(user, environment_variables)
103+
sshd_args = [
104+
"-address=#{sprintf('0.0.0.0:%<port>d', port: DEFAULT_SSH_PORT)}",
105+
"-hostKey=#{ssh_key.private_key}",
106+
"-authorizedKey=#{ssh_key.authorized_key}",
107+
'-inheritDaemonEnv',
108+
'-logLevel=fatal'
109+
]
110+
add_allowed_ssh_algorithms(sshd_args)
111+
103112
action(::Diego::Bbs::Models::RunAction.new(
104113
user: user,
105114
path: '/tmp/lifecycle/diego-sshd',
106-
args: [
107-
"-address=#{sprintf('0.0.0.0:%<port>d', port: DEFAULT_SSH_PORT)}",
108-
"-hostKey=#{ssh_key.private_key}",
109-
"-authorizedKey=#{ssh_key.authorized_key}",
110-
'-inheritDaemonEnv',
111-
'-logLevel=fatal'
112-
],
115+
args: sshd_args,
113116
env: environment_variables,
114117
resource_limits: ::Diego::Bbs::Models::ResourceLimits.new(nofile: process.file_descriptors),
115118
log_source: SSHD_LOG_SOURCE
116119
))
117120
end
121+
122+
def add_allowed_ssh_algorithms(args)
123+
sshd_config = VCAP::CloudController::Config.config.get(:diego, :sshd)
124+
return if sshd_config.nil?
125+
126+
args << "-allowedCiphers=#{sshd_config[:allowed_ciphers]}" if sshd_config[:allowed_ciphers].present?
127+
args << "-allowedHostKeyAlgorithms=#{sshd_config[:allowed_host_key_algorithms]}" if sshd_config[:allowed_host_key_algorithms].present?
128+
args << "-allowedKeyExchanges=#{sshd_config[:allowed_key_exchanges]}" if sshd_config[:allowed_key_exchanges].present?
129+
args << "-allowedMACs=#{sshd_config[:allowed_macs]}" if sshd_config[:allowed_macs].present?
130+
end
118131
end
119132
end
120133
end

spec/unit/lib/cloud_controller/diego/main_lrp_action_builder_spec.rb

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,99 @@ module Diego
247247
)
248248
)
249249
end
250+
251+
context 'when SSH algorithm configuration is provided' do
252+
before do
253+
TestConfig.override(
254+
diego: {
255+
sshd: {
256+
allowed_ciphers: 'cipher-1,cipher-2',
257+
allowed_host_key_algorithms: 'hostkeyalg-1,hostkeyalg-2',
258+
allowed_key_exchanges: 'kex-1,kex-2',
259+
allowed_macs: 'mac-1,mac-2'
260+
}
261+
}
262+
)
263+
end
264+
265+
it 'includes SSH algorithm flags in the diego-sshd arguments' do
266+
actions = MainLRPActionBuilder.build(process, lrp_builder, ssh_key).codependent_action.actions.map(&:run_action)
267+
ssh_action = actions.find { |action| action.path == '/tmp/lifecycle/diego-sshd' }
268+
269+
expect(ssh_action.args).to include('-allowedCiphers=cipher-1,cipher-2')
270+
expect(ssh_action.args).to include('-allowedHostKeyAlgorithms=hostkeyalg-1,hostkeyalg-2')
271+
expect(ssh_action.args).to include('-allowedKeyExchanges=kex-1,kex-2')
272+
expect(ssh_action.args).to include('-allowedMACs=mac-1,mac-2')
273+
end
274+
end
275+
276+
context 'when SSH algorithm configuration has empty strings' do
277+
before do
278+
TestConfig.override(
279+
diego: {
280+
sshd: {
281+
allowed_ciphers: '',
282+
allowed_host_key_algorithms: '',
283+
allowed_key_exchanges: '',
284+
allowed_macs: ''
285+
}
286+
}
287+
)
288+
end
289+
290+
it 'does not include SSH algorithm flags in the arguments' do
291+
actions = MainLRPActionBuilder.build(process, lrp_builder, ssh_key).codependent_action.actions.map(&:run_action)
292+
ssh_action = actions.find { |action| action.path == '/tmp/lifecycle/diego-sshd' }
293+
294+
expect(ssh_action.args).not_to include(match(/-allowedCiphers=/))
295+
expect(ssh_action.args).not_to include(match(/-allowedHostKeyAlgorithms=/))
296+
expect(ssh_action.args).not_to include(match(/-allowedKeyExchanges=/))
297+
expect(ssh_action.args).not_to include(match(/-allowedMACs=/))
298+
end
299+
end
300+
301+
context 'when SSH algorithm configuration is partially provided' do
302+
before do
303+
TestConfig.override(
304+
diego: {
305+
sshd: {
306+
allowed_ciphers: 'cipher-1',
307+
allowed_host_key_algorithms: '',
308+
}
309+
}
310+
)
311+
end
312+
313+
it 'includes only the configured algorithm flags' do
314+
actions = MainLRPActionBuilder.build(process, lrp_builder, ssh_key).codependent_action.actions.map(&:run_action)
315+
ssh_action = actions.find { |action| action.path == '/tmp/lifecycle/diego-sshd' }
316+
317+
expect(ssh_action.args).to include('-allowedCiphers=cipher-1')
318+
expect(ssh_action.args).not_to include(match(/-allowedHostKeyAlgorithms=/))
319+
expect(ssh_action.args).not_to include(match(/-allowedKeyExchanges=/))
320+
expect(ssh_action.args).not_to include(match(/-allowedMACs=/))
321+
end
322+
end
323+
324+
context 'when diego sshd configuration is missing' do
325+
before do
326+
TestConfig.override(diego: {})
327+
end
328+
329+
it 'does not raise an error and excludes algorithm flags' do
330+
expect do
331+
MainLRPActionBuilder.build(process, lrp_builder, ssh_key)
332+
end.not_to raise_error
333+
334+
actions = MainLRPActionBuilder.build(process, lrp_builder, ssh_key).codependent_action.actions.map(&:run_action)
335+
ssh_action = actions.find { |action| action.path == '/tmp/lifecycle/diego-sshd' }
336+
337+
expect(ssh_action.args).not_to include(match(/-allowedCiphers=/))
338+
expect(ssh_action.args).not_to include(match(/-allowedHostKeyAlgorithms=/))
339+
expect(ssh_action.args).not_to include(match(/-allowedKeyExchanges=/))
340+
expect(ssh_action.args).not_to include(match(/-allowedMACs=/))
341+
end
342+
end
250343
end
251344

252345
describe 'VCAP_PLATFORM_OPTIONS' do

0 commit comments

Comments
 (0)