Skip to content

Avoid accessing missing interfaces on other nodes #7392

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.

Sign up for GitHub

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

Closed
eddyashton wants to merge 23 commits into from
Closed

Avoid accessing missing interfaces on other nodes #7392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
ab21fb2
Include node address in snapshot redirect, to avoid load balancer mis…
eddyashton Oct 17, 2025
2894255
Follow redirect chains ourself
eddyashton Oct 17, 2025
27ffac0
Merge branch 'main' of https://github.com/microsoft/CCF into snapshot…
eddyashton Oct 17, 2025
d2402af
Hmmm
eddyashton Oct 17, 2025
f0d62e0
Update src/snapshots/fetch.h
eddyashton Oct 17, 2025
1ff2b29
Add header const, return HeaderMap directly
eddyashton Oct 21, 2025
b99cbde
Requesting past-end shrinks range, rather than returning error
eddyashton Oct 21, 2025
5547d6a
Remove larger than file test case
eddyashton Oct 21, 2025
4649f76
Remove initial HEAD request, do GET directly
eddyashton Oct 21, 2025
e614cf2
Update docs, make max snapshot size configurable
eddyashton Oct 21, 2025
5c2f9b5
Merge branch 'main' of https://github.com/microsoft/CCF into snapshot…
eddyashton Oct 21, 2025
a3d2e50
Check for interface presence safely
eddyashton Oct 21, 2025
b0b6c99
Add test case of contacting non-universal interface
eddyashton Oct 17, 2025
12f5040
Missed helper function
eddyashton Oct 17, 2025
bfa562a
Use helper for /node/primary redirect
eddyashton Oct 21, 2025
6d628f6
Switch helper to JSON return types, so we can use it in other endpoints
eddyashton Oct 21, 2025
8312118
Factor out join redirects to use helper
eddyashton Oct 21, 2025
5c7483e
Typos and templates
eddyashton Oct 21, 2025
c3d6c5f
Merge branch 'main' of https://github.com/microsoft/CCF into no_inter…
eddyashton Oct 22, 2025
f4ba3ca
Merge branch 'main' of https://github.com/microsoft/CCF into no_inter…
eddyashton Oct 22, 2025
a1d9a7d
Yuck
eddyashton Oct 22, 2025
a0de42a
Tweak signatures to remove template
eddyashton Oct 22, 2025
8cb8e27
Format
eddyashton Oct 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
246 changes: 119 additions & 127 deletions src/node/rpc/node_frontend.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,35 +178,36 @@ namespace ccf
NetworkState& network;
ccf::AbstractNodeOperation& node_operation;

