bitflicker64 commented on code in PR #3192:
URL: https://github.com/apache/hugegraph/pull/3192#discussion_r4092697117
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/enable-auth.sh:
##########
@@ -34,23 +36,179 @@ GREMLIN_SERVER_CONF="gremlin-server.yaml"
REST_SERVER_CONF="rest-server.properties"
GRAPH_CONF="hugegraph.properties"
+fail() {
+ echo "enable-auth.sh: $*" >&2
+ exit 1
+}
+
+# Reading and writing .properties files goes through props.awk, the same helper
+# the docker entrypoint uses, because the keys below can be spelled in every
way
+# java.util.Properties accepts: `=`/`:`/bare-whitespace separators, a form feed
+# as whitespace, `\.` or `\u002e` for the dots, and LF, CRLF or CR line
+# terminators. grep and sed see a different file. A legal
+# `gremlin\u002egraph=org.apache.hugegraph.HugeFactory` matched no pattern at
+# all, so the factory was never wrapped for auth even though both servers were
+# told authentication was on -- and the CR byte that the previous pattern had
+# to be handed a carriage return for is now handled by the reader itself.
+#
+# props.awk is packaged in this same bin/ directory by the release assembly, so
+# it is present in the tarball and in the image. PROPS_AWK lets a caller point
+# at a different copy; the entrypoint reads that same variable for itself.
+for candidate in "${PROPS_AWK:-}" "${BIN}/props.awk" "${TOP}/props.awk"; do
+ if [[ -n "${candidate}" && -f "${candidate}" ]]; then
+ PROPS_AWK="${candidate}"
+ break
+ fi
+done
+[[ -n "${PROPS_AWK:-}" ]] || fail "props.awk not found beside this script"
+
+# The Gremlin half of the decision below has to be made by the same reader the
+# entrypoint uses. yamlscan.awk is not in bin/: the Dockerfile places it in
the
+# install home, one above this script, which is where the image layout is
+# mirrored in the test tree; PROPS_AWK and YAMLSCAN_AWK cover a caller that
+# keeps it elsewhere. The release tarball carries no copy at all, so the
+# fallback below has to answer on its own.
+YAMLSCAN=""
+for candidate in "${YAMLSCAN_AWK:-}" "${TOP}/yamlscan.awk"
"${BIN}/yamlscan.awk"; do
+ if [[ -n "${candidate}" && -f "${candidate}" ]]; then
+ YAMLSCAN="${candidate}"
+ break
+ fi
+done
+
+# props_get is the only reader used here, and it treats any nonzero status from
+# props.awk as an error: 2 means the file could not be read at all, which must
+# not be mistaken for "the key is not there" and answered with a write.
+props_get() {
+ local status=0 value
+ value=$(PROPS_MODE=get PROPS_DECODED=1 PROPS_KEY="$1" PROPS_FILE="$2" \
+ awk -f "${PROPS_AWK}" /dev/null) || status=$?
+ if (( status > 0 )); then
+ fail "cannot read $2"
+ fi
+ printf '%s' "${value}"
+}
+
+props_set() {
+ # The only values written here are Java class names, whose characters need
+ # no properties escaping; anything else would have to go through the
+ # entrypoint's encoder first.
+ case "$2" in
+ *[!A-Za-z0-9_\.\$]*) fail "refusing to write an unescaped value: $2" ;;
+ esac
+ PROPS_MODE=set PROPS_KEY="$1" PROPS_VALUE_ENCODED="$2" PROPS_FILE="$3" \
+ awk -f "${PROPS_AWK}" /dev/null || fail "cannot update $3"
+}
+
+# Give `$3` its default `$2` for key `$1`, in place, unless it already has a
+# value. Guarding with props_has and appending was not the same question:
+# `auth.authenticator=` and a bare `auth.authenticator` line both parse to the
+# empty string (measured against java.util.Properties, which also strips the
+# trailing blanks of `auth.authenticator= `), so props_has reported them as
+# answered and the append was skipped -- while the entrypoint's
+# check_auth_sides, which asks for the value rather than the key, counted the
+# same file as unconfigured. `loadAuthenticator("")` returns null, so REST
then
+# served without authentication next to a Gremlin that required it.
+#
+# props_set covers both shapes the guard had to split: with a definition
+# present it replaces the first one where it stands (no duplicate for
+# first-definition-wins to bury), with none present it appends.
+ensure_rest_prop() {
+ [[ -n "$(props_get "$1" "$3")" ]] && return 0
+ props_set "$1" "$2" "$3"
+}
+
# make a backup
BAK_CONF="$TOP/conf-bak"
if [ ! -d "$BAK_CONF" ]; then
- mkdir -p "$BAK_CONF"
- cp "${CONF}/${GREMLIN_SERVER_CONF}"
"${BAK_CONF}/${GREMLIN_SERVER_CONF}.bak"
- cp "${CONF}/${REST_SERVER_CONF}" "${BAK_CONF}/${REST_SERVER_CONF}.bak"
- cp "${CONF}/graphs/${GRAPH_CONF}" "${BAK_CONF}/${GRAPH_CONF}.bak"
+ mkdir -p "$BAK_CONF" || fail "cannot create ${BAK_CONF}"
+ cp "${CONF}/${GREMLIN_SERVER_CONF}"
"${BAK_CONF}/${GREMLIN_SERVER_CONF}.bak" ||
+ fail "cannot back up ${GREMLIN_SERVER_CONF}"
+ cp "${CONF}/${REST_SERVER_CONF}" "${BAK_CONF}/${REST_SERVER_CONF}.bak" ||
+ fail "cannot back up ${REST_SERVER_CONF}"
+ cp "${CONF}/graphs/${GRAPH_CONF}" "${BAK_CONF}/${GRAPH_CONF}.bak" ||
+ fail "cannot back up ${GRAPH_CONF}"
+fi
+
+# Both writes below skip a side that already carries a real value, so they are
+# no-ops on a mounted config or a re-run. Appending unconditionally used to
+# create duplicate definitions that the properties parser (first definition
+# wins) and the yaml parser (last wins) resolved in opposite directions,
leaving
+# Gremlin and REST on different authenticators. That is why the REST side goes
+# through ensure_rest_prop rather than a presence guard plus an append: a
+# presence guard also lets a defined-but-empty key count as answered, and the
+# appended default would then be the definition the server never reads.
+#
+# Appended with `>>` rather than `sed -i '$a\...'`: GNU sed's `$` address never
+# matches when the file has no lines, so on an empty mounted config every
append
+# silently did nothing. `sed -i '$a'` also closed the previous last line for
us,
+# which `>>` does not, so a file without a trailing newline gets one first.
+#
+# Every write here has to be seen to succeed. The docker entrypoint runs this
+# script and trusts its exit status, and a partially updated tree -- REST
+# configured, yaml append refused by a read-only mounted file -- is exactly the
+# one-sided state the entrypoint refuses to start with. Without errexit and
+# these checks the script exited 0 on that half-done job.
+append_lines() {
+ local file="$1"
+ shift
+ if [[ ! -w "${file}" ]]; then
+ fail "cannot append to ${file}: not writable"
+ fi
+ if [[ -s "${file}" && -n "$(tail -c 1 "${file}")" ]]; then
+ printf '\n' >> "${file}" || fail "cannot append to ${file}"
+ fi
+ printf '%s\n' "$@" >> "${file}" || fail "cannot append to ${file}"
+}
+AUTHENTICATOR_CLASS="${AUTHENTICATOR_CLASS:-org.apache.hugegraph.auth.StandardAuthenticator}"
- sed -i -e '$a\authentication: {' \
- -e '$a\ authenticator:
org.apache.hugegraph.auth.StandardAuthenticator,' \
- -e '$a\ authenticationHandler:
org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler,' \
- -e '$a\ config: {tokens: conf/rest-server.properties}' \
- -e '$a\}' ${CONF}/${GREMLIN_SERVER_CONF}
+# Does the Gremlin config already carry a top-level `authentication` mapping?
+# This is the same question check_auth_sides answers, so it has to go to the
same
+# reader: a mapping is the server's only at column 0, comment text is not
+# content, and the key may be quoted. grep asks it differently -- it sees only
+# the bare spelling, so an operator's `"authentication":` block read as absent
+# and a second default block was appended beside it, after which the two
servers
+# can resolve the key in opposite directions while REST keeps its existing
+# authenticator. Anything other than `none` means a mapping is there and the
+# append is not this script's to make.
+gremlin_has_auth_block() {
+ local file="$1" state
+ [[ -f "${file}" ]] || return 1
+ if [[ -n "${YAMLSCAN}" ]]; then
+ state=$(awk -f "${YAMLSCAN}" "${file}") || fail "cannot read ${file}"
+ [[ "${state}" != "none" ]]
+ return
+ fi
+ # No parser in this layout (the plain release tarball). Match what grep
can
+ # honestly answer here: a column-0 key in either quote style or none. The
+ # nested-mapping and comment cases are the ones that need the real reader,
+ # and the image, where the entrypoint runs this script, always has it.
+ grep -Eq "^[\"']?authentication[\"']?[[:blank:]]*:" "${file}"
+}
+
+# Only a column-0 `authentication` mapping is the Gremlin server's, which is
the
+# rule yamlscan.awk applies to decide the same thing for check_auth_sides.
With
+# a guard that disagreed on nesting, the entrypoint read the file as `none`, so
+# parity held and it called this script, but the guard saw the nested key and
+# skipped the append, writing the REST side only -- StandardAuthenticator on
+# REST, TinkerPop's AllowAllAuthenticator on Gremlin.
+if ! gremlin_has_auth_block "${CONF}/${GREMLIN_SERVER_CONF}"; then
+ append_lines "${CONF}/${GREMLIN_SERVER_CONF}" \
+ 'authentication: {' \
+ " authenticator: ${AUTHENTICATOR_CLASS}," \
+ ' authenticationHandler:
org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler,' \
+ ' config: {tokens: conf/rest-server.properties}' \
+ '}'
+fi
- sed -i -e
'$a\auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator' \
- -e '$a\auth.graph_store=hugegraph' ${CONF}/${REST_SERVER_CONF}
+ensure_rest_prop "auth.authenticator" "${AUTHENTICATOR_CLASS}"
"${CONF}/${REST_SERVER_CONF}"
+ensure_rest_prop "auth.graph_store" "hugegraph" "${CONF}/${REST_SERVER_CONF}"
- sed -i
's/gremlin.graph=org.apache.hugegraph.HugeFactory/gremlin.graph=org.apache.hugegraph.auth.HugeFactoryAuthProxy/g'
${CONF}/graphs/${GRAPH_CONF}
+# Wrap the graph factory only when it really is the plain HugeFactory, which is
+# a question about the decoded value, so it goes through the same reader.
+GRAPH_FACTORY=$(props_get "gremlin.graph" "${CONF}/graphs/${GRAPH_CONF}")
+if [[ "${GRAPH_FACTORY}" == "org.apache.hugegraph.HugeFactory" ]]; then
Review Comment:
Important: This compares the decoded value without trimming it, so a
`gremlin.graph` line with trailing whitespace is no longer wrapped, and the
server still opens the graph with the plain HugeFactory. commons-configuration2
2.8.0 (what HugeConfig loads with) trims the line. props.awk keeps the trailing
blank, which is java.util.Properties behaviour.
I copied the shipped conf/ tree, added one trailing space to
`gremlin.graph=org.apache.hugegraph.HugeFactory`, and ran enable-auth.sh:
```
base (sed s/gremlin.graph=...HugeFactory/.../g) ->
gremlin.graph=org.apache.hugegraph.auth.HugeFactoryAuthProxy
cc2 getProperty ->
[org.apache.hugegraph.auth.HugeFactoryAuthProxy]
head d5ccb94 (props_get + exact ==) ->
gremlin.graph=org.apache.hugegraph.HugeFactory
cc2 getProperty ->
[org.apache.hugegraph.HugeFactory]
```
enable-auth.sh exits 0 in both runs. At this head, authentication is enabled
on both sides but the graph is not behind HugeGraphAuthProxy. GraphManager only
logs a warning about that (GraphManager.java:1851). So a mounted config that
master wrapped now runs without per-user access control on the graph. The same
trim gap was raised for the old get-decoded path in an earlier round, and it
came back when this comparison moved to `PROPS_DECODED=1`.
Requested change: strip trailing space, tab and form feed from
`GRAPH_FACTORY` before the comparison, or make the decoded read trim the way
commons-configuration2 does. Add a test case with a trailing blank after
`HugeFactory` that asserts the factory is wrapped.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/props.awk:
##########
@@ -0,0 +1,374 @@
+# 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.
+#
+# props.awk — read and rewrite Java ".properties" files with the grammar
+# HugeConfig (commons-configuration over JDK Properties) applies, so the
+# entrypoint and the server agree on what a mounted file means. grep/sed
+# rewrites do not: they see `\`-escaped keys, `:` separators, continuation
+# lines and duplicate definitions differently, which is how a mounted
+# config ends up with two definitions of one key.
+#
+# One invocation, selected with the `PROPS_MODE` environment variable:
+#
+# PROPS_MODE=get PROPS_KEY=K PROPS_FILE=F
+# print the value of K's first logical definition, in the on-disk
+# escaped form; with PROPS_DECODED=1 print it as java.util.Properties
+# would hand it to the server. Always exits 0.
+# PROPS_MODE=has PROPS_KEY=K PROPS_FILE=F
+# print nothing; exit 0 when K has any definition at all, empty
+# included, 1 when it has none, 2 on an error
+# PROPS_MODE=set PROPS_KEY=K PROPS_FILE=F
+# replace K's first definition in place, drop every other
+# definition of K, append one when the file has none. The new
+# value arrives pre-encoded in PROPS_VALUE_ENCODED (an environment
+# variable, so secrets never appear in `ps` output or in awk's
+# argv), and -v is not used for it so awk cannot mangle its
+# backslash escapes.
+#
+# Grammar implemented (java.util.Properties line reader + the
+# first-definition-wins rule Configuration.getString applies):
+# - physical lines end at \r\n, \n or a bare \r, as in java.util.Properties
+# - '#' / '!' comments and blank lines
+# - '=' / ':' / whitespace separators, where the whitespace Java counts is
+# space, tab and form feed, with whitespace then an optional single '=' or
+# ':' accepted as one separator
+# - continuations: a physical line ending in an odd number of
+# backslashes joins the next line (its leading whitespace stripped)
+# - backslash escapes in keys and values, including \uXXXX
+# - duplicate logical keys resolve to the first definition
+#
+# Rewrites keep every untouched line byte-for-byte (comments, blank
+# lines, unrelated entries), and replace the first definition where it
+# stands, so mounted configs stay reviewable in git diffs.
+
+function die(msg) {
+ printf "props.awk: %s\n", msg > "/dev/stderr"
+ # 2 for an error, so a caller that reads exit status 1 as "the key is not
+ # there" (PROPS_MODE=has) cannot mistake an unreadable file for an absent
+ # property and append a definition on top of one it failed to read.
+ exit 2
+}
+
+function hex_digit(c) {
+ return index("0123456789abcdef", tolower(c)) - 1
+}
+
+# \uXXXX is a UTF-16 code unit in Java. Values here are effectively
+# ISO-8859-1, so codes above 0xFF are kept as their literal escape text
+# rather than being mangled through a single-byte sprintf.
+function unescape(s, out, i, n, c, code, j, d, ok) {
+ out = ""
+ n = length(s)
+ for (i = 1; i <= n; i++) {
+ c = substr(s, i, 1)
+ if (c != "\\") { out = out c; continue }
+ if (i == n) break
+ i++
+ c = substr(s, i, 1)
+ if (c == "u" && i + 4 <= n) {
+ code = 0
+ ok = 1
+ for (j = 1; j <= 4; j++) {
+ d = hex_digit(substr(s, i + j, 1))
+ if (d < 0) { ok = 0; break }
+ code = code * 16 + d
+ }
+ if (ok) {
+ i += 4
+ if (code <= 255) out = out sprintf("%c", code)
+ else out = out substr(s, i - 5, 6)
+ continue
+ }
+ }
+ if (c == "t") out = out "\t"
+ else if (c == "n") out = out "\n"
+ else if (c == "r") out = out "\r"
+ else if (c == "f") out = out "\f"
+ else out = out c
+ }
+ return out
+}
+
+# A physical line is continued when it ends in an odd number of
+# backslashes (an even count escapes itself).
+function trailing_backslashes(s, n, k) {
+ n = length(s)
+ k = 0
+ while (k < n && substr(s, n - k, 1) == "\\") k++
+ return k
+}
+
+function is_skipped(raw) {
+ return raw ~ /^[ \t\f]*([#!]|$)/
+}
+
+# Split a logical line into its raw (still-escaped) key and value parts.
+# Results land in K_RAW / V_RAW because awk returns one value.
+# Java treats form feed as whitespace on both sides of the separator, so
+# `auth.authenticator<FF>=...` is one property here too; reading it as part of
+# the key name made a valid mounted configuration invisible to the guards.
+function split_kv(s, n, i, c, esc, sep_at, rest) {
+ n = length(s)
+ esc = 0
+ sep_at = 0
+ for (i = 1; i <= n; i++) {
+ c = substr(s, i, 1)
+ if (esc) { esc = 0; continue }
+ if (c == "\\") { esc = 1; continue }
+ if (c == "=" || c == ":" || c == " " || c == "\t" || c == "\f") {
sep_at = i; break }
+ }
+ if (sep_at == 0) {
+ K_RAW = s
+ V_RAW = ""
+ return
+ }
+ K_RAW = substr(s, 1, sep_at - 1)
+ rest = substr(s, sep_at)
+ c = substr(rest, 1, 1)
+ if (c == "=" || c == ":") {
+ rest = substr(rest, 2)
+ } else {
+ sub(/^[ \t\f]+/, "", rest)
+ c = substr(rest, 1, 1)
+ if (c == "=" || c == ":") rest = substr(rest, 2)
+ }
+ sub(/^[ \t\f]+/, "", rest)
+ V_RAW = rest
+}
+
+function shquote(s) {
+ gsub(/'/, "'\\''", s)
+ return "'" s "'"
+}
+
+# java.util.Properties ends a physical line at \r\n, \n or a bare \r, but
+# getline splits on \n alone. A properties file saved with CR-only endings --
+# which java.util.Properties writes for a lone `store()` on some platforms, and
+# which a mounted config can arrive with -- therefore reached the parser as one
+# enormous record: only its first key was ever seen, and rewriting that key
+# replaced the whole record and dropped every later entry, including
+# auth.authenticator. So the file is re-scanned for terminators here.
+#
+# RAW[] keeps the exact bytes of each line and RAWTERM[] its terminator, so a
+# rewrite still replays untouched lines byte-for-byte. A file whose last line
+# carries no terminator gets a \n, which is what the replay did before.
+function scan_records(s, i, n, c, start, term, len, cnt) {
+ n = length(s)
+ cnt = 0
+ start = 1
+ i = 1
+ while (i <= n) {
+ c = substr(s, i, 1)
+ if (c != "\r" && c != "\n") { i++; continue }
+ if (c == "\r" && substr(s, i + 1, 1) == "\n") {
+ term = "\r\n"
+ len = 2
+ } else {
+ term = c
+ len = 1
+ }
+ cnt++
+ RAW[cnt] = substr(s, start, i - start)
+ RAWTERM[cnt] = term
+ start = i + len
+ i = start
+ }
+ if (start <= n) {
+ cnt++
+ RAW[cnt] = substr(s, start)
+ RAWTERM[cnt] = ""
+ }
+ return cnt
+}
+
+# Load `file` into per-block arrays: one block per comment/blank line or
+# logical entry, spanning exactly the physical lines it occupies.
+function props_load(file, raw, rc, content, nl, stripped, next_raw, start,
logical) {
+ content = ""
+ while ((rc = (getline raw < file)) > 0)
+ content = content raw "\n"
+ if (rc == -1)
+ die("cannot read " file)
+ close(file)
+
+ NLINES = scan_records(content)
+
+ NBLOCK = 0
+ for (nl = 1; nl <= NLINES; nl++) {
+ # RAW[] holds one java.util.Properties physical line with its
terminator
+ # already removed, so no CR stripping is needed here.
+ stripped = RAW[nl]
+ if (is_skipped(stripped)) {
+ NBLOCK++
+ BTYPE[NBLOCK] = "skip"
+ BFIRST[NBLOCK] = nl
+ BLAST[NBLOCK] = nl
+ continue
+ }
+ start = nl
+ logical = stripped
+ while (trailing_backslashes(logical) % 2 == 1 && nl < NLINES) {
+ logical = substr(logical, 1, length(logical) - 1)
+ nl++
+ next_raw = RAW[nl]
+ sub(/^[ \t\f]+/, "", next_raw)
+ logical = logical next_raw
+ }
+ # java.util.Properties ignores whitespace before the key; strip it
+ # so split_kv's separator scan agrees (an indented key used to be
+ # read as a key whose name started with a space, and a set then
+ # appended a second definition of the real key).
+ sub(/^[ \t\f]+/, "", logical)
+ split_kv(logical)
+ NBLOCK++
+ BTYPE[NBLOCK] = "entry"
+ BFIRST[NBLOCK] = start
+ BLAST[NBLOCK] = nl
+ BKEY[NBLOCK] = unescape(K_RAW)
+ # Values stay in their on-disk escaped form. get Prop callers feed
+ # the result straight back into set, which would corrupt a decoded
+ # value by re-writing its backslashes as literals; keys are
+ # unescaped because they are matched against plain names.
+ BVAL[NBLOCK] = V_RAW
+ }
+}
+
+function props_set(file, key, enc_val, tmp, bak, cmd, b, first, ln, msg,
nbs) {
+ props_load(file)
+ # A value whose encoded form ends in an odd number of backslashes would
+ # turn the line written after it into a continuation of that value.
+ # Measured against commons-configuration2 (what HugeConfig extends), the
+ # same input read back yields no property at all, so a secret written this
+ # way would never reach the server that is supposed to authenticate with
+ # it; the entrypoint has to refuse instead of guessing a target.
+ nbs = 0
+ while (nbs < length(enc_val) && substr(enc_val, length(enc_val) - nbs, 1)
== "\\")
+ nbs++
+ if (nbs % 2 == 1)
Review Comment:
Important: This guard exists so that a written value never turns the next
line into a continuation under commons-configuration2. It misses `\ ` at the
end of a value, which is how encode_prop_value writes a trailing space. cc2
trims the line before its continuation check, so `abc\ ` becomes `abc\` and
swallows the following line.
I ran the entrypoint order (check_auth_sides, `set_prop auth.admin_pa "abc
"`, then enable-auth.sh) on the shipped default conf/. The run ends like this:
```
rest-server.properties tail:
auth.admin_pa=abc\
auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator
auth.graph_store=hugegraph
check_auth_sides -> passes (props.awk: rest named, yaml named)
cc2 getProperty auth.authenticator -> null
cc2 getProperty auth.admin_pa ->
[abcauth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator]
```
The REST server then loads with no authenticator while Gremlin requires one.
That is the one-sided state check_auth_sides exists to refuse. The same thing
happens to whatever line follows a `auth.token_secret` that ends in a space.
The encoder is older than this PR, so master behaves the same way. But this PR
adds the parity check and this cc2 guard for exactly this hazard, and neither
catches it. cc2 also keeps `\ ` inside a value: `auth.admin_pa=Strong\\Pass\
9!`, the fixture in docker-entrypoint-test.sh, reads back as `Strong\Pass\ 9!`
rather than `Strong\Pass 9!`.
Requested change: have encode_prop_value write a space as `\u0020`. Both
readers decode that the same way: cc2 and java.util.Properties both return `abc
` for `abc\u0020`. Also extend this guard to refuse an encoded value that ends
in a backslash followed by whitespace. Add a round-trip test that checks the
line after a trailing-space secret is still a separate property.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/props.awk:
##########
@@ -0,0 +1,374 @@
+# 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.
+#
+# props.awk — read and rewrite Java ".properties" files with the grammar
+# HugeConfig (commons-configuration over JDK Properties) applies, so the
+# entrypoint and the server agree on what a mounted file means. grep/sed
+# rewrites do not: they see `\`-escaped keys, `:` separators, continuation
+# lines and duplicate definitions differently, which is how a mounted
+# config ends up with two definitions of one key.
+#
+# One invocation, selected with the `PROPS_MODE` environment variable:
+#
+# PROPS_MODE=get PROPS_KEY=K PROPS_FILE=F
+# print the value of K's first logical definition, in the on-disk
+# escaped form; with PROPS_DECODED=1 print it as java.util.Properties
+# would hand it to the server. Always exits 0.
+# PROPS_MODE=has PROPS_KEY=K PROPS_FILE=F
+# print nothing; exit 0 when K has any definition at all, empty
+# included, 1 when it has none, 2 on an error
+# PROPS_MODE=set PROPS_KEY=K PROPS_FILE=F
+# replace K's first definition in place, drop every other
+# definition of K, append one when the file has none. The new
+# value arrives pre-encoded in PROPS_VALUE_ENCODED (an environment
+# variable, so secrets never appear in `ps` output or in awk's
+# argv), and -v is not used for it so awk cannot mangle its
+# backslash escapes.
+#
+# Grammar implemented (java.util.Properties line reader + the
+# first-definition-wins rule Configuration.getString applies):
+# - physical lines end at \r\n, \n or a bare \r, as in java.util.Properties
+# - '#' / '!' comments and blank lines
+# - '=' / ':' / whitespace separators, where the whitespace Java counts is
+# space, tab and form feed, with whitespace then an optional single '=' or
+# ':' accepted as one separator
+# - continuations: a physical line ending in an odd number of
+# backslashes joins the next line (its leading whitespace stripped)
+# - backslash escapes in keys and values, including \uXXXX
+# - duplicate logical keys resolve to the first definition
+#
+# Rewrites keep every untouched line byte-for-byte (comments, blank
+# lines, unrelated entries), and replace the first definition where it
+# stands, so mounted configs stay reviewable in git diffs.
+
+function die(msg) {
+ printf "props.awk: %s\n", msg > "/dev/stderr"
+ # 2 for an error, so a caller that reads exit status 1 as "the key is not
+ # there" (PROPS_MODE=has) cannot mistake an unreadable file for an absent
+ # property and append a definition on top of one it failed to read.
+ exit 2
+}
+
+function hex_digit(c) {
+ return index("0123456789abcdef", tolower(c)) - 1
+}
+
+# \uXXXX is a UTF-16 code unit in Java. Values here are effectively
+# ISO-8859-1, so codes above 0xFF are kept as their literal escape text
+# rather than being mangled through a single-byte sprintf.
+function unescape(s, out, i, n, c, code, j, d, ok) {
+ out = ""
+ n = length(s)
+ for (i = 1; i <= n; i++) {
+ c = substr(s, i, 1)
+ if (c != "\\") { out = out c; continue }
+ if (i == n) break
+ i++
+ c = substr(s, i, 1)
+ if (c == "u" && i + 4 <= n) {
+ code = 0
+ ok = 1
+ for (j = 1; j <= 4; j++) {
+ d = hex_digit(substr(s, i + j, 1))
+ if (d < 0) { ok = 0; break }
+ code = code * 16 + d
+ }
+ if (ok) {
+ i += 4
+ if (code <= 255) out = out sprintf("%c", code)
+ else out = out substr(s, i - 5, 6)
+ continue
+ }
+ }
+ if (c == "t") out = out "\t"
+ else if (c == "n") out = out "\n"
+ else if (c == "r") out = out "\r"
+ else if (c == "f") out = out "\f"
+ else out = out c
+ }
+ return out
+}
+
+# A physical line is continued when it ends in an odd number of
+# backslashes (an even count escapes itself).
+function trailing_backslashes(s, n, k) {
+ n = length(s)
+ k = 0
+ while (k < n && substr(s, n - k, 1) == "\\") k++
+ return k
+}
+
+function is_skipped(raw) {
+ return raw ~ /^[ \t\f]*([#!]|$)/
+}
+
+# Split a logical line into its raw (still-escaped) key and value parts.
+# Results land in K_RAW / V_RAW because awk returns one value.
+# Java treats form feed as whitespace on both sides of the separator, so
+# `auth.authenticator<FF>=...` is one property here too; reading it as part of
+# the key name made a valid mounted configuration invisible to the guards.
+function split_kv(s, n, i, c, esc, sep_at, rest) {
+ n = length(s)
+ esc = 0
+ sep_at = 0
+ for (i = 1; i <= n; i++) {
+ c = substr(s, i, 1)
+ if (esc) { esc = 0; continue }
+ if (c == "\\") { esc = 1; continue }
+ if (c == "=" || c == ":" || c == " " || c == "\t" || c == "\f") {
sep_at = i; break }
+ }
+ if (sep_at == 0) {
+ K_RAW = s
+ V_RAW = ""
+ return
+ }
+ K_RAW = substr(s, 1, sep_at - 1)
+ rest = substr(s, sep_at)
+ c = substr(rest, 1, 1)
+ if (c == "=" || c == ":") {
+ rest = substr(rest, 2)
+ } else {
+ sub(/^[ \t\f]+/, "", rest)
+ c = substr(rest, 1, 1)
+ if (c == "=" || c == ":") rest = substr(rest, 2)
+ }
+ sub(/^[ \t\f]+/, "", rest)
+ V_RAW = rest
+}
+
+function shquote(s) {
+ gsub(/'/, "'\\''", s)
+ return "'" s "'"
+}
+
+# java.util.Properties ends a physical line at \r\n, \n or a bare \r, but
+# getline splits on \n alone. A properties file saved with CR-only endings --
+# which java.util.Properties writes for a lone `store()` on some platforms, and
+# which a mounted config can arrive with -- therefore reached the parser as one
+# enormous record: only its first key was ever seen, and rewriting that key
+# replaced the whole record and dropped every later entry, including
+# auth.authenticator. So the file is re-scanned for terminators here.
+#
+# RAW[] keeps the exact bytes of each line and RAWTERM[] its terminator, so a
+# rewrite still replays untouched lines byte-for-byte. A file whose last line
+# carries no terminator gets a \n, which is what the replay did before.
+function scan_records(s, i, n, c, start, term, len, cnt) {
+ n = length(s)
+ cnt = 0
+ start = 1
+ i = 1
+ while (i <= n) {
+ c = substr(s, i, 1)
+ if (c != "\r" && c != "\n") { i++; continue }
+ if (c == "\r" && substr(s, i + 1, 1) == "\n") {
+ term = "\r\n"
+ len = 2
+ } else {
+ term = c
+ len = 1
+ }
+ cnt++
+ RAW[cnt] = substr(s, start, i - start)
+ RAWTERM[cnt] = term
+ start = i + len
+ i = start
+ }
+ if (start <= n) {
+ cnt++
+ RAW[cnt] = substr(s, start)
+ RAWTERM[cnt] = ""
+ }
+ return cnt
+}
+
+# Load `file` into per-block arrays: one block per comment/blank line or
+# logical entry, spanning exactly the physical lines it occupies.
+function props_load(file, raw, rc, content, nl, stripped, next_raw, start,
logical) {
+ content = ""
+ while ((rc = (getline raw < file)) > 0)
+ content = content raw "\n"
+ if (rc == -1)
+ die("cannot read " file)
+ close(file)
+
+ NLINES = scan_records(content)
+
+ NBLOCK = 0
+ for (nl = 1; nl <= NLINES; nl++) {
+ # RAW[] holds one java.util.Properties physical line with its
terminator
+ # already removed, so no CR stripping is needed here.
+ stripped = RAW[nl]
+ if (is_skipped(stripped)) {
+ NBLOCK++
+ BTYPE[NBLOCK] = "skip"
+ BFIRST[NBLOCK] = nl
+ BLAST[NBLOCK] = nl
+ continue
+ }
+ start = nl
+ logical = stripped
+ while (trailing_backslashes(logical) % 2 == 1 && nl < NLINES) {
+ logical = substr(logical, 1, length(logical) - 1)
+ nl++
+ next_raw = RAW[nl]
+ sub(/^[ \t\f]+/, "", next_raw)
+ logical = logical next_raw
+ }
+ # java.util.Properties ignores whitespace before the key; strip it
+ # so split_kv's separator scan agrees (an indented key used to be
+ # read as a key whose name started with a space, and a set then
+ # appended a second definition of the real key).
+ sub(/^[ \t\f]+/, "", logical)
+ split_kv(logical)
+ NBLOCK++
+ BTYPE[NBLOCK] = "entry"
+ BFIRST[NBLOCK] = start
+ BLAST[NBLOCK] = nl
+ BKEY[NBLOCK] = unescape(K_RAW)
+ # Values stay in their on-disk escaped form. get Prop callers feed
+ # the result straight back into set, which would corrupt a decoded
+ # value by re-writing its backslashes as literals; keys are
+ # unescaped because they are matched against plain names.
+ BVAL[NBLOCK] = V_RAW
+ }
+}
+
+function props_set(file, key, enc_val, tmp, bak, cmd, b, first, ln, msg,
nbs) {
+ props_load(file)
+ # A value whose encoded form ends in an odd number of backslashes would
+ # turn the line written after it into a continuation of that value.
+ # Measured against commons-configuration2 (what HugeConfig extends), the
+ # same input read back yields no property at all, so a secret written this
+ # way would never reach the server that is supposed to authenticate with
+ # it; the entrypoint has to refuse instead of guessing a target.
+ nbs = 0
+ while (nbs < length(enc_val) && substr(enc_val, length(enc_val) - nbs, 1)
== "\\")
+ nbs++
+ if (nbs % 2 == 1)
+ die("refusing to write " key ": the encoded value ends in an odd
number of backslashes")
+ first = 0
+ for (b = 1; b <= NBLOCK; b++) {
+ if (BTYPE[b] == "entry" && BKEY[b] == key) {
+ if (first == 0) first = b
+ else BDROP[b] = 1
+ }
+ }
+ # Staged rewrite: everything lands in a sibling temp file first, so a
+ # failure before the copy-back leaves the original untouched. The temp
+ # file can hold secrets, so it is pre-created 0600 before the first
+ # write: awk's `>` below would otherwise create it under the process
+ # umask (usually 0644), leaving auth.admin_pa or auth.token_secret
+ # briefly group- and world-readable. Truncating an existing file keeps
+ # its mode, and the chmod after close repairs a stale tmp left behind
+ # by a crashed run.
+ tmp = file ".tmp"
+ system("umask 077 && : > " shquote(tmp))
+ for (b = 1; b <= NBLOCK; b++) {
+ if (BDROP[b]) continue
+ if (b == first) {
+ printf "%s=%s\n", key, enc_val > tmp
+ } else {
+ for (ln = BFIRST[b]; ln <= BLAST[b]; ln++) {
+ # Replay the line with the terminator it was read with, so a
+ # CRLF or CR-only config keeps its endings on lines the
+ # rewrite does not touch.
+ msg = RAWTERM[ln]
+ if (msg == "") msg = "\n"
+ printf "%s%s", RAW[ln], msg > tmp
+ }
+ }
+ }
+ if (first == 0)
+ printf "%s=%s\n", key, enc_val > tmp
+ close(tmp)
+ # Copy the completed temp file back onto the original instead of
+ # renaming it: a rename replaces the inode, which would lose the
+ # file's permissions (a 0600 config holding secrets would come back
+ # umask-world-readable), turn a symlinked config into a regular file,
+ # and fail with EBUSY on a config bind-mounted as a single file — the
+ # mounted case this path exists for. The copy keeps the inode, mode,
+ # symlink and mount point.
+ #
+ # The copy itself is not atomic and the shell's `>` truncates the
+ # destination before cat writes a byte, so an ENOSPC or I/O error
+ # mid-copy used to leave a truncated config on disk — a truncated
+ # rest-server.properties loses `auth.authenticator` and boots the
+ # server with authentication off. Snapshot the original first (under
+ # umask 077 so a backup of a 0644 mounted config never ends up more
+ # permissive than it started, and chmodded in case a crashed run left
+ # one behind) and put it back when the copy fails.
+ system("chmod 600 -- " shquote(tmp))
+ bak = file ".bak"
+ cmd = "umask 077 && cp -- " shquote(file) " " shquote(bak)
+ if (system(cmd " && chmod 600 -- " shquote(bak)) != 0)
Review Comment:
Minor: `chmod 600 -- file` fails with BSD chmod, which reads `--` as a file
name after the mode. This file now ships in the release tarball's bin/, so
bin/enable-auth.sh fails on macOS. On macOS (/bin/chmod), enable-auth.sh
against the shipped conf/ printed `chmod: --: No such file or directory` and
`props.awk: cannot back up .../rest-server.properties before the copy-back`,
then exited 1. By then it had already appended the authentication block to
gremlin-server.yaml and left `rest-server.properties.tmp` and `.bak` in conf/.
The master script also failed on macOS (BSD sed rejects `$a\`), but it wrote
nothing. Requested change: drop the `--` from the two chmod calls here and on
line 312. The paths are already quoted by shquote and never start with `-` in
these calls.
--
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]
