8356997: /etc/krb5.conf parser should not forbid include/includedir directives after sections

Reviewed-by: valeriep
This commit is contained in:
Weijun Wang 2025-07-23 12:24:28 +00:00
parent e6ac956a7a
commit 06f9ff047f
4 changed files with 487 additions and 108 deletions

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2000, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2000, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -70,6 +70,11 @@ public class Config {
*/
public static final int MAX_REFERRALS;
/**
* Maximum number of files that can be included.
*/
private static final int MAX_INCLUDE_FILE = 100;
static {
String disableReferralsProp =
SecurityProperties.getOverridableProperty(
@ -96,7 +101,12 @@ public class Config {
*/
private static Config singleton = null;
/*
/**
* All lines read from all krb5 config files.
*/
private Map<Path,List<String>> allConfs = new HashMap<>();
/**
* Hashtable used to store configuration information.
*/
private Hashtable<String,Object> stanzaTable = new Hashtable<>();
@ -202,11 +212,10 @@ public class Config {
// Always read the Kerberos configuration file
try {
List<String> configFile;
String fileName = getJavaFileName();
if (fileName != null) {
configFile = loadConfigFile(fileName);
stanzaTable = parseStanzaTable(configFile);
Path p = loadConfigFile(fileName); // p is main entry
parseStanzaTable(p);
if (DEBUG != null) {
DEBUG.println("Loaded from Java config");
}
@ -224,9 +233,9 @@ public class Config {
}
}
if (!found) {
fileName = getNativeFileName();
configFile = loadConfigFile(fileName);
stanzaTable = parseStanzaTable(configFile);
fileName = getNativeFileName(); // p is main entry
Path p = loadConfigFile(fileName);
parseStanzaTable(p);
if (DEBUG != null) {
DEBUG.println("Loaded from native config");
}
@ -564,80 +573,129 @@ public class Config {
}
/**
* Reads the lines of the configuration file. All include and includedir
* directives are resolved by calling this method recursively.
* Reads a configuration file. All include and includedir directives are
* also read by calling this method recursively. All contents are stored
* in {@link #allConfs} with file name as key.
*
* @param file the krb5.conf file, must be absolute
* @param content the lines. Comment and empty lines are removed,
* all lines trimmed, include and includedir
* directives resolved, unknown directives ignored
* Comment and empty lines are removed, all lines are trimmed, include and
* includedir directives are processed and translated to "#include" followed
* by a file name (not a directory name), unknown directives are ignored.
*
* @param file a krb5 config file, must be absolute
* @param dups a set of Paths to check for possible infinite loop
* @throws IOException if there is an I/O error
* @throws KrbException other errors
*/
private static Void readConfigFileLines(
Path file, List<String> content, Set<Path> dups)
throws IOException {
private void readConfigFileLines(Path file, Set<Path> dups)
throws KrbException, IOException {
if (DEBUG != null) {
DEBUG.println("Loading krb5 profile at " + file);
}
if (!file.isAbsolute()) {
throw new IOException("Profile path not absolute");
throw new KrbException("Profile path not absolute");
}
if (allConfs.size() > MAX_INCLUDE_FILE) {
throw new KrbException("Too many include files");
}
if (!dups.add(file)) {
throw new IOException("Profile path included more than once");
throw new KrbException("Recursive include");
}
List<String> lines = Files.readAllLines(file);
boolean inDirectives = true;
for (String line: lines) {
line = line.trim();
if (line.isEmpty() || line.startsWith("#") || line.startsWith(";")) {
continue;
try {
if (allConfs.containsKey(file)) {
// Already parsed. Including a file multiple times is allowed.
// Just make sure it cannot be recursive.
return;
}
if (inDirectives) {
if (line.charAt(0) == '[') {
inDirectives = false;
content.add(line);
} else if (line.startsWith("includedir ")) {
List<String> lines = Files.readAllLines(file);
List<String> content = new ArrayList<>();
// Add content to map at the beginning to detect duplicates
allConfs.put(file, content);
boolean inSections = false;
for (String line : lines) {
line = line.trim();
if (line.isEmpty() || line.startsWith("#") || line.startsWith(";")) {
continue;
}
if (line.startsWith("includedir ")) {
Path dir = Paths.get(
line.substring("includedir ".length()).trim());
try (Stream<Path> files = Files.list(dir)) {
for (Path p: files.sorted().toList()) {
for (Path p : files.sorted().toList()) {
if (Files.isDirectory(p)) continue;
String name = p.getFileName().toString();
if (name.matches("[a-zA-Z0-9_-]+") ||
(!name.startsWith(".") &&
name.endsWith(".conf"))) {
// if dir is absolute, so is p
readConfigFileLines(p, content, dups);
readConfigFileLines(p, dups);
content.add("#include " + p);
}
}
}
} else if (line.startsWith("include ")) {
readConfigFileLines(
Paths.get(line.substring("include ".length()).trim()),
content, dups);
Path p = Paths.get(line.substring("include ".length()).trim());
content.add("#include " + p);
readConfigFileLines(p, dups);
} else {
// Unsupported directives
if (DEBUG != null) {
DEBUG.println("Unknown directive: " + line);
if (!inSections) {
if (line.charAt(0) == '[') {
inSections = true;
content.add(line);
} else {
// Unsupported directives
if (DEBUG != null) {
DEBUG.println("Line not in any section: " + line);
}
}
} else {
content.add(line);
}
}
} else {
content.add(line);
}
} finally {
dups.remove(file);
}
return null;
}
/**
* Reads the configuration file and return normalized lines.
* Reads the main configuration file.
*
* @param fileName the configuration file
* @return absolute path to the config file
*/
private Path loadConfigFile(final String fileName)
throws IOException, KrbException {
if (DEBUG != null) {
DEBUG.println("Loading config file from " + fileName);
}
Set<Path> dupsCheck = new HashSet<>();
Path fullp = Paths.get(fileName).toAbsolutePath();
if (!Files.exists(fullp)) {
// This is OK. There are other ways to get
// Kerberos 5 settings
} else {
readConfigFileLines(fullp, dupsCheck);
}
return fullp;
}
/**
* Normalizes strings read from one config file. All sections and
* subsections are enclosed in braces. Directives ("#include") are
* kept in the same place.
*
* If the original file is:
*
* [realms]
* includedir /tmp/inc
* EXAMPLE.COM =
* {
* kdc = kerberos.example.com
@ -645,10 +703,24 @@ public class Config {
* }
* ...
*
* The result will be (no indentations):
* The output of readConfigFileLines will be (no indentations):
*
* [realms]
* #include /tmp/inc/conf1
* #include /tmp/inc/conf2
* EXAMPLE.COM =
* {
* kdc = kerberos.example.com
* ...
* }
* ...
*
* The output of normalize will be (no indentations):
*
* {
* realms = {
* #include /tmp/inc/conf1
* #include /tmp/inc/conf2
* EXAMPLE.COM = {
* kdc = kerberos.example.com
* ...
@ -657,37 +729,32 @@ public class Config {
* ...
* }
*
* @param fileName the configuration file
* @return normalized lines
* @param raw input list of strings
* @return normalized list of strings
* @throws KrbException when the format is not correct
*/
private List<String> loadConfigFile(final String fileName)
throws IOException, KrbException {
if (DEBUG != null) {
DEBUG.println("Loading config file from " + fileName);
}
private static List<String> normalize(List<String> raw) throws KrbException {
List<String> result = new ArrayList<>();
List<String> raw = new ArrayList<>();
Set<Path> dupsCheck = new HashSet<>();
Path fullp = Paths.get(fileName).toAbsolutePath();
Path path = Paths.get(fileName);
if (!Files.exists(path)) {
// This is OK. There are other ways to get
// Kerberos 5 settings
} else {
readConfigFileLines(fullp, raw, dupsCheck);
}
String previous = null;
List<String> unwritten = new ArrayList<>();
String previous = null; // unfinished line
for (String line: raw) {
if (line.startsWith("[")) {
if (line.startsWith("#")) { // directives like "#include". Do not
// write out immediately, might follow
// a previous line.
if (previous == null) {
result.add(line);
} else {
unwritten.add(line);
}
} else if (line.startsWith("[")) {
if (!line.endsWith("]")) {
throw new KrbException("Illegal config content:"
+ line);
}
if (previous != null) {
result.add(previous);
unwritten.forEach(result::add);
unwritten.clear();
result.add("}");
}
String title = line.substring(
@ -706,6 +773,8 @@ public class Config {
if (line.length() > 1) {
// { and content on the same line
result.add(previous);
unwritten.forEach(result::add);
unwritten.clear();
previous = line.substring(1).trim();
}
} else {
@ -716,11 +785,15 @@ public class Config {
"Config file must starts with a section");
}
result.add(previous);
unwritten.forEach(result::add);
unwritten.clear();
previous = line;
}
}
if (previous != null) {
result.add(previous);
unwritten.forEach(result::add);
unwritten.clear();
result.add("}");
}
return result;
@ -734,44 +807,52 @@ public class Config {
* another sub-sub-section or a non-empty vector of strings for final values
* (even if there is only one value defined).
* <p>
* For top-level sections with duplicates names, their contents are merged.
* For sub-sections the former overwrites the latter. For final values,
* they are stored in a vector in their appearing order. Please note these
* values must appear in the same sub-section. Otherwise, the sub-section
* appears first should have already overridden the others.
* Contents of duplicated sections are merged. Values for duplicated names
* are stored in a vector in their appearing order. If the same name is used
* as both a section name and a value name, the first appearance decides the
* type and the latter appearances of different types are ignored.
* <p>
* As a corner case, if the same name is used as both a section name and a
* value name, the first appearance decides the type. That is to say, if the
* first one is for a section, all latter appearances are ignored. If it's
* a value, latter appearances as sections are ignored, but those as values
* are added to the vector.
* <p>
* The behavior described above is compatible to other krb5 implementations
* but it's not decumented publicly anywhere. the best practice is not to
* The behavior described above is compatible to other krb5 implementations,
* but it's not documented publicly anywhere. the best practice is not to
* assume any kind of override functionality and only specify values for
* a particular key in one place.
*
* @param v the normalized input as return by loadConfigFile
* @param entry path to config file, could be an included one
* @throws KrbException if there is a file format error
*/
@SuppressWarnings("unchecked")
private Hashtable<String,Object> parseStanzaTable(List<String> v)
private void parseStanzaTable(Path entry)
throws KrbException {
Hashtable<String,Object> current = stanzaTable;
// Current sections and subsections
Deque<Hashtable<String,Object>> stack = new ArrayDeque<>();
List<String> v = allConfs.get(entry);
if (v == null) {
// this happens when root krb5.conf is missing
return;
}
v = normalize(v);
if (DEBUG != null) {
DEBUG.println(">>> Begin Kerberos config at " + entry);
v.forEach(DEBUG::println);
DEBUG.println(">>> End Kerberos config at " + entry);
}
for (String line: v) {
if (DEBUG != null) {
DEBUG.println(line);
}
// There are only 3 kinds of lines
// 1. a = b
// 2. a = {
// 3. }
if (line.equals("}")) {
// There are only 4 kinds of lines after normalization
// 1. #include
// 2. a = b
// 3. a = {
// 4. }
if (line.startsWith("#include ")) {
// parse in-place at the top level, i.e. included file
// is not considered inside the current section.
parseStanzaTable(Path.of(line.substring(9)));
} else if (line.equals("}")) {
// Go back to parent, see below
current = (Hashtable<String,Object>)current.remove(" PARENT ");
if (current == null) {
if (stack.isEmpty()) {
throw new KrbException("Unmatched close brace");
}
current = stack.pop();
} else {
int pos = line.indexOf('=');
if (pos < 0) {
@ -784,37 +865,33 @@ public class Config {
if (current == stanzaTable) {
key = key.toLowerCase(Locale.US);
}
// When there are dup names for sections
if (current.containsKey(key)) {
if (current == stanzaTable) { // top-level, merge
// The value at top-level must be another Hashtable
subTable = (Hashtable<String,Object>)current.get(key);
} else { // otherwise, ignored
// read and ignore it (do not put into current)
Object obj = current.get(key);
if (obj instanceof Hashtable) {
// dup section, merge
subTable = (Hashtable<String,Object>) obj;
} else {
// different type, parse and ignore
subTable = new Hashtable<>();
}
} else {
subTable = new Hashtable<>();
current.put(key, subTable);
}
// A special entry for its parent. Put whitespaces around,
// so will never be confused with a normal key
subTable.put(" PARENT ", current);
// Remember where I am.
stack.push(current);
current = subTable;
} else {
Vector<String> values;
if (current.containsKey(key)) {
Object obj = current.get(key);
if (obj instanceof Vector) {
// String values are merged
values = (Vector<String>)obj;
values.add(value);
// dup value, accumulate
((Vector<String>) obj).add(value);
} else {
// If a key shows as section first and then a value,
// ignore the value.
// different type, ignore
}
} else {
values = new Vector<String>();
Vector<String> values = new Vector<>();
values.add(value);
current.put(key, values);
}
@ -824,7 +901,6 @@ public class Config {
if (current != stanzaTable) {
throw new KrbException("Not closed");
}
return current;
}
/**

View File

@ -0,0 +1,89 @@
/*
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
import jdk.test.lib.Asserts;
import sun.security.krb5.Config;
import sun.security.krb5.KrbException;
import java.nio.file.Files;
import java.nio.file.Path;
/*
* @test
* @bug 8356997
* @summary Support "include" anywhere
* @modules java.security.jgss/sun.security.krb5
* @library /test/lib
* @run main/othervm DuplicatedIncludes
*/
public class DuplicatedIncludes {
public static void main(String[] args) throws Exception {
var cwd = Path.of("").toAbsolutePath().toString();
System.setProperty("java.security.krb5.conf", "krb5.conf");
// It's OK to include a file multiple times
Files.writeString(Path.of("krb5.conf"), String.format("""
include %1$s/sub
include %1$s/sub
""", cwd));
Files.writeString(Path.of("sub"), """
[a]
b = c
""");
Config.refresh();
// But a file cannot include itself
Files.writeString(Path.of("sub"), String.format("""
include %1$s/sub
""", cwd));
Asserts.assertThrows(KrbException.class, () -> Config.refresh());
// A file also cannot include a file that includes it
Files.writeString(Path.of("sub"), String.format("""
include %1$s/sub2
""", cwd));
Files.writeString(Path.of("sub2"), String.format("""
include %1$s/sub
""", cwd));
Asserts.assertThrows(KrbException.class, () -> Config.refresh());
// It's OK for a file to include another file that has already
// been included multiple times, as long as it's not on the stack.
// This proves it's necessary to place "dups.remove(file)" in a
// finally block in Config::readConfigFileLines. This case is
// not covered by IncludeRandom.java because of the structured
// include pattern (included always longer than includee) there.
Files.writeString(Path.of("krb5.conf"), String.format("""
include %1$s/sub
include %1$s/sub
include %1$s/sub2
""", cwd));
Files.writeString(Path.of("sub"), "");
Files.writeString(Path.of("sub2"), String.format("""
include %1$s/sub
""", cwd));
Config.refresh();
}
}

View File

@ -0,0 +1,139 @@
/*
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
/*
* @test
* @bug 8356997
* @summary Support "include" anywhere
* @modules java.security.jgss/sun.security.krb5
* @library /test/lib
* @run main/othervm IncludeRandom
*/
import jdk.test.lib.Asserts;
import jdk.test.lib.security.SeededSecureRandom;
import sun.security.krb5.Config;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;
// A randomized class to prove that wherever the "include" line is inside
// a krb5.conf file, it can always be parsed correctly.
public class IncludeRandom {
static SecureRandom sr = SeededSecureRandom.one();
// Must be global. Counting in recursive methods
static int nInc = 0; // number of included files
static int nAssign = 0; // number of assignments to the same setting
public static void main(String[] args) throws Exception {
System.setProperty("java.security.krb5.conf", "f");
for (var i = 0; i < 10_000; i++) {
test();
}
}
static void test() throws Exception {
nInc = 0;
nAssign = 0;
write("f");
if (nAssign != 0) {
Config.refresh();
var j = Config.getInstance().getAll("section", "sub", "x");
var r = readRaw("f", new ArrayList<String>())
.stream()
.collect(Collectors.joining(" "));
Asserts.assertEQ(r, j);
}
try (var dir = Files.newDirectoryStream(Path.of("."), "f*")) {
for (var f : dir) {
Files.delete(f);
}
}
}
// read settings as raw files
static List<String> readRaw(String f, List<String> list) throws IOException {
for (var s : Files.readAllLines(Path.of(f))) {
if (s.startsWith("include ")) {
readRaw(s.substring(8), list);
}
if (s.contains("x = ")) {
list.add(s.substring(s.indexOf("x = ") + 4));
}
}
return list;
}
// write krb5.conf with random include
static void write(String f) throws IOException {
var p = Path.of(f);
if (Files.exists(p)) return; // do not overwrite, same file can be
// included twice
var content = new ArrayList<String>();
content.add("[section]"); // always starts with section
for (var i = 0; i < sr.nextInt(5); i++) {
if (sr.nextBoolean()) { // might have more section(s)
content.add("[section]");
}
if (sr.nextBoolean()) { // style 1: { on subsection line
content.add("sub = {");
} else {
content.add("sub = ");
if (sr.nextBoolean()) {
content.add("{"); // style 2: { on individual line
} else {
// style 3: { on key-value line
content.add("{ x = " + sr.nextInt(99999999));
nAssign++;
}
}
for (var j = 0; j < sr.nextInt(3); j++) { // might have more
content.add("x = " + sr.nextInt(99999999));
nAssign++;
}
content.add("}");
}
// randomly throw in include lines
for (var i = 0; i < sr.nextInt(3); i++) {
if (nInc < 98) {
// include file name is random, so there could be dup
// but name length always grows, so no recursive.
// Extra length could be 1 digit or 2 digits, so the
// same file can be included on 2 levels, e.g. f1 includes
// f12 and f123, and f12 includes f123 again.
var inc = f + sr.nextInt(100);
content.add(sr.nextInt(content.size() + 1),
"include " + Path.of(inc).toAbsolutePath());
nInc++;
write(inc);
}
}
Files.write(p, content);
}
}

View File

@ -0,0 +1,75 @@
/*
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
import jdk.test.lib.Asserts;
import sun.security.krb5.Config;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
/*
* @test
* @bug 8356997
* @summary Support "include" anywhere
* @modules java.security.jgss/sun.security.krb5
* @library /test/lib
* @run main/othervm IncludeSameKey
*/
public class IncludeSameKey {
public static void main(String[] args) throws Exception {
var cwd = Path.of("").toAbsolutePath().toString();
Files.writeString(Path.of("krb5.conf"), String.format("""
include %1$s/outside
[a]
include %1$s/beginsec
b = {
c = 1
}
[a]
b = {
c = 2
}
include %1$s/insec
include %1$s/insec2
b = {
include %1$s/insubsec
c = 3
include %1$s/endsubsec
}
include %1$s/endsec
""", cwd));
for (var inc : List.of("outside", "beginsec", "insec", "insec2",
"insubsec", "endsubsec", "endsec")) {
Files.writeString(Path.of(inc), String.format("""
[a]
b = {
c = %s
}
""", inc));
}
System.setProperty("java.security.krb5.conf", "krb5.conf");
Asserts.assertEQ(Config.getInstance().getAll("a", "b", "c"),
"outside beginsec 1 2 insec insec2 insubsec 3 endsubsec endsec");
}
}