Skip to content

Commit 85ec214

Browse files
jhjaggarsjhjaggars
committed
fix(helm,drivers): guard default clientCaSecretName and add env-strip tests
Signed-off-by: Jesse Jaggars <[email protected]>
1 parent aac65a9 commit 85ec214

6 files changed

Lines changed: 58 additions & 2 deletions

File tree

‎Cargo.lock‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/openshell-driver-docker/src/tests.rs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,26 @@ fn build_environment_protects_oci_identity_metadata() {
594594
assert!(!env.iter().any(|entry| entry.ends_with("=9999")));
595595
}
596596

597+
#[test]
598+
fn build_environment_strips_gateway_tls_server_name() {
599+
let mut sandbox = test_sandbox();
600+
let spec = sandbox.spec.as_mut().unwrap();
601+
spec.environment.insert(
602+
openshell_core::sandbox_env::GATEWAY_TLS_SERVER_NAME.to_string(),
603+
"evil.attacker.example.com".to_string(),
604+
);
605+
606+
let env = build_environment(&sandbox, &runtime_config());
607+
608+
assert!(
609+
!env.iter().any(|entry| entry.starts_with(&format!(
610+
"{}=",
611+
openshell_core::sandbox_env::GATEWAY_TLS_SERVER_NAME
612+
))),
613+
"GATEWAY_TLS_SERVER_NAME must be stripped from the supervisor environment"
614+
);
615+
}
616+
597617
#[test]
598618
fn container_creation_uses_inspected_immutable_image() {
599619
let sandbox = test_sandbox();

‎crates/openshell-driver-podman/src/container.rs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1418,6 +1418,25 @@ mod tests {
14181418
);
14191419
}
14201420

1421+
#[test]
1422+
fn build_env_strips_gateway_tls_server_name() {
1423+
let mut sandbox = test_sandbox("test-id", "test-name");
1424+
let spec = sandbox.spec.get_or_insert_default();
1425+
spec.environment.insert(
1426+
openshell_core::sandbox_env::GATEWAY_TLS_SERVER_NAME.to_string(),
1427+
"evil.attacker.example.com".to_string(),
1428+
);
1429+
1430+
let container = build_container_spec(&sandbox, &test_config());
1431+
1432+
assert_eq!(
1433+
container["env"]
1434+
.get(openshell_core::sandbox_env::GATEWAY_TLS_SERVER_NAME),
1435+
None,
1436+
"GATEWAY_TLS_SERVER_NAME must be stripped from the supervisor environment"
1437+
);
1438+
}
1439+
14211440
#[test]
14221441
fn volume_name_uses_id() {
14231442
assert_eq!(

‎deploy/helm/openshell/templates/cert-manager-pki.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ spec:
4343
secretName: {{ .Values.certManager.caSecretName | quote }}
4444
---
4545
{{- $externalServerIssuer := .Values.certManager.serverIssuerRef.name }}
46+
{{- if and (not .Values.certManager.clientCaFromServerTlsSecret) (eq .Values.server.tls.clientCaSecretName "openshell-server-client-ca") }}
47+
{{- fail "certManager.clientCaFromServerTlsSecret is false but server.tls.clientCaSecretName is still the default (openshell-server-client-ca), which nothing creates when cert-manager owns TLS. Set server.tls.clientCaSecretName to the secret containing the client CA (e.g. the value of certManager.caSecretName, which defaults to openshell-ca-tls), or set it to empty to disable mTLS client verification." }}
48+
{{- end }}
4649
{{- if and $externalServerIssuer .Values.certManager.clientCaFromServerTlsSecret }}
4750
{{- fail "certManager.serverIssuerRef.name is set but certManager.clientCaFromServerTlsSecret is still true \u2014 the server cert Secret's ca.crt comes from the external issuer, not the CA that signs the client (mTLS) certificate. Set certManager.clientCaFromServerTlsSecret=false and set server.tls.clientCaSecretName to the secret containing the client CA (e.g. the value of certManager.caSecretName, which defaults to openshell-ca-tls)." }}
4851
{{- end }}

‎deploy/helm/openshell/tests/cert_manager_pki_test.yaml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ tests:
6060
certManager.serverIssuerRef.name: letsencrypt-prod
6161
certManager.serverIssuerRef.kind: ClusterIssuer
6262
certManager.clientCaFromServerTlsSecret: false
63+
server.tls.clientCaSecretName: openshell-ca-tls
6364
certManager.serverDnsNames:
6465
- openshell.example.com
6566
asserts:
@@ -83,6 +84,7 @@ tests:
8384
certManager.serverIssuerRef.name: letsencrypt-prod
8485
certManager.serverIssuerRef.kind: ClusterIssuer
8586
certManager.clientCaFromServerTlsSecret: false
87+
server.tls.clientCaSecretName: openshell-ca-tls
8688
certManager.serverDnsNames:
8789
- openshell.example.com
8890
asserts:
@@ -126,18 +128,29 @@ tests:
126128
certManager.enabled: true
127129
certManager.serverIssuerRef.name: letsencrypt-prod
128130
certManager.clientCaFromServerTlsSecret: false
131+
server.tls.clientCaSecretName: openshell-ca-tls
129132
# serverDnsNames is left at the default which contains "openshell.openshell.svc", "localhost", etc.
130133
asserts:
131134
- failedTemplate:
132135
errorMessage: "certManager.serverIssuerRef.name is set (external issuer) but certManager.serverDnsNames contains \"openshell.openshell.svc\" \u2014 external CAs (e.g. ACME / Let's Encrypt) reject internal-only names per CA/Browser Forum baseline requirements. Override certManager.serverDnsNames with your externally-resolvable hostname(s)."
133136

137+
- it: fails when clientCaFromServerTlsSecret is false but clientCaSecretName is the default
138+
template: templates/cert-manager-pki.yaml
139+
set:
140+
certManager.enabled: true
141+
certManager.clientCaFromServerTlsSecret: false
142+
asserts:
143+
- failedTemplate:
144+
errorMessage: "certManager.clientCaFromServerTlsSecret is false but server.tls.clientCaSecretName is still the default (openshell-server-client-ca), which nothing creates when cert-manager owns TLS. Set server.tls.clientCaSecretName to the secret containing the client CA (e.g. the value of certManager.caSecretName, which defaults to openshell-ca-tls), or set it to empty to disable mTLS client verification."
145+
134146
- it: client Certificate issuerRef is unaffected by serverIssuerRef
135147
template: templates/cert-manager-pki.yaml
136148
set:
137149
certManager.enabled: true
138150
certManager.serverIssuerRef.name: letsencrypt-prod
139151
certManager.serverIssuerRef.kind: ClusterIssuer
140152
certManager.clientCaFromServerTlsSecret: false
153+
server.tls.clientCaSecretName: openshell-ca-tls
141154
certManager.serverDnsNames:
142155
- openshell.example.com
143156
asserts:

‎deploy/helm/openshell/tests/statefulset_client_ca_test.yaml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,14 @@ tests:
5353
certManager.enabled: true
5454
certManager.clientCaFromServerTlsSecret: false
5555
pkiInitJob.enabled: true
56+
server.tls.clientCaSecretName: openshell-ca-tls
5657
asserts:
5758
- equal:
5859
path: spec.template.spec.volumes[3].name
5960
value: tls-client-ca
6061
- equal:
6162
path: spec.template.spec.volumes[3].secret.secretName
62-
value: openshell-server-client-ca
63+
value: openshell-ca-tls
6364
- notExists:
6465
path: spec.template.spec.volumes[3].secret.items
6566

0 commit comments

Comments
 (0)