bitflicker64 commented on code in PR #3189:
URL: https://github.com/apache/hugegraph/pull/3189#discussion_r3952112293
##########
hugegraph-server/hugegraph-dist/src/assembly/travis/start-pd.sh:
##########
@@ -31,6 +31,11 @@ fi
PD_DIR=$HOME_DIR/hugegraph-pd/apache-hugegraph-pd-$VersionInBash
+# conf/application.yml ships auth.secret-key empty on purpose, so PD would
+# refuse every authenticated REST request. Supply a test-only secret; it must
+# match the value the PD test suites send.
+export
SPRING_APPLICATION_JSON='{"auth":{"secret-key":"pd-ci-test-secret-not-for-production"}}'
Review Comment:
Fixed in `10f590a02`. The export is gone; the secret and the launch now sit
inside a subshell:
```bash
(
export SPRING_APPLICATION_JSON='{"auth":{"secret-key":"..."}}'
. bin/start-hugegraph-pd.sh
)
```
So `install-hstore.sh` sourcing this script and then `start-store.sh` in the
same shell no longer leaves PD's ports and raft properties set for Store's
Spring context. The comment above it now records why the scoping matters, so it
does not get flattened back into an export later.
##########
hugegraph-server/hugegraph-dist/src/assembly/travis/test-pd-shipped-config.sh:
##########
@@ -0,0 +1,59 @@
+#!/usr/bin/env 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.
+#
+# Every PD configuration that ships in the archive or the jar must carry the
+# same REST hardening: no wildcard actuator exposure (that path is anonymous),
+# an auth.secret-key that is present and empty, and no copy of the secret that
+# earlier revisions published. A fix applied to one variant and not the others
+# is what this catches.
+
+set -euo pipefail
+
+ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../../../.." && pwd)"
+PUBLISHED_SECRET='FXQXbJtbCLxODc6tGci732pkH1cyf8Qg'
+FAIL=0
+
+check() {
+ local file="$1" rel="${1#"${ROOT}/"}" before="$FAIL"
+ [[ -f "$file" ]] || { echo " FAIL ${rel}: missing"; FAIL=1; return; }
+
+ # The actuator exposure specifically: a config that grows an unrelated
+ # include: above this block must not satisfy the check by accident.
+ local exposure
+ exposure=$(awk '/^[[:space:]]*exposure:/ {found = 1; next}
+ found && /^[[:space:]]*include:/
{sub(/^[[:space:]]*include:[[:space:]]*/, ""); print; exit}' "$file")
+ if [[ "$exposure" == *'*'* ]]; then
+ echo " FAIL ${rel}: actuator exposure is a wildcard (${exposure})";
FAIL=1
+ fi
+ if ! grep -qE '^[[:space:]]*secret-key:[[:space:]]*$' "$file"; then
+ echo " FAIL ${rel}: auth.secret-key must be present and empty"; FAIL=1
+ fi
+ if grep -q "${PUBLISHED_SECRET}" "$file"; then
+ echo " FAIL ${rel}: contains the published secret"; FAIL=1
+ fi
+ # Only when nothing above raised FAIL, or the file contradicts its own
report
+ if [[ "$FAIL" == "$before" ]]; then
+ echo " ok ${rel}"
+ fi
+}
+
+echo "PD shipped configuration hardening"
+for f in
"${ROOT}"/hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml* \
Review Comment:
Fixed in `e420f9ab2`.
`hugegraph-cluster-test/hugegraph-clustertest-dist/src/assembly/static/conf/pd-application.yml.template`
now carries the same allowlist and empty-secret contract as the other shipped
configs, and `test-pd-shipped-config.sh` covers it, so the script's claim to
check every shipped PD configuration is now true rather than aspirational.
##########
hugegraph-pd/README.md:
##########
@@ -280,6 +290,30 @@ docker/docker-compose-3pd-3store-3server.yml
- Ensure low latency (<5ms) between PD nodes for Raft consensus
- Open required ports: `8620` (REST), `8686` (gRPC), `8610` (Raft)
+### Security
+
+- Keep all three ports on a trusted network. The REST API on `8620` includes
+ management endpoints that mutate the cluster (peer changes, store removal,
+ data movement), and the gRPC and Raft ports carry no authentication.
+- REST requests need HTTP Basic auth: one of the internal service names
+ (`hg`, `store`, `hubble`, `vermeer`) with the `auth.secret-key` value as
+ the password. Health probes (`/v1/health`, `/actuator/**`,
+ `/v1/prom/targets/*`) stay unauthenticated.
+- `auth.secret-key` has no shipped default, because a secret in the source
+ tree is published to everyone. Generate one per deployment (`openssl rand
+ -hex 24`) and set it in the config file, or through
+ `HG_PD_AUTH_SECRET_KEY`, which the Docker image requires. Give every REST
+ client the same value: the Server's `bin/wait-storage.sh` reads
+ `PD_AUTH_PASSWORD` (and `PD_AUTH_USER`, default `store`), and Hubble reads
+ `operations.pd.password`. A client left on a stale secret gets 401, and
+ `wait-storage.sh` aborts the Server's startup on the first one rather than
+ waiting out `WAIT_STORAGE_TIMEOUT_S`.
+- An existing `conf/application.yml` carried over from an earlier release has
Review Comment:
Fixed in `edc95180c`. The upgrade bullet now names both settings, so a
carried-over `conf/application.yml` gets the allowlist replaced as well as
`auth.secret-key` added.
##########
hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh:
##########
@@ -51,14 +72,22 @@ require_env "HG_PD_GRPC_HOST"
require_env "HG_PD_RAFT_ADDRESS"
require_env "HG_PD_RAFT_PEERS_LIST"
require_env "HG_PD_INITIAL_STORE_LIST"
+# The REST API refuses every authenticated request without this, and the image
+# ships no default because a published secret is not a secret.
+require_env "HG_PD_AUTH_SECRET_KEY"
: "${HG_PD_GRPC_PORT:=8686}"
: "${HG_PD_REST_PORT:=8620}"
: "${HG_PD_DATA_PATH:=/hugegraph-pd/pd_data}"
: "${HG_PD_INITIAL_STORE_COUNT:=1}"
+# Secret for REST Basic authentication (auth.secret-key). Required above and
+# never logged.
+AUTH_JSON="\"auth\": { \"secret-key\": \"$(json_escape
"${HG_PD_AUTH_SECRET_KEY}")\" },"
Review Comment:
Fixed in `edc95180c`. The entrypoint now emits the exposure allowlist in the
same JSON document as the secret, so it outranks whatever a mounted
`conf/application.yml` carries and the image stays closed even when an operator
bind-mounts a pre-1.8 config. `test-pd-docker-entrypoint.sh` covers the
generated document.
One consequence worth stating: `management.endpoints.web.exposure.include`
is now effectively pinned in the PD image, and `docs/configuration.md` still
presents it as a settable key. I will add a note there so an operator who
mounts a config to expose `loggers` is not left wondering why nothing happens.
--
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]