std::optional<std::string> get_redirect_address_for_node(
const ccf::endpoints::ReadOnlyEndpointContext& ctx,
using AddressOrError =
std::variant<std::string, ccf::jsonhandler::JsonAdapterResponse>;
AddressOrError get_redirect_address_for_node(
const ccf::endpoints::CommandEndpointContext& ctx,
ccf::kv::ReadOnlyTx& ro_tx,
const ccf::NodeId& target_node)
{
auto nodes = ctx.tx.ro(network.nodes);
auto nodes = ro_tx.ro(network.nodes);

auto node_info = nodes->get(target_node);
if (!node_info.has_value())
{
LOG_FAIL_FMT("Node redirection error: Unknown node {}", target_node);
ctx.rpc_ctx->set_error(
return make_error(
HTTP_STATUS_INTERNAL_SERVER_ERROR,
ccf::errors::InternalError,
fmt::format(
"Cannot find node info to produce redirect response for node {}",
target_node));
return std::nullopt;
}

const auto interface_id =
ctx.rpc_ctx->get_session_context()->interface_id;
if (!interface_id.has_value())
{
LOG_FAIL_FMT("Node redirection error: Non-RPC request");
ctx.rpc_ctx->set_error(
return make_error(
HTTP_STATUS_INTERNAL_SERVER_ERROR,
ccf::errors::InternalError,
"Cannot redirect non-RPC request");
return std::nullopt;
}

const auto& interfaces = node_info->rpc_interfaces;
Expand All @@ -216,15 +217,14 @@ namespace ccf
LOG_FAIL_FMT(
"Node redirection error: Target missing interface {}",
interface_id.value());
ctx.rpc_ctx->set_error(
return make_error(
HTTP_STATUS_INTERNAL_SERVER_ERROR,
ccf::errors::InternalError,
fmt::format(
"Cannot redirect request. Received on RPC interface {}, which is "
"not present on target node {}",
interface_id.value(),
target_node));
return std::nullopt;
}

const auto& interface = interface_it->second;
Expand Down Expand Up @@ -522,6 +522,43 @@ namespace ccf
auto nodes = args.tx.rw(this->network.nodes);
auto service = args.tx.rw(this->network.service);

auto check_for_join_redirect =
[&]() -> std::optional<ccf::jsonhandler::JsonAdapterResponse> {
if (consensus != nullptr && !this->node_operation.can_replicate())
{
auto primary_id = consensus->primary();
if (primary_id.has_value())
{
const AddressOrError address_or_error =
get_redirect_address_for_node(
args, args.tx, primary_id.value());
if (std::holds_alternative<ccf::jsonhandler::JsonAdapterResponse>(
address_or_error))
{
return std::get<ccf::jsonhandler::JsonAdapterResponse>(
address_or_error);
}

const auto address = std::get<std::string>(address_or_error);
args.rpc_ctx->set_response_header(
http::headers::LOCATION,
fmt::format("https://{}/node/join", address));

return make_error(
HTTP_STATUS_PERMANENT_REDIRECT,
ccf::errors::NodeCannotHandleRequest,
"Node is not primary; cannot handle write");
}

return make_error(
HTTP_STATUS_INTERNAL_SERVER_ERROR,
ccf::errors::InternalError,
"Primary unknown");
}

return std::nullopt;
};

auto active_service = service->get();
if (!active_service.has_value())
{
Expand Down Expand Up @@ -563,40 +600,10 @@ namespace ccf
return make_success(rep);
}

if (consensus != nullptr && !this->node_operation.can_replicate())
const auto redirect_response = check_for_join_redirect();
if (redirect_response.has_value())
{
auto primary_id = consensus->primary();
if (primary_id.has_value())
{
auto info = nodes->get(primary_id.value());
if (info)
{
auto& interface_id =
args.rpc_ctx->get_session_context()->interface_id;
if (!interface_id.has_value())
{
return make_error(
HTTP_STATUS_INTERNAL_SERVER_ERROR,
ccf::errors::InternalError,
"Cannot redirect non-RPC request.");
}
const auto& address =
info->rpc_interfaces[interface_id.value()].published_address;
args.rpc_ctx->set_response_header(
http::headers::LOCATION,
fmt::format("https://{}/node/join", address));

return make_error(
HTTP_STATUS_PERMANENT_REDIRECT,
ccf::errors::NodeCannotHandleRequest,
"Node is not primary; cannot handle write");
}
}

return make_error(
HTTP_STATUS_INTERNAL_SERVER_ERROR,
ccf::errors::InternalError,
"Primary unknown");
return redirect_response.value();
}

return add_node(
Expand Down Expand Up @@ -655,40 +662,10 @@ namespace ccf
}
else
{
if (consensus != nullptr && !this->node_operation.can_replicate())
const auto redirect_response = check_for_join_redirect();
if (redirect_response.has_value())
{
auto primary_id = consensus->primary();
if (primary_id.has_value())
{
auto info = nodes->get(primary_id.value());
if (info)
{
auto& interface_id =
args.rpc_ctx->get_session_context()->interface_id;
if (!interface_id.has_value())
{
return make_error(
HTTP_STATUS_INTERNAL_SERVER_ERROR,
ccf::errors::InternalError,
"Cannot redirect non-RPC request.");
}
const auto& address =
info->rpc_interfaces[interface_id.value()].published_address;
args.rpc_ctx->set_response_header(
http::headers::LOCATION,
fmt::format("https://{}/node/join", address));

return make_error(
HTTP_STATUS_PERMANENT_REDIRECT,
ccf::errors::NodeCannotHandleRequest,
"Node is not primary; cannot handle write");
}
}

return make_error(
HTTP_STATUS_INTERNAL_SERVER_ERROR,
ccf::errors::InternalError,
"Primary unknown");
return redirect_response.value();
}

// If the node does not exist, add it to the KV in state pending
Expand Down Expand Up @@ -1344,51 +1321,58 @@ namespace ccf
.set_auto_schema<void, GetNode::Out>()
.install();

auto head_primary = [this](auto& args) {
auto head_primary = [this](auto& args, nlohmann::json&&) {
if (this->node_operation.can_replicate())
{
args.rpc_ctx->set_response_status(HTTP_STATUS_OK);
// Body is ignored for head requests, but for backwards compatibility
// we want to ensure a 200 is returned rather than a 204
return ccf::make_success("Ignored body");
}
else
{
args.rpc_ctx->set_response_status(HTTP_STATUS_PERMANENT_REDIRECT);
if (consensus != nullptr)
if (consensus == nullptr)
{
auto primary_id = consensus->primary();
if (!primary_id.has_value())
{
args.rpc_ctx->set_error(
HTTP_STATUS_INTERNAL_SERVER_ERROR,
ccf::errors::InternalError,
"Primary unknown");
return;
}
return ccf::make_error(
HTTP_STATUS_INTERNAL_SERVER_ERROR,
ccf::errors::InternalError,
"Unable to determine primary - consensus object not created");
}

auto nodes = args.tx.ro(this->network.nodes);
auto info = nodes->get(primary_id.value());
if (info)
{
auto& interface_id =
args.rpc_ctx->get_session_context()->interface_id;
if (!interface_id.has_value())
{
args.rpc_ctx->set_error(
HTTP_STATUS_INTERNAL_SERVER_ERROR,
ccf::errors::InternalError,
"Cannot redirect non-RPC request.");
return;
}
const auto& address =
info->rpc_interfaces[interface_id.value()].published_address;
args.rpc_ctx->set_response_header(
http::headers::LOCATION,
fmt::format("https://{}/node/primary", address));
}
auto primary_id = consensus->primary();

if (!primary_id.has_value())
{
return ccf::make_error(
HTTP_STATUS_INTERNAL_SERVER_ERROR,
ccf::errors::InternalError,
"Primary unknown");
}

const AddressOrError address_or_error =
get_redirect_address_for_node(args, args.tx, primary_id.value());
if (std::holds_alternative<ccf::jsonhandler::JsonAdapterResponse>(
address_or_error))
{
return std::get<ccf::jsonhandler::JsonAdapterResponse>(
address_or_error);
}

const auto address = std::get<std::string>(address_or_error);

args.rpc_ctx->set_response_header(
ccf::http::headers::LOCATION,
fmt::format("https://{}/node/primary", address));
return ccf::make_error(
HTTP_STATUS_PERMANENT_REDIRECT,
ccf::errors::NodeCannotHandleRequest,
"Redirecting to primary");
}
};
make_read_only_endpoint(
"/primary", HTTP_HEAD, head_primary, no_auth_required)
"/primary",
HTTP_HEAD,
json_read_only_adapter(head_primary),
no_auth_required)
.set_forwarding_required(endpoints::ForwardingRequired::Never)
.install();

Expand Down Expand Up @@ -1910,16 +1894,15 @@ namespace ccf
static constexpr auto snapshot_since_param_key = "since";
// Redirects to endpoint for a single specific snapshot
auto find_snapshot =
[this](ccf::endpoints::ReadOnlyEndpointContext& ctx) {
[this](ccf::endpoints::ReadOnlyEndpointContext& ctx, nlohmann::json&&) {
auto node_configuration_subsystem =
this->context.get_subsystem<NodeConfigurationSubsystem>();
if (!node_configuration_subsystem)
{
ctx.rpc_ctx->set_error(
return make_error(
HTTP_STATUS_INTERNAL_SERVER_ERROR,
ccf::errors::InternalError,
"NodeConfigurationSubsystem is not available");
return;
}

const auto& snapshots_config =
Expand All @@ -1939,11 +1922,10 @@ namespace ccf
{
if (error_reason != "")
{
ctx.rpc_ctx->set_error(
return make_error(
HTTP_STATUS_BAD_REQUEST,
ccf::errors::InvalidQueryParameterValue,
std::move(error_reason));
return;
}
latest_idx = snapshot_since.value();
}
Expand All @@ -1956,41 +1938,51 @@ namespace ccf

if (!latest_committed_snapshot.has_value())
{
ctx.rpc_ctx->set_error(
return make_error(
HTTP_STATUS_NOT_FOUND,
ccf::errors::ResourceNotFound,
fmt::format(
"This node has no committed snapshots since {}", orig_latest));
return;
}

const auto& snapshot_path = latest_committed_snapshot.value();

const auto address =
get_redirect_address_for_node(ctx, this->context.get_node_id());
if (!address.has_value())
const AddressOrError address_or_error = get_redirect_address_for_node(
ctx, ctx.tx, this->context.get_node_id());
if (std::holds_alternative<ccf::jsonhandler::JsonAdapterResponse>(
address_or_error))
{
// Helper function should have populated error response, so return
// now
return;
return std::get<ccf::jsonhandler::JsonAdapterResponse>(
address_or_error);
}

auto redirect_url = fmt::format(
"https://{}/node/snapshot/{}", address.value(), snapshot_path);
const auto address = std::get<std::string>(address_or_error);
auto redirect_url =
fmt::format("https://{}/node/snapshot/{}", address, snapshot_path);
LOG_DEBUG_FMT("Redirecting to snapshot: {}", redirect_url);

ctx.rpc_ctx->set_response_header(
ccf::http::headers::LOCATION, redirect_url);
ctx.rpc_ctx->set_response_status(HTTP_STATUS_PERMANENT_REDIRECT);
return ccf::make_error(
HTTP_STATUS_PERMANENT_REDIRECT,
ccf::errors::NodeCannotHandleRequest,
"Redirecting to primary");
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message 'Redirecting to primary' is misleading in the context of the /snapshot endpoint. This message suggests redirection to the primary node, but this endpoint redirects to the current node's snapshot. Consider changing the message to something like 'Redirecting to snapshot' to accurately reflect the operation.

Suggested change
"Redirecting to primary");
"Redirecting to snapshot");

Copilot uses AI. Check for mistakes.
};
make_read_only_endpoint(
"/snapshot", HTTP_HEAD, find_snapshot, no_auth_required)
"/snapshot",
HTTP_HEAD,
json_read_only_adapter(find_snapshot),
no_auth_required)
.set_forwarding_required(endpoints::ForwardingRequired::Never)
.add_query_parameter<ccf::SeqNo>(
snapshot_since_param_key, ccf::endpoints::OptionalParameter)
.set_openapi_hidden(true)
.install();
make_read_only_endpoint(
"/snapshot", HTTP_GET, find_snapshot, no_auth_required)
"/snapshot",
HTTP_GET,
json_read_only_adapter(find_snapshot),
no_auth_required)
.set_forwarding_required(endpoints::ForwardingRequired::Never)
.add_query_parameter<ccf::SeqNo>(
snapshot_since_param_key, ccf::endpoints::OptionalParameter)
Expand Down
3 changes: 3 additions & 0 deletions src/snapshots/fetch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,9 @@ namespace snapshots

response_body = std::move(request->get_response_ptr());
curl_easy = std::move(request->get_easy_handle_ptr());

// Ignore any body from redirect responses
response_body->buffer.clear();
Comment on lines +280 to +281
Copy link
Member Author

@eddyashton eddyashton Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explanation: When we make a GET request, we don't know whether it's going to be a redirect or a partial body. We use the same response_body object for all calls, and it blindly pastes the body onto the end of an internal buffer. That's exactly the behaviour that we want when we're calling subsequent range requests, but we don't want to include the body of a redirect response. Previously they had no body so this wasn't a problem, now they do.

Ideally we'd be deciding how to handle the body based on the headers, but that's awkward with curl. Perhaps we should create a fresh ResponseBody per response, and glue together the ones we want at the end? That implies some extra resizing and probably a copy, so this is faster, just ugly.

}

while (!fetched_all)
Expand Down
Loading
Loading