Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -1098,9 +1098,22 @@ Map<ProtoNameVer, ProtoClassProtoImpl> getProtocolImplMap(RPC.RpcKind rpcKind) {
new HashMap<ProtoNameVer, ProtoClassProtoImpl>(10));
}
}
return protocolImplMapArray.get(rpcKind.ordinal());
return protocolImplMapArray.get(rpcKind.ordinal());
}


/**
* Returns {@code true} only if at least one protocol has been registered
* on this server instance for the given {@link RPC.RpcKind}.
* Used to reject incoming requests for unsupported RPC kinds before any
* deserialization of the request payload takes place.
* @param rpcKind the RPC kind from the incoming request header.
* @return {@code true} if at least one protocol is registered for this kind.
*/
boolean hasRegisteredProtocols(RPC.RpcKind rpcKind) {
Map<ProtoNameVer, ProtoClassProtoImpl> implMap = getProtocolImplMap(rpcKind);
return implMap != null && !implMap.isEmpty();
}

// Register protocol and its impl for rpc calls
void registerProtocolAndImpl(RpcKind rpcKind, Class<?> protocolClass,
Object protocolImpl) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2946,28 +2946,40 @@ private void processRpcRequest(RpcRequestHeaderProto header,
throw new FatalRpcServerException(
RpcErrorCodeProto.FATAL_INVALID_RPC_HEADER, err);
}
Class<? extends Writable> rpcRequestClass =
// Reject requests for RPC kinds with no registered protocols on this
// server instance. This prevents deserialization of untrusted payloads
// for unsupported kinds. See HADOOP-19864.
if (Server.this instanceof RPC.Server server) {
final RPC.RpcKind kind = ProtoUtil.convert(header.getRpcKind());
if (!server.hasRegisteredProtocols(kind)) {
final String err = "No protocols registered on this server for RpcKind "
+ header.getRpcKind()
+ ". Rejecting request without deserialization.";
LOG.info("{} Client: {}", err, getHostAddress());
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

rejected as upgrading to warn is even noisier. We could go to LogExactlyOnce

throw new FatalRpcServerException(
RpcErrorCodeProto.FATAL_INVALID_RPC_HEADER, err);
}
}
Class<? extends Writable> rpcRequestClass =
getRpcRequestWrapper(header.getRpcKind());
if (rpcRequestClass == null) {
LOG.warn("Unknown rpc kind " + header.getRpcKind() +
" from client " + getHostAddress());
final String err = "Unknown rpc kind in rpc header" +
header.getRpcKind();
LOG.warn("Unknown rpc kind {} from client {}", header.getRpcKind(), getHostAddress());
throw new FatalRpcServerException(
RpcErrorCodeProto.FATAL_INVALID_RPC_HEADER, err);
RpcErrorCodeProto.FATAL_INVALID_RPC_HEADER,
"Unknown rpc kind in rpc header" + header.getRpcKind());
}
Writable rpcRequest;
try { //Read the rpc request
rpcRequest = buffer.newInstance(rpcRequestClass, conf);
} catch (RpcServerException rse) { // lets tests inject failures.
throw rse;
} catch (Throwable t) { // includes runtime exception from newInstance
LOG.warn("Unable to read call parameters for client " +
getHostAddress() + "on connection protocol " +
this.protocolName + " for rpcKind " + header.getRpcKind(), t);
String err = "IPC server unable to read call parameters: "+ t.getMessage();
LOG.warn(
"Unable to read call parameters for client {}on connection protocol {} for rpcKind {}",
getHostAddress(), this.protocolName, header.getRpcKind(), t);
throw new FatalRpcServerException(
RpcErrorCodeProto.FATAL_DESERIALIZING_REQUEST, err);
RpcErrorCodeProto.FATAL_DESERIALIZING_REQUEST,
"IPC server unable to read call parameters: "+ t.getMessage());
}

Span span = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2116,6 +2116,31 @@ public void testNumTotalRequestsMetrics() throws Exception {
}


/**
* Test that a Protobuf-only RPC server rejects requests for RpcKinds
* that have no registered protocols, without deserializing the payload.
*/
@Test
@Timeout(value = 30)
public void testUnregisteredRpcKindRejectedWithoutDeserialization()
throws Exception {
// Standard test server: only RPC_PROTOCOL_BUFFER protocols are registered.
RPC.Server server = setupTestServer(conf, 1);
try {
// RPC_PROTOCOL_BUFFER has registered protocols — must be accepted.
assertThat(server.hasRegisteredProtocols(RPC.RpcKind.RPC_PROTOCOL_BUFFER))
.as("RPC_PROTOCOL_BUFFER should have registered protocols")
.isTrue();

// RPC_BUILTIN has no protocols registered on this server — must be rejected.
assertThat(server.hasRegisteredProtocols(RPC.RpcKind.RPC_BUILTIN))
.as("RPC_BUILTIN should have no registered protocols on a Protobuf-only server")
.isFalse();
} finally {
server.stop();
}
}
Comment on lines +2119 to +2142

public static void main(String[] args) throws Exception {
new TestRPC().testCallsInternal(conf);
}
Expand Down
Loading