bitflicker64 commented on code in PR #3126:
URL: https://github.com/apache/hugegraph/pull/3126#discussion_r3697850423
##########
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:
Real issue, fixed in ba36d5ce. `hugegraph-server.sh` now caps the security
check at Java 23 and exits with a message naming JEP 486 and pointing at
`start-hugegraph.sh -s false`, instead of the JVM dying on
`-Djava.security.manager=allow`. Security-disabled startup on JDK 24+ is
unchanged.
The 23/24 boundary is exercised through the existing mock-JVM harness (23
still gets `allow`, 24 is rejected, 24 with security off still launches). Real
JDK 23/24 runners would mean adding them to the Server CI matrix, which is
outside this fix.
##########
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:
Correct, fixed in ba36d5ce. Both launcher helpers now assert a positive
sentinel — the failure must arrive through `HugeGraphServerBootstrap.main` ->
`HugeGraphServer.main` -> `Failed to load yaml config file`, which is only
reachable once the TTL check and the security-manager install have both
succeeded.
Confirmed by mutation: pointing the launcher at a missing bootstrap class
produced `ClassNotFoundException`, which passed the old negative-only check and
fails the new one.
--
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]
