Jabrique commented on code in PR #13373: URL: https://github.com/apache/trafficserver/pull/13373#discussion_r3556005488
########## tests/gold_tests/tls/ssl_multicert_partial_reload.test.py: ########## @@ -0,0 +1,481 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +Test.Summary = ''' +Test ssl_multicert partial_reload: with the knob on, a single bad cert should not +block the reload of all other valid certs. +''' + +# --------------------------------------------------------------------------- +# Scenario A - default behavior (partial_reload=0): one bad cert aborts the +# entire reload; old config stays fully in place. +# --------------------------------------------------------------------------- + +sni_valid = 'valid.example.com' + +ts_strict = Test.MakeATSProcess("ts_strict", enable_tls=True, disable_log_checks=True) +server_strict = Test.MakeOriginServer("server_strict") +request_header = {"headers": f"GET / HTTP/1.1\r\nHost: {sni_valid}\r\n\r\n", "timestamp": "1469733493.993", "body": ""} +response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""} +server_strict.addResponse("sessionlog.json", request_header, response_header) + +ts_strict.Disk.records_config.update( + { + 'proxy.config.ssl.server.cert.path': f'{ts_strict.Variables.SSLDir}', + 'proxy.config.ssl.server.private_key.path': f'{ts_strict.Variables.SSLDir}', + 'proxy.config.ssl.server.multicert.exit_on_load_fail': 0, + # partial_reload intentionally left at default (0) + }) + +ts_strict.addDefaultSSLFiles() +ts_strict.Disk.remap_config.AddLine(f'map / http://127.0.0.1:{server_strict.Variables.Port}') + +ts_strict.Disk.ssl_multicert_yaml.AddLines( + """ +ssl_multicert: + - dest_ip: "*" + ssl_cert_name: server.pem + ssl_key_name: server.key +""".split("\n")) + +tr_strict_1 = Test.AddTestRun("Strict: initial request succeeds") +tr_strict_1.Processes.Default.StartBefore(Test.Processes.ts_strict) +tr_strict_1.Processes.Default.StartBefore(server_strict) +tr_strict_1.StillRunningAfter = ts_strict +tr_strict_1.MakeCurlCommand( + f"-q -s -v -k --resolve '{sni_valid}:{ts_strict.Variables.ssl_port}:127.0.0.1' " + f"https://{sni_valid}:{ts_strict.Variables.ssl_port}";, + ts=ts_strict) +tr_strict_1.Processes.Default.ReturnCode = 0 + +# Overwrite config with one bad entry (missing file) + keep the good default +tr_strict_update = Test.AddTestRun("Strict: inject bad cert into config") +strict_yaml_path = ts_strict.Disk.ssl_multicert_yaml.AbsPath +tr_strict_update.Disk.File(strict_yaml_path, id="strict_yaml", typename="ats:config") +tr_strict_update.Disk.strict_yaml.AddLines( + """ +ssl_multicert: + - ssl_cert_name: does_not_exist.pem + ssl_key_name: does_not_exist.key + - dest_ip: "*" + ssl_cert_name: server.pem + ssl_key_name: server.key +""".split("\n")) +tr_strict_update.StillRunningAfter = ts_strict +tr_strict_update.Processes.Default.Command = 'echo Updated strict config' +tr_strict_update.Processes.Default.Env = ts_strict.Env +tr_strict_update.Processes.Default.ReturnCode = 0 + +# Reload should fail - strict mode rejects the whole config +tr_strict_reload = Test.AddConfigReload( + ts_strict, expect="fail", expect_tasks=["ssl_multicert.yaml"], description="Strict: reload expected to fail") +tr_strict_reload.StillRunningAfter = server_strict + +# Old cert still served because reload was rolled back +tr_strict_2 = Test.AddTestRun("Strict: old cert still served after failed reload") +tr_strict_2.StillRunningAfter = ts_strict +tr_strict_2.MakeCurlCommand( + f"-q -s -v -k --resolve '{sni_valid}:{ts_strict.Variables.ssl_port}:127.0.0.1' " + f"https://{sni_valid}:{ts_strict.Variables.ssl_port}";, + ts=ts_strict) +tr_strict_2.Processes.Default.ReturnCode = 0 +tr_strict_2.Processes.Default.Streams.stderr = Testers.IncludesExpression( + "CN=example.com", "Old default cert should still be served") + +# The counter must be incremented even in strict mode so operators can alert on failures +# regardless of which reload policy is active. +tr_strict_metric = Test.AddTestRun("Strict: ssl_multicert_load_failures counter incremented") +tr_strict_metric.StillRunningAfter = ts_strict +tr_strict_metric.Processes.Default.Command = ( + f"{ts_strict.Variables.BINDIR}/traffic_ctl metric get" + f" proxy.process.ssl.ssl_multicert_load_failures") +tr_strict_metric.Processes.Default.Env = ts_strict.Env +tr_strict_metric.Processes.Default.ReturnCode = 0 +tr_strict_metric.Processes.Default.Streams.stdout = Testers.IncludesExpression( + "proxy.process.ssl.ssl_multicert_load_failures [1-9]", + "Failure counter must be incremented even when strict mode rolls back the config") + +# --------------------------------------------------------------------------- +# Scenario B - partial_reload=1: a bad cert is skipped, valid certs are +# applied. The domain whose cert failed falls back to the default context. +# --------------------------------------------------------------------------- + +ts_partial = Test.MakeATSProcess("ts_partial", enable_tls=True, disable_log_checks=True) +server_partial = Test.MakeOriginServer("server_partial") +server_partial.addResponse("sessionlog.json", request_header, response_header) + +ts_partial.Disk.records_config.update( + { + 'proxy.config.ssl.server.cert.path': f'{ts_partial.Variables.SSLDir}', + 'proxy.config.ssl.server.private_key.path': f'{ts_partial.Variables.SSLDir}', + 'proxy.config.ssl.server.multicert.exit_on_load_fail': 0, + 'proxy.config.ssl.server.multicert.partial_reload': 1, + }) + +ts_partial.addDefaultSSLFiles() +ts_partial.Disk.remap_config.AddLine(f'map / http://127.0.0.1:{server_partial.Variables.Port}') + +ts_partial.Disk.ssl_multicert_yaml.AddLines( + """ +ssl_multicert: + - dest_ip: "*" + ssl_cert_name: server.pem + ssl_key_name: server.key +""".split("\n")) + +tr_partial_1 = Test.AddTestRun("Partial: initial request succeeds") +tr_partial_1.Processes.Default.StartBefore(Test.Processes.ts_partial) +tr_partial_1.Processes.Default.StartBefore(server_partial) +tr_partial_1.StillRunningAfter = ts_partial +tr_partial_1.MakeCurlCommand( + f"-q -s -v -k --resolve '{sni_valid}:{ts_partial.Variables.ssl_port}:127.0.0.1' " + f"https://{sni_valid}:{ts_partial.Variables.ssl_port}";, + ts=ts_partial) +tr_partial_1.Processes.Default.ReturnCode = 0 + +# Inject a mix: one bad entry, one NEWLY-GENERATED good default (different CN) +# The new default cert has CN=reloaded.example.com so that after the partial reload +# we can verify the new config was committed rather than the old one retained. +tr_partial_update = Test.AddTestRun("Partial: inject bad cert alongside new valid default") +partial_yaml_path = ts_partial.Disk.ssl_multicert_yaml.AbsPath +ssl_dir = ts_partial.Variables.SSLDir + +# Generate a new self-signed cert with a distinct CN in the same SSL dir +new_cert = f"{ssl_dir}/newdefault.pem" +new_key = f"{ssl_dir}/newdefault.key" +gen_cmd = ( + f"openssl req -x509 -newkey rsa:2048 " + f"-keyout {new_key} -out {new_cert} " + f"-days 365 -nodes -subj '/CN=reloaded.example.com' " + f"2>/dev/null") + +tr_partial_update.Disk.File(partial_yaml_path, id="partial_yaml", typename="ats:config") +tr_partial_update.Disk.partial_yaml.AddLines( + f""" +ssl_multicert: + - ssl_cert_name: does_not_exist.pem + ssl_key_name: does_not_exist.key + - dest_ip: "*" + ssl_cert_name: newdefault.pem + ssl_key_name: newdefault.key +""".split("\n")) +tr_partial_update.StillRunningAfter = ts_partial +tr_partial_update.Processes.Default.Command = f"{gen_cmd} && echo Updated partial config" +tr_partial_update.Processes.Default.Env = ts_partial.Env +tr_partial_update.Processes.Default.ReturnCode = 0 + +# Reload should succeed despite the bad entry (partial_reload=1) +tr_partial_reload = Test.AddConfigReload( + ts_partial, + expect="success", + expect_tasks=["ssl_multicert.yaml"], + description="Partial: reload expected to succeed despite bad cert") +tr_partial_reload.StillRunningAfter = server_partial + +# Default cert is still reachable and the CN proves the NEW config was committed, +# not the old server.pem (CN=example.com) that was kept. +tr_partial_2 = Test.AddTestRun("Partial: new cert committed and served after partial reload") +tr_partial_2.StillRunningAfter = ts_partial +tr_partial_2.MakeCurlCommand( + f"-q -s -v -k --resolve '{sni_valid}:{ts_partial.Variables.ssl_port}:127.0.0.1' " + f"https://{sni_valid}:{ts_partial.Variables.ssl_port}";, + ts=ts_partial) +tr_partial_2.Processes.Default.ReturnCode = 0 +tr_partial_2.Processes.Default.Streams.stderr = Testers.IncludesExpression( + "CN=reloaded.example.com", "New default cert (not old server.pem) must be served") + +# The bad cert must still produce an ERROR log entry (partial does not silence errors) +ts_partial.Disk.diags_log.Content = Testers.IncludesExpression('ERROR', 'bad cert should still produce an error log entry') + Review Comment: Fixed. 'ERROR' is too broad. One note on the suggested fix though "Failed to load certificate 'does_not_exist.pem'" is from errata.note(), which goes to the JSONRPC output, not diags.log. What actually shows up in the log is from SSLError(): `failed to load certificate secret for .../does_not_exist.pem with key path ...` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
