Skip to content

Commit

Permalink
[hyperledger#4292] Fix mounted data path directory permissions for be…
Browse files Browse the repository at this point in the history
…su user (hyperledger#7575)

* Fix mounted data path directory permissions for besu user

Signed-off-by: Bhanu Pulluri <bhanu.pulluri@kaleido.io>

* Add besu CLI option to output dirs needing permission update

Signed-off-by: Bhanu Pulluri <bhanu.pulluri@kaleido.io>

* run spotless apply to handle PR test failure

Signed-off-by: Bhanu Pulluri <bhanu.pulluri@kaleido.io>

* Remove newly added --print-paths-and-exit option from config file test

This option doesn't have a corresponding config file entry as it's a
standalone option to be used with docker containers

Signed-off-by: Bhanu Pulluri <bhanu.pulluri@kaleido.io>

* Add optional user argument to --print-paths-and-exit and fix directory permissions

Signed-off-by: Bhanu Pulluri <bhanu.pulluri@kaleido.io>

* Correct build.gradle changes, remove a duplicate line and extra whitespaces

Signed-off-by: Bhanu Pulluri <bhanu.pulluri@kaleido.io>

* Fix checking for user in path's group membership

Signed-off-by: Bhanu Pulluri <bhanu.pulluri@kaleido.io>

* Add platform check to restrict --print-paths-and-exit option usage to Linux and Mac

Signed-off-by: Bhanu Pulluri <bhanu.pulluri@kaleido.io>

* Apply suggestions from code review

Co-authored-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Bhanu Pulluri <59369753+pullurib@users.noreply.github.com>

---------

Signed-off-by: Bhanu Pulluri <bhanu.pulluri@kaleido.io>
Signed-off-by: Bhanu Pulluri <59369753+pullurib@users.noreply.github.com>
Co-authored-by: Bhanu Pulluri <bhanu.pulluri@kaleido.io>
Co-authored-by: Fabio Di Fabio <fabio.difabio@consensys.net>
  • Loading branch information
3 people authored Sep 18, 2024
1 parent 578104e commit 0d9fa16
Show file tree
Hide file tree
Showing 9 changed files with 217 additions and 3 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
- Remove privacy test classes support [#7569](https://github.com/hyperledger/besu/pull/7569)

### Bug fixes
- Fix mounted data path directory permissions for besu user [#7575](https://github.com/hyperledger/besu/pull/7575)
- Fix for `debug_traceCall` to handle transactions without specified gas price. [#7510](https://github.com/hyperledger/besu/pull/7510)


## 24.9.1

### Upcoming Breaking Changes
Expand Down
137 changes: 137 additions & 0 deletions besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Arrays.asList;
import static java.util.Collections.singletonList;
import static org.hyperledger.besu.cli.DefaultCommandValues.getDefaultBesuDataPath;
Expand Down Expand Up @@ -203,16 +204,23 @@
import org.hyperledger.besu.util.number.Percentage;
import org.hyperledger.besu.util.number.PositiveNumber;

import java.io.BufferedReader;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.math.BigInteger;
import java.net.InetAddress;
import java.net.SocketException;
import java.net.URI;
import java.net.URL;
import java.net.UnknownHostException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.GroupPrincipal;
import java.nio.file.attribute.PosixFileAttributes;
import java.nio.file.attribute.PosixFilePermission;
import java.nio.file.attribute.UserPrincipal;
import java.time.Clock;
import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -232,6 +240,7 @@
import java.util.stream.Collectors;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import com.google.common.base.Suppliers;
import com.google.common.collect.ImmutableMap;
Expand All @@ -243,6 +252,8 @@
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.units.bigints.UInt256;
import org.slf4j.Logger;
import oshi.PlatformEnum;
import oshi.SystemInfo;
import picocli.AutoComplete;
import picocli.CommandLine;
import picocli.CommandLine.Command;
Expand Down Expand Up @@ -382,6 +393,28 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
arity = "1")
private final Optional<String> identityString = Optional.empty();

private Boolean printPathsAndExit = Boolean.FALSE;
private String besuUserName = "besu";

@Option(
names = "--print-paths-and-exit",
paramLabel = "<username>",
description = "Print the configured paths and exit without starting the node.",
arity = "0..1")
void setUserName(final String userName) {
PlatformEnum currentPlatform = SystemInfo.getCurrentPlatform();
// Only allow on Linux and macOS
if (currentPlatform == PlatformEnum.LINUX || currentPlatform == PlatformEnum.MACOS) {
if (userName != null) {
besuUserName = userName;
}
printPathsAndExit = Boolean.TRUE;
} else {
throw new UnsupportedOperationException(
"--print-paths-and-exit is only supported on Linux and macOS.");
}
}

// P2P Discovery Option Group
@CommandLine.ArgGroup(validate = false, heading = "@|bold P2P Discovery Options|@%n")
P2PDiscoveryOptionGroup p2PDiscoveryOptionGroup = new P2PDiscoveryOptionGroup();
Expand Down Expand Up @@ -1093,6 +1126,12 @@ public void run() {
try {
configureLogging(true);

if (printPathsAndExit) {
// Print configured paths requiring read/write permissions to be adjusted
checkPermissionsAndPrintPaths(besuUserName);
System.exit(0); // Exit before any services are started
}

// set merge config on the basis of genesis config
setMergeConfigOptions();

Expand Down Expand Up @@ -1138,6 +1177,104 @@ public void run() {
}
}

private void checkPermissionsAndPrintPaths(final String userName) {
// Check permissions for the data path
checkPermissions(dataDir(), userName, false);

// Check permissions for genesis file
try {
if (genesisFile != null) {
checkPermissions(genesisFile.toPath(), userName, true);
}
} catch (Exception e) {
commandLine
.getOut()
.println("Error: Failed checking genesis file: Reason: " + e.getMessage());
}
}

// Helper method to check permissions on a given path
private void checkPermissions(final Path path, final String besuUser, final boolean readOnly) {
try {
// Get the permissions of the file
// check if besu user is the owner - get owner permissions if yes
// else, check if besu user and owner are in the same group - if yes, check the group
// permission
// otherwise check permissions for others

// Get the owner of the file or directory
UserPrincipal owner = Files.getOwner(path);
boolean hasReadPermission, hasWritePermission;

// Get file permissions
Set<PosixFilePermission> permissions = Files.getPosixFilePermissions(path);

// Check if besu is the owner
if (owner.getName().equals(besuUser)) {
// Owner permissions
hasReadPermission = permissions.contains(PosixFilePermission.OWNER_READ);
hasWritePermission = permissions.contains(PosixFilePermission.OWNER_WRITE);
} else {
// Get the group of the file
// Get POSIX file attributes and then group
PosixFileAttributes attrs = Files.readAttributes(path, PosixFileAttributes.class);
GroupPrincipal group = attrs.group();

// Check if besu user belongs to this group
boolean isMember = isGroupMember(besuUserName, group);

if (isMember) {
// Group's permissions
hasReadPermission = permissions.contains(PosixFilePermission.GROUP_READ);
hasWritePermission = permissions.contains(PosixFilePermission.GROUP_WRITE);
} else {
// Others' permissions
hasReadPermission = permissions.contains(PosixFilePermission.OTHERS_READ);
hasWritePermission = permissions.contains(PosixFilePermission.OTHERS_WRITE);
}
}

if (!hasReadPermission || (!readOnly && !hasWritePermission)) {
String accessType = readOnly ? "READ" : "READ_WRITE";
commandLine.getOut().println("PERMISSION_CHECK_PATH:" + path + ":" + accessType);
}
} catch (Exception e) {
// Do nothing upon catching an error
commandLine
.getOut()
.println(
"Error: Failed to check permissions for path: '"
+ path
+ "'. Reason: "
+ e.getMessage());
}
}

private static boolean isGroupMember(final String userName, final GroupPrincipal group)
throws IOException {
// Get the groups of the user by executing 'id -Gn username'
Process process = Runtime.getRuntime().exec(new String[] {"id", "-Gn", userName});
BufferedReader reader =
new BufferedReader(new InputStreamReader(process.getInputStream(), UTF_8));

// Read the output of the command
String line = reader.readLine();
boolean isMember = false;
if (line != null) {
// Split the groups
Iterable<String> userGroups = Splitter.on(" ").split(line);
// Check if any of the user's groups match the file's group

for (String grp : userGroups) {
if (grp.equals(group.getName())) {
isMember = true;
break;
}
}
}
return isMember;
}

@VisibleForTesting
void setBesuConfiguration(final BesuConfigurationImpl pluginCommonConfiguration) {
this.pluginCommonConfiguration = pluginCommonConfiguration;
Expand Down
49 changes: 49 additions & 0 deletions besu/src/main/scripts/besu-entry.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
#!/bin/bash
##
## Copyright contributors to Hyperledger Besu.
##
## Licensed 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.
##
## SPDX-License-Identifier: Apache-2.0
##

# Run Besu first to get paths needing permission adjustment
output=$(/opt/besu/bin/besu --print-paths-and-exit $BESU_USER_NAME "$@")

# Parse the output to find the paths and their required access types
echo "$output" | while IFS=: read -r prefix path accessType; do
if [[ "$prefix" == "PERMISSION_CHECK_PATH" ]]; then
# Change ownership to besu user and group
chown -R $BESU_USER_NAME:$BESU_USER_NAME $path

# Ensure read/write permissions for besu user

echo "Setting permissions for: $path with access: $accessType"

if [[ "$accessType" == "READ" ]]; then
# Set read-only permissions for besu user
# Add execute for directories to allow access
find $path -type d -exec chmod u+rx {} \;
find $path -type f -exec chmod u+r {} \;
elif [[ "$accessType" == "READ_WRITE" ]]; then
# Set read/write permissions for besu user
# Add execute for directories to allow access
find $path -type d -exec chmod u+rwx {} \;
find $path -type f -exec chmod u+rw {} \;
fi
fi
done

# Finally, run Besu with the actual arguments passed to the container
# Construct the command as a single string
COMMAND="/opt/besu/bin/besu $@"

# Switch to the besu user and execute the command
exec su -s /bin/bash $BESU_USER_NAME -c "$COMMAND"
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,7 @@ public void tomlThatConfiguresEverythingExceptPermissioningToml() throws IOExcep
options.remove(spec.optionsMap().get("--config-file"));
options.remove(spec.optionsMap().get("--help"));
options.remove(spec.optionsMap().get("--version"));
options.remove(spec.optionsMap().get("--print-paths-and-exit"));

for (final String tomlKey : tomlResult.keySet()) {
final CommandLine.Model.OptionSpec optionSpec = spec.optionsMap().get("--" + tomlKey);
Expand Down
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -1097,6 +1097,7 @@ distributions {
from("build/reports/license/license-dependency.html") { into "." }
from("./docs/GettingStartedBinaries.md") { into "." }
from("./docs/DocsArchive0.8.0.html") { into "." }
from("./besu/src/main/scripts/besu-entry.sh") { into "./bin/" }
from(autocomplete) { into "." }
}
}
Expand Down
10 changes: 8 additions & 2 deletions docker/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ RUN apt-get update $NO_PROXY_CACHE && \
chown besu:besu /opt/besu && \
chmod 0755 /opt/besu

USER besu
ARG BESU_USER=besu
USER ${BESU_USER}
WORKDIR /opt/besu

COPY --chown=besu:besu besu /opt/besu/
Expand All @@ -43,7 +44,12 @@ ENV OTEL_RESOURCE_ATTRIBUTES="service.name=besu,service.version=$VERSION"
ENV OLDPATH="${PATH}"
ENV PATH="/opt/besu/bin:${OLDPATH}"

ENTRYPOINT ["besu"]
USER root
RUN chmod +x /opt/besu/bin/besu-entry.sh

ENV BESU_USER_NAME=${BESU_USER}

ENTRYPOINT ["besu-entry.sh"]
HEALTHCHECK --start-period=5s --interval=5s --timeout=1s --retries=10 CMD bash -c "[ -f /tmp/pid ]"

# Build-time metadata as defined at http://label-schema.org
Expand Down
8 changes: 8 additions & 0 deletions docker/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,12 @@ bash $TEST_PATH/dgoss run --sysctl net.ipv6.conf.all.disable_ipv6=1 $DOCKER_IMAG
--graphql-http-enabled \
> ./reports/01.xml || i=`expr $i + 1`

if [[ $i != 0 ]]; then exit $i; fi

# Test for directory permissions
GOSS_FILES_PATH=$TEST_PATH/02 \
bash $TEST_PATH/dgoss run --sysctl net.ipv6.conf.all.disable_ipv6=1 -v besu-data:/var/lib/besu $DOCKER_IMAGE --data-path=/var/lib/besu \
--network=dev \
> ./reports/02.xml || i=`expr $i + 1`

exit $i
10 changes: 10 additions & 0 deletions docker/tests/02/goss.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
# runtime docker tests
file:
/var/lib/besu:
exists: true
owner: besu
mode: "0755"
process:
java:
running: true
2 changes: 1 addition & 1 deletion docker/tests/dgoss
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ GOSS_PATH="${GOSS_PATH:-$(which goss 2> /dev/null || true)}"
[[ $GOSS_PATH ]] || { error "Couldn't find goss installation, please set GOSS_PATH to it"; }
[[ ${GOSS_OPTS+x} ]] || GOSS_OPTS="--color --format documentation"
[[ ${GOSS_WAIT_OPTS+x} ]] || GOSS_WAIT_OPTS="-r 30s -s 1s > /dev/null"
GOSS_SLEEP=${GOSS_SLEEP:-0.2}
GOSS_SLEEP=${GOSS_SLEEP:-1.0}

case "$1" in
run)
Expand Down

0 comments on commit 0d9fa16

Please sign in to comment.