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.

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

Jump to bottom

Data node rest resources contains endpoints without permission checks #18854

Closed
kroepke opened this issue Mar 29, 2024 · 0 comments · Fixed by #19328
Closed

Data node rest resources contains endpoints without permission checks #18854

kroepke opened this issue Mar 29, 2024 · 0 comments · Fixed by #19328
Assignees
@moesterheld
Labels
bug

Comments

@kroepke
Copy link
Member

kroepke commented Mar 29, 2024 •
edited

The endpoints /datanode/configured and /datanode/{nodeid} have no permission checks.
Please review whether that is required for bootstrap purposes or add a relevant permission annotation.

graylog2-server/graylog2-server/src/main/java/org/graylog2/rest/resources/datanodes/DataNodeManagementResource.java

Lines 99 to 122 in d8e1aee

@GET
@Path("{nodeId}")
@ApiOperation("Get data node information")
public DataNodeDto getDataNode(@ApiParam(name = "nodeId", required = true) @PathParam("nodeId") String nodeId) {
try {
return certRenewalService.addProvisioningInformation(nodeService.byNodeId(nodeId));
} catch (NodeNotFoundException e) {
throw new NotFoundException("Node " + nodeId + " not found");
}
}
@DELETE
@Path("{nodeId}")
@ApiOperation("Remove node from cluster")
@AuditEvent(type = DATANODE_REMOVE)
@RequiresPermissions(RestPermissions.DATANODE_REMOVE)
public DataNodeDto removeNode(@ApiParam(name = "nodeId", required = true) @PathParam("nodeId") String nodeId,
@Context UserContext userContext) {
try {
return dataNodeService.removeNode(nodeId);
} catch (NodeNotFoundException e) {
throw new NotFoundException("Node " + nodeId + " not found");
}
}

The endpoint POST /datanode/provision/generate does not seem to be used, at least I couldn't find any usages, but also has no permission checks.

graylog2-server/graylog2-server/src/main/java/org/graylog2/bootstrap/preflight/web/resources/DataNodeProvisioningResource.java

Lines 50 to 56 in d884902

@POST
@Path("/generate")
@NoAuditEvent("No Audit Event needed")
public void generate() {
final Map<String, DataNodeDto> activeDataNodes = nodeService.allActive();
activeDataNodes.values().forEach(node -> dataNodeProvisioningService.changeState(node.getNodeId(), DataNodeProvisioningConfig.State.CONFIGURED));
}

In a related PR (#18736), I'm adding checks for all resources, and these need your input.
If the omission of permission checks is on purpose, please let me know so I can add the correct annotation in that PR.
Thanks!
@kroepke kroepke added the bug label Mar 29, 2024
@kroepke kroepke changed the title Data node rest resource contains resources without permission checks Data node rest resources contains endpoints without permission checks Mar 29, 2024
@moesterheld moesterheld mentioned this issue May 13, 2024
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@moesterheld moesterheld

Labels
bug
Projects
None yet
Milestone
No milestone
2 participants
@kroepke @moesterheld