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

update test_silo_certificates #6716

Open
ahl opened this issue Sep 27, 2024 · 0 comments
Open

update test_silo_certificates #6716

ahl opened this issue Sep 27, 2024 · 0 comments
Labels
good first issue Issues that are good for learning the codebase

Comments

@ahl
Copy link
Contributor

ahl commented Sep 27, 2024

Theses tests are intended to validate what happens when we use the wrong certificate to connect to a silo

// For good measure, to make sure we got the certificate stuff right, let's
// try to use the wrong certificate to reach each endpoint and confirm that
// we get a TLS error.
let silo2_client_wrong_cert = silo2.oxide_client(
silo3.reqwest_client(),
resolver.clone(),
AuthnMode::SiloUser(silo2_user),
nexus_port,
);
let error =
silo2_client_wrong_cert.current_user_view().send().await.expect_err(
"unexpectedly connected with wrong certificate trusted",
);
if let oxide_client::Error::CommunicationError(error) = error {
assert!(error.is_connect());
assert!(error.chain().to_string().contains("self-signed certificate"));
} else {
panic!(
"unexpected error connecting with wrong certificate: {:#}",
error
);
}
let silo3_client_wrong_cert = silo3.oxide_client(
silo2.reqwest_client(),
resolver.clone(),
AuthnMode::SiloUser(silo2_user),
nexus_port,
);
let error =
silo3_client_wrong_cert.current_user_view().send().await.expect_err(
"unexpectedly connected with wrong certificate trusted",
);
if let oxide_client::Error::CommunicationError(error) = error {
assert!(error.is_connect());
assert!(error.chain().to_string().contains("self-signed certificate"));
} else {
panic!(
"unexpected error connecting with wrong certificate: {:#}",
error
);

As @jmpesp notes:

I'm not sure this check is right: it looks like this part of the test is meant to check that a client can't connect using the wrong certificate, and this check is asserting it won't trust a self-signed certificate. That's not quite the same thing. I think we need either add_root_certificate or danger_accept_invalid_certs in order to test that the client accepts the self-signed cert but fails due to the incorrect certificate being used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues that are good for learning the codebase
Projects
None yet
Development

No branches or pull requests

1 participant