From 20e1a8c6feacf2df57c6f05b4b5636da68d4d552 Mon Sep 17 00:00:00 2001 From: arshil-khan Date: Tue, 13 Oct 2026 12:54:41 +0530 Subject: [PATCH 1/5] feat(server): read the initial admin password from a file auth.admin_pa goes through the properties grammar, which trims the value, processes backslash escapes and decodes it as ISO-8859-1. A padded, backslashed or non-ASCII password can be stored differently from what the operator wrote. auth.admin_pa_file names a file that is read as raw UTF-8, with one trailing line break dropped. When set, it replaces auth.admin_pa on the PD startup path and the in-memory backend path, and it replaces the stdin prompt in init-store. A missing, empty or non-UTF-8 file stops startup with an error naming the option; the server never falls back to the 'pa' default. With init-store disabled, init-store reads the file up front so a bad path fails before the server starts. Fixes #3285 --- docker/README.md | 5 + .../hugegraph/auth/HugeFactoryAuthProxy.java | 3 +- .../hugegraph/auth/StandardAuthenticator.java | 64 +++++++- .../hugegraph/config/ServerOptions.java | 21 ++- .../apache/hugegraph/core/GraphManager.java | 7 +- .../static/conf/rest-server.properties | 3 + .../org/apache/hugegraph/cmd/InitStore.java | 11 +- .../apache/hugegraph/unit/UnitTestSuite.java | 2 + .../unit/auth/AdminPasswordFileTest.java | 155 ++++++++++++++++++ .../unit/cmd/InitStoreConfigTest.java | 41 +++++ 10 files changed, 300 insertions(+), 12 deletions(-) create mode 100644 hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/AdminPasswordFileTest.java diff --git a/docker/README.md b/docker/README.md index 869b5d819e..1d607fb1b7 100644 --- a/docker/README.md +++ b/docker/README.md @@ -59,6 +59,11 @@ compatible with the existing authentication protocol and the chart's documented password constraints. When the signing-key environment variable is omitted, the image reuses the configured key instead of rotating it. +Instead of `auth.admin_pa`, a REST configuration you mount yourself can set +`auth.admin_pa_file` to the path of a mounted secret. The Server reads that file as +raw UTF-8 and drops one trailing line break, so the password needs no properties +escaping. When set, it overrides `auth.admin_pa` and the init-store password prompt. + Older images could change signing keys containing spaces or Unicode, and a trailing space could hide the following configuration property. On upgrade, explicitly provide the intended key for those deployments. Tokens signed with diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeFactoryAuthProxy.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeFactoryAuthProxy.java index 98f9ea8bc4..8b7feaa40f 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeFactoryAuthProxy.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeFactoryAuthProxy.java @@ -130,7 +130,8 @@ private static void registerPrivateActions() { // HugeGraph Reflection.registerFieldsToFilter(StandardAuthenticator.class, "graph"); Reflection.registerMethodsToFilter(StandardAuthenticator.class, "initAdminUser", - "inputPassword", "graph"); + "inputPassword", "graph", "initialAdminPassword", + "readAdminPasswordFile"); Reflection.registerFieldsToFilter(HugeFactoryAuthProxy.class, "PROTECT_METHODS"); Reflection.registerMethodsToFilter(HugeFactoryAuthProxy.class, "genRegisterPrivateActions", "registerClass", "registerPrivateActions", diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java index f64ead4fb2..4bad6384e8 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java @@ -18,8 +18,14 @@ package org.apache.hugegraph.auth; import java.io.Console; +import java.io.IOException; import java.net.InetAddress; +import java.nio.ByteBuffer; +import java.nio.charset.CharacterCodingException; +import java.nio.charset.CodingErrorAction; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; import java.util.Arrays; import java.util.HashMap; import java.util.Map; @@ -27,6 +33,7 @@ import org.apache.commons.lang.StringUtils; import org.apache.hugegraph.HugeGraph; +import org.apache.hugegraph.config.ConfigException; import org.apache.hugegraph.config.CoreOptions; import org.apache.hugegraph.config.HugeConfig; import org.apache.hugegraph.config.ServerOptions; @@ -46,9 +53,11 @@ public class StandardAuthenticator implements HugeAuthenticator { private HugeGraph graph = null; - private void initAdminUser() throws Exception { + private void initAdminUser(HugeConfig config) throws Exception { if (this.requireInitAdminUser()) { - this.initAdminUser(this.inputPassword()); + String file = config.get(ServerOptions.ADMIN_PA_FILE); + this.initAdminUser(file.isEmpty() ? this.inputPassword() : + readAdminPasswordFile(file)); } this.graph.close(); } @@ -202,10 +211,59 @@ public static void initAdminUserIfNeeded(String confFile) throws Exception { config.addProperty(INITING_STORE, true); auth.setup(config); if (auth.graph().backendStoreFeatures().supportsPersistence()) { - auth.initAdminUser(); + auth.initAdminUser(config); } } + /** + * The initial admin password for the bootstrap paths that cannot prompt. + * auth.admin_pa goes through the properties grammar, which trims it, + * processes backslash escapes and decodes it as ISO-8859-1, so + * auth.admin_pa_file takes precedence when both are set. + */ + public static String initialAdminPassword(HugeConfig config) { + String file = config.get(ServerOptions.ADMIN_PA_FILE); + if (!file.isEmpty()) { + return readAdminPasswordFile(file); + } + return config.get(ServerOptions.ADMIN_PA); + } + + private static String readAdminPasswordFile(String file) { + String option = ServerOptions.ADMIN_PA_FILE.name(); + byte[] bytes; + try { + bytes = Files.readAllBytes(Paths.get(file)); + } catch (IOException e) { + throw new ConfigException("Failed to read the file '%s' set by '%s'", + e, file, option); + } + + String password; + try { + password = StandardCharsets.UTF_8.newDecoder() + .onMalformedInput(CodingErrorAction.REPORT) + .onUnmappableCharacter(CodingErrorAction.REPORT) + .decode(ByteBuffer.wrap(bytes)) + .toString(); + } catch (CharacterCodingException e) { + throw new ConfigException("The file '%s' set by '%s' is not valid UTF-8", + e, file, option); + } + + // Drop the line break that echo, editors and most secret tools append + if (password.endsWith("\r\n")) { + password = password.substring(0, password.length() - 2); + } else if (password.endsWith("\n")) { + password = password.substring(0, password.length() - 1); + } + if (password.isEmpty()) { + throw new ConfigException("The file '%s' set by '%s' is empty", + file, option); + } + return password; + } + private class TokenSaslAuthenticator implements SaslNegotiator { private static final byte NUL = 0; diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java index 285c4b23a5..0f7790c21a 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java @@ -480,11 +480,10 @@ public class ServerOptions extends OptionHolder { "" ); - // TODO: accept the initial admin password from the environment (or document the properties - // contract): read through PropertiesConfiguration, the value is trimmed, backslash-unescaped - // and decoded as ISO-8859-1, so the stored password can differ from what the operator set. - // The Helm chart (helm/hugegraph) refuses padded, backslash or non-ASCII admin passwords in - // its schema and Server wrapper; relax that guard once the credential bypasses the file. + // Read through PropertiesConfiguration: the value is trimmed, backslash-unescaped and decoded + // as ISO-8859-1, so a password outside printable ASCII needs Unicode escapes in the file. + // ADMIN_PA_FILE skips that grammar. The Helm chart (helm/hugegraph) still refuses padded, + // backslash or non-ASCII admin passwords; relax that guard once it passes ADMIN_PA_FILE. public static final ConfigOption ADMIN_PA = new ConfigOption<>( "auth.admin_pa", @@ -494,6 +493,18 @@ public class ServerOptions extends OptionHolder { "pa" ); + public static final ConfigOption ADMIN_PA_FILE = + new ConfigOption<>( + "auth.admin_pa_file", + "The path of a file holding the default password for " + + "built-in admin account, read as raw UTF-8 with one " + + "trailing line break removed. When set, it takes " + + "precedence over auth.admin_pa and the init-store prompt; " + + "takes effect on first startup.", + null, + "" + ); + public static final ConfigOption SSL_KEYSTORE_FILE = new ConfigOption<>( "ssl.keystore_file", diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java index c7d821bf38..ae5d597fdf 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java @@ -374,7 +374,8 @@ private void loadMetaFromPD() { this.initK8sManagerIfNeeded(conf); if (shouldBootstrapAdmin(this.authenticator, conf.get(ServerOptions.AUTH_REMOTE_URL))) { - this.initAdminUserIfNeeded(conf.get(ServerOptions.ADMIN_PA)); + this.initAdminUserIfNeeded( + StandardAuthenticator.initialAdminPassword(conf)); } this.createDefaultGraphSpaceIfNeeded(conf); @@ -1877,7 +1878,9 @@ private void checkBackendVersionOrExit(HugeConfig config) { if (!hugegraph.backendStoreFeatures().supportsPersistence()) { hugegraph.initBackend(); if (this.requireAuthentication()) { - String adminPassword = config.get(ServerOptions.ADMIN_PA); + // Outside the try so a bad password file names itself + String adminPassword = + StandardAuthenticator.initialAdminPassword(config); try { this.authenticator().initAdminUser(adminPassword); } catch (Exception e) { diff --git a/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties b/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties index daeb092b77..f4aa34cf22 100644 --- a/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties +++ b/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties @@ -21,6 +21,9 @@ arthas.disabled_commands=jad #auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator # for admin password, By default, it is pa and takes effect upon the first startup #auth.admin_pa=pa +# or read it from a file as raw UTF-8 (one trailing line break is dropped), so +# spaces, backslashes and non-ASCII characters need no escaping; overrides auth.admin_pa +#auth.admin_pa_file=/run/secrets/hugegraph-admin-password #auth.graph_store=hugegraph # use pd diff --git a/hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java b/hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java index 5362a1821d..517c9d5da5 100644 --- a/hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java +++ b/hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java @@ -223,13 +223,22 @@ private static void checkAdminBootstrapReachable(HugeConfig conf, "', not 'hstore'"); } + // A password file is read now, so an unreadable or empty one fails + // here instead of after the server has started + if (!conf.get(ServerOptions.ADMIN_PA_FILE).isEmpty()) { + StandardAuthenticator.initialAdminPassword(conf); + return; + } + // The server creates the admin from this value and cannot prompt for // it, and Docker PASSWORD never reaches this path. An absent or empty // one would hand out the public 'pa' default, so fail instead. Checked // with containsKey because the default is not a configured secret. if (!conf.containsKey(ServerOptions.ADMIN_PA.name()) || conf.get(ServerOptions.ADMIN_PA).isEmpty()) { - throw unreachableAdmin(restConf, "no explicit non-empty '" + + throw unreachableAdmin(restConf, "neither '" + + ServerOptions.ADMIN_PA_FILE.name() + + "' nor an explicit non-empty '" + ServerOptions.ADMIN_PA.name() + "' is configured, so the admin " + "would be created with the " + diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java index 3cd65b0d8f..1a31cba837 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java @@ -50,6 +50,7 @@ import org.apache.hugegraph.unit.api.gremlin.GremlinQueryAPITest; import org.apache.hugegraph.unit.api.space.GraphSpaceAPITest; import org.apache.hugegraph.unit.api.space.SchemaTemplateAPITest; +import org.apache.hugegraph.unit.auth.AdminPasswordFileTest; import org.apache.hugegraph.unit.auth.HugeGraphAuthProxyTest; import org.apache.hugegraph.unit.cache.CacheManagerTest; import org.apache.hugegraph.unit.cache.CacheTest; @@ -227,6 +228,7 @@ ServerInfoManagerTest.class, TaskSchedulerServerInfoTest.class, HugeGraphAuthProxyTest.class, + AdminPasswordFileTest.class, SchemaElementTest.class, HugeGraphTestInfrastructureTest.class, HugeElementViewTest.class, diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/AdminPasswordFileTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/AdminPasswordFileTest.java new file mode 100644 index 0000000000..788f655ed7 --- /dev/null +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/AdminPasswordFileTest.java @@ -0,0 +1,155 @@ +/* + * 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. + */ + +package org.apache.hugegraph.unit.auth; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.Comparator; +import java.util.stream.Stream; + +import org.apache.commons.configuration2.PropertiesConfiguration; +import org.apache.hugegraph.auth.StandardAuthenticator; +import org.apache.hugegraph.config.ConfigException; +import org.apache.hugegraph.config.HugeConfig; +import org.apache.hugegraph.config.ServerOptions; +import org.apache.hugegraph.dist.RegisterUtil; +import org.apache.hugegraph.testutil.Assert; +import org.junit.After; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +/** + * auth.admin_pa goes through the properties grammar, so a padded, escaped or + * non-ASCII password can reach the server changed. auth.admin_pa_file is read + * as raw UTF-8 and must hand back exactly what the operator wrote. + */ +public class AdminPasswordFileTest { + + private Path workDir; + + @BeforeClass + public static void registerOptions() { + RegisterUtil.registerServer(); + } + + @Before + public void setup() throws IOException { + this.workDir = Files.createTempDirectory("admin-password-file-test"); + } + + @After + public void teardown() throws IOException { + try (Stream paths = Files.walk(this.workDir)) { + for (Path path : paths.sorted(Comparator.reverseOrder()) + .toArray(Path[]::new)) { + Files.deleteIfExists(path); + } + } + } + + @Test + public void testFallsBackToAdminPa() { + Assert.assertEquals("pa", initialAdminPassword(null, null)); + Assert.assertEquals("secret", initialAdminPassword(null, "secret")); + } + + @Test + public void testFileKeepsCharactersThePropertiesReaderChanges() + throws IOException { + for (String password : Arrays.asList(" padded ", "two\\back", + "x\ty", "pässword", + "密码 ☃", "a:b=c#d")) { + Path file = this.write(password.getBytes(StandardCharsets.UTF_8)); + Assert.assertEquals(password, initialAdminPassword(file, null)); + } + } + + @Test + public void testFileTakesPrecedenceOverAdminPa() throws IOException { + Path file = this.write("from-file".getBytes(StandardCharsets.UTF_8)); + Assert.assertEquals("from-file", initialAdminPassword(file, "from-pa")); + } + + @Test + public void testFileDropsOneTrailingLineBreak() throws IOException { + Assert.assertEquals("secret", initialAdminPassword( + this.write(utf8("secret\n")), null)); + Assert.assertEquals("secret", initialAdminPassword( + this.write(utf8("secret\r\n")), null)); + Assert.assertEquals("secret\n", initialAdminPassword( + this.write(utf8("secret\n\n")), null)); + Assert.assertEquals("secret\r", initialAdminPassword( + this.write(utf8("secret\r")), null)); + } + + @Test + public void testEmptyFileFails() throws IOException { + for (String content : Arrays.asList("", "\n", "\r\n")) { + Path file = this.write(utf8(content)); + Assert.assertThrows(ConfigException.class, () -> { + initialAdminPassword(file, "secret"); + }, e -> Assert.assertContains("is empty", e.getMessage())); + } + } + + @Test + public void testMissingFileFails() { + Path file = this.workDir.resolve("missing"); + Assert.assertThrows(ConfigException.class, () -> { + initialAdminPassword(file, "secret"); + }, e -> Assert.assertContains("Failed to read", e.getMessage())); + } + + /** + * Guessing another charset is what turned 'pässword' into 'pässword' + * in the first place, so bytes that are not UTF-8 are refused. + */ + @Test + public void testNonUtf8FileFails() throws IOException { + Path file = this.write("pässword".getBytes(StandardCharsets.ISO_8859_1)); + Assert.assertThrows(ConfigException.class, () -> { + initialAdminPassword(file, null); + }, e -> Assert.assertContains("not valid UTF-8", e.getMessage())); + } + + private static byte[] utf8(String value) { + return value.getBytes(StandardCharsets.UTF_8); + } + + private Path write(byte[] content) throws IOException { + Path file = Files.createTempFile(this.workDir, "admin", ".pa"); + Files.write(file, content); + return file; + } + + private static String initialAdminPassword(Path file, String adminPa) { + PropertiesConfiguration properties = new PropertiesConfiguration(); + if (file != null) { + properties.setProperty(ServerOptions.ADMIN_PA_FILE.name(), + file.toString()); + } + if (adminPa != null) { + properties.setProperty(ServerOptions.ADMIN_PA.name(), adminPa); + } + return StandardAuthenticator.initialAdminPassword(new HugeConfig(properties)); + } +} diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/cmd/InitStoreConfigTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/cmd/InitStoreConfigTest.java index ba88ec7224..fb83fcbe9e 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/cmd/InitStoreConfigTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/cmd/InitStoreConfigTest.java @@ -395,6 +395,47 @@ public void testDisabledInitStoreRejectsDefaultAdminPassword() } } + @Test + public void testDisabledInitStoreAllowsAdminPasswordFile() + throws Exception { + Path graphsDir = this.writeAuthGraphConfig("hstore"); + Path passwordFile = this.workDir.resolve("admin.pa"); + Files.write(passwordFile, "p\u00e4ssword\n".getBytes(StandardCharsets.UTF_8)); + String restConf = this.writeDisabledRestServerConfForGraphs( + graphsDir, + ServerOptions.AUTHENTICATOR.name() + + "=" + StandardAuthenticator.class.getName(), + ServerOptions.AUTH_GRAPH_STORE.name() + "=hugegraph", + ServerOptions.USE_PD.name() + "=true", + ServerOptions.ADMIN_PA_FILE.name() + "=" + passwordFile); + + InitStore.main(new String[]{restConf}); + } + + /** + * The server would only read the file after it starts, so init-store + * reads it first and fails on a file the server could not use. + */ + @Test + public void testDisabledInitStoreRejectsUnreadableAdminPasswordFile() + throws IOException { + Path graphsDir = this.writeAuthGraphConfig("hstore"); + String restConf = this.writeDisabledRestServerConfForGraphs( + graphsDir, + ServerOptions.AUTHENTICATOR.name() + + "=" + StandardAuthenticator.class.getName(), + ServerOptions.AUTH_GRAPH_STORE.name() + "=hugegraph", + ServerOptions.USE_PD.name() + "=true", + ServerOptions.ADMIN_PA.name() + "=secret", + ServerOptions.ADMIN_PA_FILE.name() + "=" + + this.workDir.resolve("missing.pa")); + + Assert.assertThrows(ConfigException.class, () -> { + InitStore.main(new String[]{restConf}); + }, e -> Assert.assertContains(ServerOptions.ADMIN_PA_FILE.name(), + e.getMessage())); + } + /** * Remote auth delegates to another service, so there is no local admin to * create and the check above must not fire. From 09488c21705368f116edd8d7f26170364c5d655c Mon Sep 17 00:00:00 2001 From: arshil-khan Date: Tue, 13 Oct 2026 14:15:58 +0530 Subject: [PATCH 2/5] test(server): cover init-store reading the admin password file init-store prompts for the admin password on stdin when the backend is persistent. Pin that a configured auth.admin_pa_file replaces the prompt and the admin is created with the file's password. --- .../unit/auth/AdminPasswordFileTest.java | 38 ++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/AdminPasswordFileTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/AdminPasswordFileTest.java index 788f655ed7..0a94cceb48 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/AdminPasswordFileTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/AdminPasswordFileTest.java @@ -26,16 +26,23 @@ import java.util.stream.Stream; import org.apache.commons.configuration2.PropertiesConfiguration; +import org.apache.hugegraph.HugeGraph; +import org.apache.hugegraph.auth.HugeUser; +import org.apache.hugegraph.auth.StandardAuthManager; import org.apache.hugegraph.auth.StandardAuthenticator; import org.apache.hugegraph.config.ConfigException; import org.apache.hugegraph.config.HugeConfig; import org.apache.hugegraph.config.ServerOptions; import org.apache.hugegraph.dist.RegisterUtil; import org.apache.hugegraph.testutil.Assert; +import org.apache.hugegraph.testutil.Whitebox; +import org.apache.hugegraph.util.StringEncoding; import org.junit.After; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; /** * auth.admin_pa goes through the properties grammar, so a padded, escaped or @@ -131,6 +138,31 @@ public void testNonUtf8FileFails() throws IOException { }, e -> Assert.assertContains("not valid UTF-8", e.getMessage())); } + /** + * init-store prompts on stdin for a persistent backend. With a password + * file configured it must create the admin from the file instead. + */ + @Test + public void testInitStoreTakesFileInsteadOfPrompt() throws Exception { + Path file = this.write(utf8("p\u00e4ssword\n")); + StandardAuthManager authManager = Mockito.mock(StandardAuthManager.class); + HugeGraph graph = Mockito.mock(HugeGraph.class); + Mockito.when(graph.hugegraph()).thenReturn(graph); + Mockito.when(graph.authManager()).thenReturn(authManager); + StandardAuthenticator authenticator = new StandardAuthenticator(); + Whitebox.setInternalState(authenticator, "graph", graph); + + Whitebox.invoke(StandardAuthenticator.class, + new Class[]{HugeConfig.class}, "initAdminUser", + authenticator, config(file, null)); + + ArgumentCaptor admin = ArgumentCaptor.forClass(HugeUser.class); + Mockito.verify(authManager).createUser(admin.capture()); + Assert.assertTrue(StringEncoding.checkPassword("p\u00e4ssword", + admin.getValue().password())); + Mockito.verify(graph).close(); + } + private static byte[] utf8(String value) { return value.getBytes(StandardCharsets.UTF_8); } @@ -142,6 +174,10 @@ private Path write(byte[] content) throws IOException { } private static String initialAdminPassword(Path file, String adminPa) { + return StandardAuthenticator.initialAdminPassword(config(file, adminPa)); + } + + private static HugeConfig config(Path file, String adminPa) { PropertiesConfiguration properties = new PropertiesConfiguration(); if (file != null) { properties.setProperty(ServerOptions.ADMIN_PA_FILE.name(), @@ -150,6 +186,6 @@ private static String initialAdminPassword(Path file, String adminPa) { if (adminPa != null) { properties.setProperty(ServerOptions.ADMIN_PA.name(), adminPa); } - return StandardAuthenticator.initialAdminPassword(new HugeConfig(properties)); + return new HugeConfig(properties); } } From 128c0b320fcf54af73228f5cf0fec3394a9c3d4f Mon Sep 17 00:00:00 2001 From: arshil-khan Date: Tue, 13 Oct 2026 14:40:46 +0530 Subject: [PATCH 3/5] fix(server): tighten reading of the admin password file Strip a leading UTF-8 BOM. PowerShell and some Windows editors write one, and if kept it becomes an invisible first character of the admin password. Read at most 4096 bytes, so a path that points at a log or a device fails instead of being loaded whole, and report an invalid path as a ConfigException. When the file is refused, init-store now closes the auth graph and keeps the file error even if the close also fails. The BackendException raised when a non-persistent backend can't create the admin now carries its cause. Document that PD and non-persistent backends read the file on every start, and that a disabled init-store still needs this option or an explicit auth.admin_pa. The init-store tests run with an empty stdin, so falling back to the prompt fails instead of hanging. New tests cover the BOM, the size cap, closing the graph on a refused file, and the GraphManager non-persistent path. --- docker/README.md | 8 +- .../hugegraph/auth/StandardAuthenticator.java | 43 ++++++-- .../hugegraph/config/ServerOptions.java | 17 +-- .../apache/hugegraph/core/GraphManager.java | 2 +- .../static/conf/rest-server.properties | 5 +- .../unit/auth/AdminPasswordFileTest.java | 103 ++++++++++++++++-- .../unit/core/GraphManagerAdminInitTest.java | 84 ++++++++++++++ 7 files changed, 233 insertions(+), 29 deletions(-) diff --git a/docker/README.md b/docker/README.md index 1d607fb1b7..f5cde2f12a 100644 --- a/docker/README.md +++ b/docker/README.md @@ -61,8 +61,12 @@ the image reuses the configured key instead of rotating it. Instead of `auth.admin_pa`, a REST configuration you mount yourself can set `auth.admin_pa_file` to the path of a mounted secret. The Server reads that file as -raw UTF-8 and drops one trailing line break, so the password needs no properties -escaping. When set, it overrides `auth.admin_pa` and the init-store password prompt. +raw UTF-8 and drops a leading BOM and one trailing line break, so the password +needs no properties escaping. When set, it overrides `auth.admin_pa` and the +init-store password prompt. With PD (hstore), including when init-store is +disabled, the file is read on every start, so keep the secret mounted for as long +as the option is set. With init-store disabled, startup also requires this option +or an explicit non-empty `auth.admin_pa`, so do not drop both. Older images could change signing keys containing spaces or Unicode, and a trailing space could hide the following configuration property. On upgrade, diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java index 4bad6384e8..d63b292aa7 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java @@ -19,12 +19,14 @@ import java.io.Console; import java.io.IOException; +import java.io.InputStream; import java.net.InetAddress; import java.nio.ByteBuffer; import java.nio.charset.CharacterCodingException; import java.nio.charset.CodingErrorAction; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.InvalidPathException; import java.nio.file.Paths; import java.util.Arrays; import java.util.HashMap; @@ -50,14 +52,27 @@ public class StandardAuthenticator implements HugeAuthenticator { private static final String INITING_STORE = "initing_store"; + private static final int MAX_ADMIN_PA_FILE_BYTES = 4096; private HugeGraph graph = null; private void initAdminUser(HugeConfig config) throws Exception { - if (this.requireInitAdminUser()) { - String file = config.get(ServerOptions.ADMIN_PA_FILE); - this.initAdminUser(file.isEmpty() ? this.inputPassword() : - readAdminPasswordFile(file)); + // init-store only closes the graphs it initialized and skips hstore + // ones, so close the auth graph here even when the password file is + // refused, without letting a close failure hide that error + try { + if (this.requireInitAdminUser()) { + String file = config.get(ServerOptions.ADMIN_PA_FILE); + this.initAdminUser(file.isEmpty() ? this.inputPassword() : + readAdminPasswordFile(file)); + } + } catch (Throwable e) { + try { + this.graph.close(); + } catch (Throwable closeError) { + e.addSuppressed(closeError); + } + throw e; } this.graph.close(); } @@ -232,11 +247,16 @@ public static String initialAdminPassword(HugeConfig config) { private static String readAdminPasswordFile(String file) { String option = ServerOptions.ADMIN_PA_FILE.name(); byte[] bytes; - try { - bytes = Files.readAllBytes(Paths.get(file)); - } catch (IOException e) { - throw new ConfigException("Failed to read the file '%s' set by '%s'", - e, file, option); + // Bounded, so a path that points at a log or a device fails quickly + try (InputStream in = Files.newInputStream(Paths.get(file))) { + bytes = in.readNBytes(MAX_ADMIN_PA_FILE_BYTES + 1); + } catch (IOException | InvalidPathException e) { + throw new ConfigException("Failed to read the file '%s' set by '%s' (%s)", + e, file, option, e.getClass().getSimpleName()); + } + if (bytes.length > MAX_ADMIN_PA_FILE_BYTES) { + throw new ConfigException("The file '%s' set by '%s' is larger than %s bytes", + file, option, MAX_ADMIN_PA_FILE_BYTES); } String password; @@ -251,6 +271,11 @@ private static String readAdminPasswordFile(String file) { e, file, option); } + // Some Windows editors and PowerShell prepend a BOM; it is never meant + // as part of the password and would make the admin unreachable + if (password.startsWith("\uFEFF")) { + password = password.substring(1); + } // Drop the line break that echo, editors and most secret tools append if (password.endsWith("\r\n")) { password = password.substring(0, password.length() - 2); diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java index 0f7790c21a..ab822162e9 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java @@ -481,9 +481,10 @@ public class ServerOptions extends OptionHolder { ); // Read through PropertiesConfiguration: the value is trimmed, backslash-unescaped and decoded - // as ISO-8859-1, so a password outside printable ASCII needs Unicode escapes in the file. - // ADMIN_PA_FILE skips that grammar. The Helm chart (helm/hugegraph) still refuses padded, - // backslash or non-ASCII admin passwords; relax that guard once it passes ADMIN_PA_FILE. + // as ISO-8859-1, so leading or trailing spaces, backslashes and characters outside printable + // ASCII need escaping in the file. ADMIN_PA_FILE skips that grammar. The Helm chart + // (helm/hugegraph) still refuses padded, backslash or non-ASCII admin passwords; relax that + // guard once it passes ADMIN_PA_FILE. public static final ConfigOption ADMIN_PA = new ConfigOption<>( "auth.admin_pa", @@ -497,10 +498,12 @@ public class ServerOptions extends OptionHolder { new ConfigOption<>( "auth.admin_pa_file", "The path of a file holding the default password for " + - "built-in admin account, read as raw UTF-8 with one " + - "trailing line break removed. When set, it takes " + - "precedence over auth.admin_pa and the init-store prompt; " + - "takes effect on first startup.", + "built-in admin account, read as raw UTF-8 with a " + + "leading BOM and one trailing line break removed. When " + + "set, it takes precedence over auth.admin_pa and the " + + "init-store prompt; takes effect on first startup. PD " + + "and non-persistent backends read it on every start, so " + + "keep it readable while this option is set.", null, "" ); diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java index ae5d597fdf..d038b7961b 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java @@ -1886,7 +1886,7 @@ private void checkBackendVersionOrExit(HugeConfig config) { } catch (Exception e) { throw new BackendException( "The backend store of '%s' can't " + - "initialize admin user", hugegraph.spaceGraphName()); + "initialize admin user", e, hugegraph.spaceGraphName()); } } } diff --git a/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties b/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties index f4aa34cf22..4bdc3fa606 100644 --- a/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties +++ b/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties @@ -21,8 +21,9 @@ arthas.disabled_commands=jad #auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator # for admin password, By default, it is pa and takes effect upon the first startup #auth.admin_pa=pa -# or read it from a file as raw UTF-8 (one trailing line break is dropped), so -# spaces, backslashes and non-ASCII characters need no escaping; overrides auth.admin_pa +# or read it from a file as raw UTF-8 (a leading BOM and one trailing line break are dropped), so +# spaces, backslashes and non-ASCII characters need no escaping; overrides auth.admin_pa. +# PD and non-persistent backends read it on every start, so keep it readable while set #auth.admin_pa_file=/run/secrets/hugegraph-admin-password #auth.graph_store=hugegraph diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/AdminPasswordFileTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/AdminPasswordFileTest.java index 0a94cceb48..2e3f941d1b 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/AdminPasswordFileTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/AdminPasswordFileTest.java @@ -17,7 +17,9 @@ package org.apache.hugegraph.unit.auth; +import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.InputStream; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; @@ -138,6 +140,35 @@ public void testNonUtf8FileFails() throws IOException { }, e -> Assert.assertContains("not valid UTF-8", e.getMessage())); } + /** + * PowerShell and some Windows editors write a BOM before UTF-8 text. Kept, + * it would become an invisible first character of the admin password. + */ + @Test + public void testFileDropsLeadingBom() throws IOException { + Assert.assertEquals("secret", initialAdminPassword( + this.write(utf8("\ufeffsecret\r\n")), null)); + Assert.assertThrows(ConfigException.class, () -> { + initialAdminPassword(this.write(utf8("\ufeff\n")), "secret"); + }, e -> Assert.assertContains("is empty", e.getMessage())); + } + + /** + * A path pointing at a log or a device must not be read whole into memory. + */ + @Test + public void testOversizedFileFails() throws IOException { + byte[] limit = new byte[4096]; + Arrays.fill(limit, (byte) 'a'); + Assert.assertEquals(4096, initialAdminPassword( + this.write(limit), null).length()); + + Path file = this.write(Arrays.copyOf(limit, limit.length + 1)); + Assert.assertThrows(ConfigException.class, () -> { + initialAdminPassword(file, "secret"); + }, e -> Assert.assertContains("larger than 4096 bytes", e.getMessage())); + } + /** * init-store prompts on stdin for a persistent backend. With a password * file configured it must create the admin from the file instead. @@ -146,15 +177,9 @@ public void testNonUtf8FileFails() throws IOException { public void testInitStoreTakesFileInsteadOfPrompt() throws Exception { Path file = this.write(utf8("p\u00e4ssword\n")); StandardAuthManager authManager = Mockito.mock(StandardAuthManager.class); - HugeGraph graph = Mockito.mock(HugeGraph.class); - Mockito.when(graph.hugegraph()).thenReturn(graph); - Mockito.when(graph.authManager()).thenReturn(authManager); - StandardAuthenticator authenticator = new StandardAuthenticator(); - Whitebox.setInternalState(authenticator, "graph", graph); + HugeGraph graph = mockAuthGraph(authManager); - Whitebox.invoke(StandardAuthenticator.class, - new Class[]{HugeConfig.class}, "initAdminUser", - authenticator, config(file, null)); + initStoreAdminUser(graph, config(file, null)); ArgumentCaptor admin = ArgumentCaptor.forClass(HugeUser.class); Mockito.verify(authManager).createUser(admin.capture()); @@ -163,6 +188,68 @@ public void testInitStoreTakesFileInsteadOfPrompt() throws Exception { Mockito.verify(graph).close(); } + /** + * init-store does not close an hstore auth graph itself, so a refused + * file must still close it rather than leave the backend open on exit. + */ + @Test + public void testInitStoreClosesGraphWhenFileIsRefused() throws Exception { + StandardAuthManager authManager = Mockito.mock(StandardAuthManager.class); + HugeGraph graph = mockAuthGraph(authManager); + HugeConfig config = config(this.workDir.resolve("missing"), "secret"); + + Assert.assertThrows(ConfigException.class, () -> { + initStoreAdminUser(graph, config); + }, e -> Assert.assertContains("Failed to read", e.getMessage())); + Mockito.verify(authManager, Mockito.never()).createUser(Mockito.any()); + Mockito.verify(graph).close(); + } + + /** + * The refused file is what the operator has to fix, so a failure while + * closing the graph afterwards must not replace it. + */ + @Test + public void testInitStoreCloseFailureKeepsFileError() throws Exception { + StandardAuthManager authManager = Mockito.mock(StandardAuthManager.class); + HugeGraph graph = mockAuthGraph(authManager); + IllegalStateException closeError = new IllegalStateException("close"); + Mockito.doThrow(closeError).when(graph).close(); + HugeConfig config = config(this.workDir.resolve("missing"), "secret"); + + Assert.assertThrows(ConfigException.class, () -> { + initStoreAdminUser(graph, config); + }, e -> { + Assert.assertContains("Failed to read", e.getMessage()); + Assert.assertEquals(closeError, e.getSuppressed()[0]); + }); + } + + private static HugeGraph mockAuthGraph(StandardAuthManager authManager) { + HugeGraph graph = Mockito.mock(HugeGraph.class); + Mockito.when(graph.hugegraph()).thenReturn(graph); + Mockito.when(graph.authManager()).thenReturn(authManager); + return graph; + } + + /** + * stdin is empty, so a regression that falls back to the prompt fails + * with NoSuchElementException instead of blocking the test run. + */ + private static void initStoreAdminUser(HugeGraph graph, HugeConfig config) { + StandardAuthenticator authenticator = new StandardAuthenticator(); + Whitebox.setInternalState(authenticator, "graph", graph); + InputStream stdin = System.in; + System.setIn(new ByteArrayInputStream(new byte[0])); + try { + Whitebox.invoke(StandardAuthenticator.class, + new Class[]{HugeConfig.class}, "initAdminUser", + authenticator, config); + } finally { + System.setIn(stdin); + } + } + private static byte[] utf8(String value) { return value.getBytes(StandardCharsets.UTF_8); } diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerAdminInitTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerAdminInitTest.java index 490ffb9920..360d5c38b2 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerAdminInitTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerAdminInitTest.java @@ -18,15 +18,23 @@ package org.apache.hugegraph.unit.core; import java.lang.reflect.Field; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.HashMap; import java.util.Map; import org.apache.commons.configuration2.PropertiesConfiguration; +import org.apache.hugegraph.HugeGraph; import org.apache.hugegraph.exception.HugeException; import org.apache.hugegraph.auth.HugeAuthenticator; import org.apache.hugegraph.auth.HugeUser; import org.apache.hugegraph.auth.StandardAuthenticator; +import org.apache.hugegraph.backend.store.BackendFeatures; +import org.apache.hugegraph.backend.store.BackendStoreInfo; +import org.apache.hugegraph.config.ConfigException; import org.apache.hugegraph.config.HugeConfig; +import org.apache.hugegraph.config.ServerOptions; import org.apache.hugegraph.core.GraphManager; import org.apache.hugegraph.event.EventHub; import org.apache.hugegraph.meta.MetaDriver; @@ -35,6 +43,7 @@ import org.apache.hugegraph.meta.managers.SpaceMetaManager; import org.apache.hugegraph.testutil.Assert; import org.apache.hugegraph.testutil.Whitebox; +import org.apache.tinkerpop.gremlin.structure.Graph; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -154,6 +163,81 @@ public void testAdminBootstrapOnlyUsesLocalStandardAuthenticator() { Mockito.mock(HugeAuthenticator.class), "")); } + /** + * Non-persistent backends bootstrap the admin on every start, so + * auth.admin_pa_file has to reach that path as well as the PD one. + */ + @Test + public void testNonPersistentBackendTakesAdminPasswordFile() + throws Exception { + Path file = Files.createTempFile("admin-init-test", ".pa"); + try { + Files.write(file, "p\u00e4ss word\n".getBytes(StandardCharsets.UTF_8)); + HugeAuthenticator authenticator = this.mockMemoryGraphWithAuth(); + + this.checkBackendVersionOrExit(adminPaFileConfig(file)); + + Mockito.verify(authenticator).initAdminUser("p\u00e4ss word"); + } finally { + Files.deleteIfExists(file); + } + } + + /** + * A refused file must abort startup with its own message rather than be + * folded into the generic backend error or fall back to auth.admin_pa. + */ + @Test + public void testNonPersistentBackendRefusesMissingAdminPasswordFile() + throws Exception { + Path missing = Files.createTempDirectory("admin-init-test") + .resolve("missing"); + try { + HugeAuthenticator authenticator = this.mockMemoryGraphWithAuth(); + HugeConfig config = adminPaFileConfig(missing); + + Assert.assertThrows(ConfigException.class, () -> { + this.checkBackendVersionOrExit(config); + }, e -> Assert.assertContains(ServerOptions.ADMIN_PA_FILE.name(), + e.getMessage())); + Mockito.verify(authenticator, Mockito.never()) + .initAdminUser(Mockito.anyString()); + } finally { + Files.deleteIfExists(missing.getParent()); + } + } + + private HugeAuthenticator mockMemoryGraphWithAuth() { + HugeGraph graph = Mockito.mock(HugeGraph.class); + Mockito.when(graph.backendStoreFeatures()) + .thenReturn(Mockito.mock(BackendFeatures.class)); + BackendStoreInfo info = Mockito.mock(BackendStoreInfo.class); + Mockito.when(info.exists()).thenReturn(true); + Mockito.when(info.checkVersion()).thenReturn(true); + Mockito.when(graph.backendStoreInfo()).thenReturn(info); + Map graphs = Whitebox.getInternalState(this.manager, + "graphs"); + graphs.put("DEFAULT-memory", graph); + + HugeAuthenticator authenticator = Mockito.mock(HugeAuthenticator.class); + Mockito.when(authenticator.requireAuthentication()).thenReturn(true); + Whitebox.setInternalState(this.manager, "authenticator", authenticator); + return authenticator; + } + + private void checkBackendVersionOrExit(HugeConfig config) { + Whitebox.invoke(GraphManager.class, new Class[]{HugeConfig.class}, + "checkBackendVersionOrExit", this.manager, config); + } + + private static HugeConfig adminPaFileConfig(Path file) { + PropertiesConfiguration properties = new PropertiesConfiguration(); + properties.setProperty(ServerOptions.ADMIN_PA_FILE.name(), + file.toString()); + properties.setProperty(ServerOptions.ADMIN_PA.name(), "from-pa"); + return new HugeConfig(properties); + } + private static Object swapMetaManagerField(String field, Object replacement) throws Exception { From 7ba00bd21d967773e7d8b0e516a5af26a172aea5 Mon Sep 17 00:00:00 2001 From: arshil-khan Date: Tue, 13 Oct 2026 23:51:21 +0530 Subject: [PATCH 4/5] fix(server): refuse non-regular and over-72-byte admin password files Check the path's attributes before opening it and refuse anything that is not a regular file. Opening a FIFO blocks until a writer connects, so a wrong path could hang startup before the size limit applied. Symlinks are followed, so a Kubernetes Secret mounted through its ..data links still works. Refuse a password longer than 72 bytes in UTF-8. jBCrypt hashes only the first 72 bytes, so a longer secret would also accept any password sharing that prefix. The 4096-byte limit still bounds how much of the file is read. Tests cover a directory, a FIFO (with a timeout so a regression fails instead of hanging), a symlink to a regular file, and the 72-byte limit for ASCII and multi-byte passwords. --- docker/README.md | 5 +- .../hugegraph/auth/StandardAuthenticator.java | 28 ++++++- .../hugegraph/config/ServerOptions.java | 8 +- .../static/conf/rest-server.properties | 1 + .../unit/auth/AdminPasswordFileTest.java | 79 +++++++++++++++++-- 5 files changed, 107 insertions(+), 14 deletions(-) diff --git a/docker/README.md b/docker/README.md index f5cde2f12a..bf8d28c95e 100644 --- a/docker/README.md +++ b/docker/README.md @@ -62,7 +62,10 @@ the image reuses the configured key instead of rotating it. Instead of `auth.admin_pa`, a REST configuration you mount yourself can set `auth.admin_pa_file` to the path of a mounted secret. The Server reads that file as raw UTF-8 and drops a leading BOM and one trailing line break, so the password -needs no properties escaping. When set, it overrides `auth.admin_pa` and the +needs no properties escaping. The path must lead to a regular file, though +symlinks such as the ones Kubernetes uses for mounted Secrets are followed. The +password may be at most 72 bytes in UTF-8, because the password hash ignores +anything after that. When set, it overrides `auth.admin_pa` and the init-store password prompt. With PD (hstore), including when init-store is disabled, the file is read on every start, so keep the secret mounted for as long as the option is set. With init-store disabled, startup also requires this option diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java index d63b292aa7..77662b8275 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java @@ -27,7 +27,9 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.InvalidPathException; +import java.nio.file.Path; import java.nio.file.Paths; +import java.nio.file.attribute.BasicFileAttributes; import java.util.Arrays; import java.util.HashMap; import java.util.Map; @@ -53,6 +55,7 @@ public class StandardAuthenticator implements HugeAuthenticator { private static final String INITING_STORE = "initing_store"; private static final int MAX_ADMIN_PA_FILE_BYTES = 4096; + private static final int MAX_ADMIN_PASSWORD_BYTES = 72; private HugeGraph graph = null; @@ -247,9 +250,20 @@ public static String initialAdminPassword(HugeConfig config) { private static String readAdminPasswordFile(String file) { String option = ServerOptions.ADMIN_PA_FILE.name(); byte[] bytes; - // Bounded, so a path that points at a log or a device fails quickly - try (InputStream in = Files.newInputStream(Paths.get(file))) { - bytes = in.readNBytes(MAX_ADMIN_PA_FILE_BYTES + 1); + try { + Path path = Paths.get(file); + // Checked before opening, because opening a FIFO blocks until a + // writer connects. Symlinks are followed, so a Kubernetes secret + // mounted through ..data links still counts as a regular file + if (!Files.readAttributes(path, BasicFileAttributes.class) + .isRegularFile()) { + throw new ConfigException("The file '%s' set by '%s' is not a regular file", + file, option); + } + // Bounded, so a path that points at a large file fails quickly + try (InputStream in = Files.newInputStream(path)) { + bytes = in.readNBytes(MAX_ADMIN_PA_FILE_BYTES + 1); + } } catch (IOException | InvalidPathException e) { throw new ConfigException("Failed to read the file '%s' set by '%s' (%s)", e, file, option, e.getClass().getSimpleName()); @@ -286,6 +300,14 @@ private static String readAdminPasswordFile(String file) { throw new ConfigException("The file '%s' set by '%s' is empty", file, option); } + // jBCrypt hashes only the first 72 bytes, so a longer password would + // also accept anything that shares its first 72 bytes + if (password.getBytes(StandardCharsets.UTF_8).length > MAX_ADMIN_PASSWORD_BYTES) { + throw new ConfigException("The password in the file '%s' set by '%s' is " + + "longer than %s bytes in UTF-8, the most that " + + "the password hash uses", + file, option, MAX_ADMIN_PASSWORD_BYTES); + } return password; } diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java index ab822162e9..4fd08f499c 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java @@ -497,9 +497,11 @@ public class ServerOptions extends OptionHolder { public static final ConfigOption ADMIN_PA_FILE = new ConfigOption<>( "auth.admin_pa_file", - "The path of a file holding the default password for " + - "built-in admin account, read as raw UTF-8 with a " + - "leading BOM and one trailing line break removed. When " + + "The path of a regular file (symlinks are followed) " + + "holding the default password for built-in admin " + + "account, read as raw UTF-8 with a leading BOM and one " + + "trailing line break removed. The password may be at " + + "most 72 bytes in UTF-8, the most the hash uses. When " + "set, it takes precedence over auth.admin_pa and the " + "init-store prompt; takes effect on first startup. PD " + "and non-persistent backends read it on every start, so " + diff --git a/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties b/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties index 4bdc3fa606..92638eea4c 100644 --- a/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties +++ b/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties @@ -23,6 +23,7 @@ arthas.disabled_commands=jad #auth.admin_pa=pa # or read it from a file as raw UTF-8 (a leading BOM and one trailing line break are dropped), so # spaces, backslashes and non-ASCII characters need no escaping; overrides auth.admin_pa. +# It must be a regular file (symlinks are followed) holding at most 72 UTF-8 bytes. # PD and non-persistent backends read it on every start, so keep it readable while set #auth.admin_pa_file=/run/secrets/hugegraph-admin-password #auth.graph_store=hugegraph diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/AdminPasswordFileTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/AdminPasswordFileTest.java index 2e3f941d1b..071d575605 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/AdminPasswordFileTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/AdminPasswordFileTest.java @@ -40,6 +40,7 @@ import org.apache.hugegraph.testutil.Whitebox; import org.apache.hugegraph.util.StringEncoding; import org.junit.After; +import org.junit.Assume; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; @@ -154,21 +155,85 @@ public void testFileDropsLeadingBom() throws IOException { } /** - * A path pointing at a log or a device must not be read whole into memory. + * A path pointing at a large file must not be read whole into memory. */ @Test public void testOversizedFileFails() throws IOException { - byte[] limit = new byte[4096]; - Arrays.fill(limit, (byte) 'a'); - Assert.assertEquals(4096, initialAdminPassword( - this.write(limit), null).length()); - - Path file = this.write(Arrays.copyOf(limit, limit.length + 1)); + byte[] content = new byte[4097]; + Arrays.fill(content, (byte) 'a'); + Path file = this.write(content); Assert.assertThrows(ConfigException.class, () -> { initialAdminPassword(file, "secret"); }, e -> Assert.assertContains("larger than 4096 bytes", e.getMessage())); } + /** + * jBCrypt hashes only the first 72 bytes, so a longer password would also + * accept every password sharing that prefix. The limit counts UTF-8 bytes + * after the BOM and line break are dropped. + */ + @Test + public void testPasswordOverBcryptLimitFails() throws IOException { + String ascii = "a".repeat(72); + String umlauts = "\u00e4".repeat(36); + Assert.assertEquals(ascii, initialAdminPassword( + this.write(utf8("\uFEFF" + ascii + "\r\n")), null)); + Assert.assertEquals(umlauts, initialAdminPassword( + this.write(utf8(umlauts)), null)); + + for (String password : Arrays.asList(ascii + "a", umlauts + "a")) { + Path file = this.write(utf8(password)); + Assert.assertThrows(ConfigException.class, () -> { + initialAdminPassword(file, "secret"); + }, e -> Assert.assertContains("longer than 72 bytes", e.getMessage())); + } + } + + @Test + public void testDirectoryFails() { + Assert.assertThrows(ConfigException.class, () -> { + initialAdminPassword(this.workDir, "secret"); + }, e -> Assert.assertContains("not a regular file", e.getMessage())); + } + + /** + * Opening a FIFO blocks until a writer connects, so it must be refused + * from its attributes before it is opened. The timeout turns a + * regression into a failure instead of a hung build. + */ + @Test(timeout = 10000) + public void testFifoFailsWithoutBlocking() throws Exception { + Path fifo = this.workDir.resolve("fifo"); + int exit; + try { + exit = new ProcessBuilder("mkfifo", fifo.toString()).start() + .waitFor(); + } catch (IOException e) { + exit = -1; + } + Assume.assumeTrue("mkfifo is not available", exit == 0); + + Assert.assertThrows(ConfigException.class, () -> { + initialAdminPassword(fifo, "secret"); + }, e -> Assert.assertContains("not a regular file", e.getMessage())); + } + + /** + * Kubernetes mounts each Secret key as a symlink into a ..data directory, + * so the regular-file check has to follow links. + */ + @Test + public void testSymlinkToRegularFileIsFollowed() throws IOException { + Path target = this.write(utf8("secret\n")); + Path link = this.workDir.resolve("link"); + try { + Files.createSymbolicLink(link, target); + } catch (UnsupportedOperationException | IOException e) { + Assume.assumeNoException("symlinks are not supported", e); + } + Assert.assertEquals("secret", initialAdminPassword(link, null)); + } + /** * init-store prompts on stdin for a persistent backend. With a password * file configured it must create the admin from the file instead. From f7f38ac2597b432ffd6249b51752148a4addb6e1 Mon Sep 17 00:00:00 2001 From: arshil-khan Date: Wed, 14 Oct 2026 08:37:45 +0530 Subject: [PATCH 5/5] fix(server): skip the admin password file under remote auth With auth.remote_url set, StandardAuthenticator uses a remote AuthManager and never creates a local admin, but the non-persistent backend path still resolved auth.admin_pa_file first. A missing file that would never be used stopped startup. Skip the lookup in that case, as the PD startup path already does. Custom authenticators still receive initAdminUser(). --- .../apache/hugegraph/core/GraphManager.java | 16 ++++++++- .../unit/core/GraphManagerAdminInitTest.java | 35 ++++++++++++++++++- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java index d038b7961b..dea872ef15 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java @@ -395,6 +395,18 @@ private static boolean shouldBootstrapAdmin(HugeAuthenticator authenticator, remoteUrl.isEmpty(); } + /** + * With auth.remote_url the built-in authenticator switches to a remote + * AuthManager and never creates a local admin, so the admin password, and + * a password file that may not even be mounted, must not be resolved. + * Custom authenticators keep receiving initAdminUser() as before. + */ + private static boolean usesRemoteAuth(HugeAuthenticator authenticator, + String remoteUrl) { + return authenticator instanceof StandardAuthenticator && + !remoteUrl.isEmpty(); + } + /** * Creates the built-in admin account in PD metadata. With init-store * disabled this is the only bootstrap that admin gets, and init-store's @@ -1877,7 +1889,9 @@ private void checkBackendVersionOrExit(HugeConfig config) { assert hugegraph != null; if (!hugegraph.backendStoreFeatures().supportsPersistence()) { hugegraph.initBackend(); - if (this.requireAuthentication()) { + if (this.requireAuthentication() && + !usesRemoteAuth(this.authenticator, + config.get(ServerOptions.AUTH_REMOTE_URL))) { // Outside the try so a bad password file names itself String adminPassword = StandardAuthenticator.initialAdminPassword(config); diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerAdminInitTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerAdminInitTest.java index 360d5c38b2..18e09d98de 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerAdminInitTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerAdminInitTest.java @@ -207,7 +207,40 @@ public void testNonPersistentBackendRefusesMissingAdminPasswordFile() } } + /** + * Remote auth keeps the admin in another service, so an unused password + * file that is missing must not stop a non-persistent backend from + * starting, matching the guard on the PD startup path. + */ + @Test + public void testNonPersistentBackendWithRemoteAuthSkipsAdminPasswordFile() + throws Exception { + Path missing = Files.createTempDirectory("admin-init-test") + .resolve("missing"); + try { + HugeAuthenticator authenticator = this.mockMemoryGraphWithAuth( + StandardAuthenticator.class); + PropertiesConfiguration properties = new PropertiesConfiguration(); + properties.setProperty(ServerOptions.ADMIN_PA_FILE.name(), + missing.toString()); + properties.setProperty(ServerOptions.AUTH_REMOTE_URL.name(), + "127.0.0.1:8899"); + + this.checkBackendVersionOrExit(new HugeConfig(properties)); + + Mockito.verify(authenticator, Mockito.never()) + .initAdminUser(Mockito.anyString()); + } finally { + Files.deleteIfExists(missing.getParent()); + } + } + private HugeAuthenticator mockMemoryGraphWithAuth() { + return this.mockMemoryGraphWithAuth(HugeAuthenticator.class); + } + + private HugeAuthenticator mockMemoryGraphWithAuth( + Class authenticatorClass) { HugeGraph graph = Mockito.mock(HugeGraph.class); Mockito.when(graph.backendStoreFeatures()) .thenReturn(Mockito.mock(BackendFeatures.class)); @@ -219,7 +252,7 @@ private HugeAuthenticator mockMemoryGraphWithAuth() { "graphs"); graphs.put("DEFAULT-memory", graph); - HugeAuthenticator authenticator = Mockito.mock(HugeAuthenticator.class); + HugeAuthenticator authenticator = Mockito.mock(authenticatorClass); Mockito.when(authenticator.requireAuthentication()).thenReturn(true); Whitebox.setInternalState(this.manager, "authenticator", authenticator); return authenticator;