imbajin commented on code in PR #3126:
URL: https://github.com/apache/hugegraph/pull/3126#discussion_r3694062221
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/hugegraph-server.sh:
##########
@@ -142,8 +142,15 @@ case "$GC_OPTION" in
esac
JVM_OPTIONS="-Dlog4j.configurationFile=${CONF}/log4j2.xml"
+SECURITY_MANAGER_OPTION=""
if [[ ${OPEN_SECURITY_CHECK} == "true" ]]; then
- JVM_OPTIONS="${JVM_OPTIONS}
-Djava.security.manager=org.apache.hugegraph.security.HugeSecurityManager"
+ SECURITY_PROPERTIES="${CONF}/java-security.properties"
+ JVM_OPTIONS="${JVM_OPTIONS} \
+ -Djava.security.properties=${SECURITY_PROPERTIES}"
+ if [[ ${JAVA_VERSION} -ge 18 ]]; then
Review Comment:
‼️ This unbounded JDK 18+ branch also includes JDK 24+, where
`-Djava.security.manager=allow` is a fatal VM-initialization error and
`System.setSecurityManager()` cannot install the fallback manager (OpenJDK JEP
486). Since the project documents Java 11+ without an upper bound,
security-enabled JDK 24+ deployments cannot start. Please explicitly cap or
reject JDK 24+ with a clear compatibility message until the sandbox is
replaced, or provide a supported non-SecurityManager path, and exercise the
23/24 boundary with real JVMs.
##########
hugegraph-server/hugegraph-dist/src/assembly/travis/test-java-security-properties.sh:
##########
@@ -0,0 +1,442 @@
+#!/bin/bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You 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.
+
+set -euo pipefail
+
+SERVER_ROOT_INPUT="${1:?Usage: $0 PATH_TO_SERVER_DIST}"
+SERVER_ROOT=$(cd "$SERVER_ROOT_INPUT" && pwd)
+SERVER_SCRIPT="${SERVER_ROOT}/bin/hugegraph-server.sh"
+CONF="${SERVER_ROOT}/conf"
+SECURITY_PROPERTIES="${CONF}/java-security.properties"
+
+fail() {
+ echo "FAIL: $1" >&2
+ exit 1
+}
+
+assert_argument() {
+ local argument="$1"
+ local capture="$2"
+ grep -Fxq -- "$argument" "$capture" || \
+ fail "missing JVM argument: $argument"
+}
+
+assert_no_argument() {
+ local pattern="$1"
+ local capture="$2"
+ if grep -Eq -- "$pattern" "$capture"; then
+ fail "unexpected JVM argument matching: $pattern"
+ fi
+}
+
+if [[ ! -x "$SERVER_SCRIPT" ]]; then
+ fail "server script is not executable: $SERVER_SCRIPT"
+fi
+if [[ ! -f "$SECURITY_PROPERTIES" ]]; then
+ fail "security properties file is missing: $SECURITY_PROPERTIES"
+fi
+
+if [[ -n "${JAVA_HOME:-}" ]]; then
+ JAVA_BIN="${JAVA_HOME}/bin/java"
+else
+ JAVA_BIN="java"
+fi
+JAVA_MAJOR=$($JAVA_BIN -version 2>&1 | head -1 | cut -d'"' -f2 |
+ sed 's/^1\.//' | cut -d'.' -f1)
+SECURITY_MANAGER_OPTION=""
+if [[ "$JAVA_MAJOR" -ge 18 ]]; then
+ SECURITY_MANAGER_OPTION="-Djava.security.manager=allow"
+fi
+
+TEMP_DIR=$(mktemp -d)
+SECURITY_PROPERTIES_BACKUP="${TEMP_DIR}/java-security.properties"
+
+cleanup() {
+ if [[ -d "$SECURITY_PROPERTIES" ]]; then
+ rmdir "$SECURITY_PROPERTIES"
+ fi
+ if [[ -f "$SECURITY_PROPERTIES_BACKUP" &&
+ ! -e "$SECURITY_PROPERTIES" ]]; then
+ mv "$SECURITY_PROPERTIES_BACKUP" "$SECURITY_PROPERTIES"
+ fi
+ rm -rf "$TEMP_DIR"
+}
+
+trap cleanup EXIT
+
+CHECK_SOURCE="${TEMP_DIR}/ReadDnsCacheTtl.java"
+cat > "$CHECK_SOURCE" <<'JAVA'
+import java.security.Security;
+
+public class ReadDnsCacheTtl {
+ public static void main(String[] args) {
+ String value = Security.getProperty("networkaddress.cache.ttl");
+ if (args.length == 0) {
+ System.out.print(value);
+ return;
+ }
+ try {
+ if (Integer.parseInt(value) <= 0) {
+ System.exit(1);
+ }
+ } catch (NumberFormatException e) {
+ System.exit(1);
+ }
+ }
+}
+JAVA
+
+assert_valid_security_properties() {
+ "$JAVA_BIN" "$@" "$CHECK_SOURCE" --validate >/dev/null ||
+ fail "expected valid Java security properties: $*"
+}
+
+assert_invalid_security_properties() {
+ if "$JAVA_BIN" "$@" "$CHECK_SOURCE" --validate >/dev/null 2>&1; then
+ fail "expected invalid Java security properties: $*"
+ fi
+}
+
+assert_clean_bootstrap_error() {
+ local error_file="$1"
+ if grep -Eq 'Log4j|NetUtils|UnknownHost|hostname' "$error_file"; then
+ fail "bootstrap initialized logging or hostname resolution"
+ fi
+}
+
+assert_bootstrap_rejects_security_properties() {
+ local error_file="${TEMP_DIR}/server-validation.err"
+ if "$JAVA_BIN" "$@" \
+ ${SECURITY_MANAGER_OPTION} \
+ -cp "${SERVER_ROOT}/lib/*" \
+ org.apache.hugegraph.bootstrap.HugeGraphServerBootstrap true \
+ >/dev/null 2>"$error_file"; then
+ fail "server accepted invalid Java security properties: $*"
+ fi
+ grep -Fq "networkaddress.cache.ttl must load as a finite positive integer"
\
+ "$error_file" || fail "server did not report the invalid DNS TTL"
+ assert_clean_bootstrap_error "$error_file"
+}
+
+assert_bootstrap_accepts_security_properties() {
+ local error_file="${TEMP_DIR}/server-validation.err"
+ if "$JAVA_BIN" "$@" \
+ ${SECURITY_MANAGER_OPTION} \
+ -cp "${SERVER_ROOT}/lib/*" \
+ org.apache.hugegraph.bootstrap.HugeGraphServerBootstrap true \
+ >/dev/null 2>"$error_file"; then
+ fail "server unexpectedly started without configuration arguments"
+ fi
+ grep -Fq "Expected validation flag and two HugeGraphServer" \
+ "$error_file" || fail "valid DNS TTL did not reach argument
validation"
+ assert_clean_bootstrap_error "$error_file"
+}
+
+assert_bootstrap_handles_security_properties_load_failure() {
+ local error_file="${TEMP_DIR}/server-validation.err"
+ if "$JAVA_BIN" "$@" \
+ ${SECURITY_MANAGER_OPTION} \
+ -cp "${SERVER_ROOT}/lib/*" \
+ org.apache.hugegraph.bootstrap.HugeGraphServerBootstrap true \
+ >/dev/null 2>"$error_file"; then
+ fail "server accepted unloadable Java security properties: $*"
+ fi
+ grep -Fq "networkaddress.cache.ttl must load as a finite positive integer"
\
+ "$error_file" || fail "server did not report a stable load error"
+ assert_clean_bootstrap_error "$error_file"
+}
+
+assert_bootstrap_skips_security_validation() {
+ local error_file="${TEMP_DIR}/server-validation.err"
+ if "$JAVA_BIN" "$@" -cp "${SERVER_ROOT}/lib/*" \
+ org.apache.hugegraph.bootstrap.HugeGraphServerBootstrap false \
+ >/dev/null 2>"$error_file"; then
+ fail "server unexpectedly started without configuration arguments"
+ fi
+ grep -Fq "Expected validation flag and two HugeGraphServer" \
+ "$error_file" || fail "disabled DNS TTL validation was not
skipped"
+ assert_clean_bootstrap_error "$error_file"
+}
+
+assert_launcher_rejects_marker_bypass() {
+ local marker_value="$1"
+ local error_file="${TEMP_DIR}/launcher-marker-${marker_value}.err"
+ if
_JAVA_OPTIONS="-Dhugegraph.security.validate_dns_cache_ttl=${marker_value}" \
+ JAVA_OPTIONS="" STDOUT_MODE=true "$SERVER_SCRIPT" \
+ "${CONF}/gremlin-server.yaml" "${CONF}/rest-server.properties" true \
+ "-Djava.security.properties=${INFINITE_PROPERTIES}" \
+ >/dev/null 2>"$error_file"; then
+ fail "_JAVA_OPTIONS marker bypassed DNS TTL validation"
+ fi
+ grep -Fq "networkaddress.cache.ttl must load as a finite positive integer"
\
+ "$error_file" || fail "launcher did not report invalid DNS TTL"
+ assert_clean_bootstrap_error "$error_file"
+}
+
+assert_launcher_rejects_security_properties() {
+ local properties_path="$1"
+ local error_file="${TEMP_DIR}/launcher-properties.err"
+ if JAVA_OPTIONS="" STDOUT_MODE=true "$SERVER_SCRIPT" \
+ "${CONF}/gremlin-server.yaml" "${CONF}/rest-server.properties" true \
+ "-Djava.security.properties=${properties_path}" \
+ >/dev/null 2>"$error_file"; then
+ fail "launcher accepted invalid Java security properties"
+ fi
+ grep -Fq "networkaddress.cache.ttl must load as a finite positive integer"
\
+ "$error_file" || fail "launcher did not report invalid DNS TTL"
+ assert_clean_bootstrap_error "$error_file"
+}
+
+assert_launcher_accepts_security_properties() {
+ local properties_path="$1"
+ local error_file="${TEMP_DIR}/launcher-valid.err"
+ if JAVA_OPTIONS="" STDOUT_MODE=true "$SERVER_SCRIPT" \
+ "${TEMP_DIR}/missing-gremlin.yaml" \
+ "${TEMP_DIR}/missing-rest.properties" true \
+ "-Djava.security.properties=${properties_path}" \
+ >/dev/null 2>"$error_file"; then
Review Comment:
⚠️ This success-path check accepts any nonzero exit as long as stderr omits
the two bootstrap messages below. A `ClassNotFoundException`, malformed command
line, or lost configuration argument therefore makes the test pass without
proving that TTL validation and security-manager installation succeeded; the
disabled-path helper has the same negative-only pattern. Please assert a stable
downstream sentinel/error after the intended bootstrap path, or add a
validation-only mode with an explicit success signal.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
