# HG changeset patch # User weijun # Date 1403231230 -28800 # Node ID 7a670121602ef4718b6d3c1cca164725fac3748a # Parent 14cc6eb021d5109f6a09c796b661305831054d41 8029994: Support "include" and "includedir" in krb5.conf Reviewed-by: mullan diff -r 14cc6eb021d5 -r 7a670121602e jdk/src/share/classes/sun/security/krb5/Config.java --- a/jdk/src/share/classes/sun/security/krb5/Config.java Thu Jun 19 13:12:08 2014 -0700 +++ b/jdk/src/share/classes/sun/security/krb5/Config.java Fri Jun 20 10:27:10 2014 +0800 @@ -31,10 +31,13 @@ package sun.security.krb5; import java.io.File; -import java.io.FileInputStream; +import java.io.FilePermission; +import java.nio.file.DirectoryStream; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.nio.file.Path; +import java.security.PrivilegedAction; import java.util.*; -import java.io.BufferedReader; -import java.io.InputStreamReader; import java.io.IOException; import java.net.InetAddress; import java.net.UnknownHostException; @@ -196,8 +199,11 @@ } } } catch (IOException ioe) { - // I/O error, mostly like krb5.conf missing. - // No problem. We'll use DNS or system property etc. + if (DEBUG) { + System.out.println("Exception thrown in loading config:"); + ioe.printStackTrace(System.out); + } + throw new KrbException("krb5.conf loading failed"); } } @@ -206,7 +212,7 @@ * @param keys the keys, as an array from section name, sub-section names * (if any), to value name. * @return the value. When there are multiple values for the same key, - * returns the last one. {@code null} is returned if not all the keys are + * returns the first one. {@code null} is returned if not all the keys are * defined. For example, {@code get("libdefaults", "forwardable")} will * return null if "forwardable" is not defined in [libdefaults], and * {@code get("realms", "R", "kdc")} will return null if "R" is not @@ -223,7 +229,7 @@ public String get(String... keys) { Vector v = getString0(keys); if (v == null) return null; - return v.lastElement(); + return v.firstElement(); } /** @@ -273,7 +279,7 @@ } /** - * Returns true if keys exists, can be either final string(s) or sub-stanza + * Returns true if keys exists, can be final string(s) or a sub-section * @throws IllegalArgumentException if any of the keys is illegal * (See {@link #get}) */ @@ -291,9 +297,9 @@ } } - // Internal method. Returns the value for keys, which can be a sub-stanza - // or final string value(s). - // The only method (except for toString) that reads stanzaTable directly. + // Internal method. Returns the value for keys, which can be a sub-section + // (as a Hashtable) or final string value(s) (as a Vector). This is the + // only method (except for toString) that reads stanzaTable directly. @SuppressWarnings("unchecked") private Object get0(String... keys) { Object current = stanzaTable; @@ -453,142 +459,207 @@ } /** - * Reads lines to the memory from the configuration file. + * Reads the lines of the configuration file. All include and includedir + * directives are resolved by calling this method recursively. * - * Configuration file contains information about the default realm, - * ticket parameters, location of the KDC and the admin server for - * known realms, etc. The file is divided into sections. Each section - * contains one or more name/value pairs with one pair per line. A - * typical file would be: - * 
-     * [libdefaults]
-     *          default_realm = EXAMPLE.COM
-     *          default_tgs_enctypes = des-cbc-md5
-     *          default_tkt_enctypes = des-cbc-md5
-     * [realms]
-     *          EXAMPLE.COM = {
-     *                  kdc = kerberos.example.com
-     *                  kdc = kerberos-1.example.com
-     *                  admin_server = kerberos.example.com
-     *                  }
-     *          SAMPLE_COM = {
-     *                  kdc = orange.sample.com
-     *                  admin_server = orange.sample.com
-     *                  }
-     * [domain_realm]
-     *          blue.sample.com = TEST.SAMPLE.COM
-     *          .backup.com     = EXAMPLE.COM
-     *
- * @return an ordered list of strings representing the config file after - * some initial processing, including:
    - *
  1. Comment lines and empty lines are removed - *
  2. "{" not at the end of a line is appended to the previous line - *
  3. The content of a section is also placed between "{" and "}". - *
  4. Lines are trimmed
+ * @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 + * @param dups a set of Paths to check for possible infinite loop * @throws IOException if there is an I/O error - * @throws KrbException if there is a file format error + */ + private static Void readConfigFileLines( + Path file, List content, Set dups) + throws IOException { + + if (DEBUG) { + System.out.println("Loading krb5 profile at " + file); + } + if (!file.isAbsolute()) { + throw new IOException("Profile path not absolute"); + } + + if (!dups.add(file)) { + throw new IOException("Profile path included more than once"); + } + + List lines = Files.readAllLines(file); + + boolean inDirectives = true; + for (String line: lines) { + line = line.trim(); + if (line.isEmpty() || line.startsWith("#")) { + continue; + } + if (inDirectives) { + if (line.charAt(0) == '[') { + inDirectives = false; + content.add(line); + } else if (line.startsWith("includedir ")) { + Path dir = Paths.get( + line.substring("includedir ".length()).trim()); + try (DirectoryStream files = + Files.newDirectoryStream(dir)) { + for (Path p: files) { + if (Files.isDirectory(p)) continue; + String name = p.getFileName().toString(); + if (name.matches("[a-zA-Z0-9_-]+")) { + // if dir is absolute, so is p + readConfigFileLines(p, content, dups); + } + } + } + } else if (line.startsWith("include ")) { + readConfigFileLines( + Paths.get(line.substring("include ".length()).trim()), + content, dups); + } else { + // Unsupported directives + if (DEBUG) { + System.out.println("Unknown directive: " + line); + } + } + } else { + content.add(line); + } + } + return null; + } + + /** + * Reads the configuration file and return normalized lines. + * If the original file is: + * + * [realms] + * EXAMPLE.COM = + * { + * kdc = kerberos.example.com + * ... + * } + * ... + * + * The result will be (no indentations): + * + * { + * realms = { + * EXAMPLE.COM = { + * kdc = kerberos.example.com + * ... + * } + * } + * ... + * } + * + * @param fileName the configuration file + * @return normalized lines */ private List loadConfigFile(final String fileName) throws IOException, KrbException { + + List result = new ArrayList<>(); + List raw = new ArrayList<>(); + Set dupsCheck = new HashSet<>(); + try { - List v = new ArrayList<>(); - try (BufferedReader br = new BufferedReader(new InputStreamReader( - AccessController.doPrivileged( - new PrivilegedExceptionAction () { - public FileInputStream run() throws IOException { - return new FileInputStream(fileName); + Path fullp = AccessController.doPrivileged((PrivilegedAction) + () -> Paths.get(fileName).toAbsolutePath(), + null, + new PropertyPermission("user.dir", "read")); + AccessController.doPrivileged( + new PrivilegedExceptionAction() { + @Override + public Void run() throws IOException { + Path path = Paths.get(fileName); + if (!Files.exists(path)) { + // This is OK. There are other ways to get + // Kerberos 5 settings + return null; + } else { + return readConfigFileLines( + fullp, raw, dupsCheck); + } } - })))) { - String line; - String previous = null; - while ((line = br.readLine()) != null) { - line = line.trim(); - if (line.startsWith("#") || line.isEmpty()) { - // ignore comments and blank line - // Comments start with #. - continue; - } - // In practice, a subsection might look like: - // [realms] - // EXAMPLE.COM = - // { - // kdc = kerberos.example.com - // ... - // } - // Before parsed into stanza table, it needs to be - // converted into a canonicalized style (no indent): - // realms = { - // EXAMPLE.COM = { - // kdc = kerberos.example.com - // ... - // } - // } - // - if (line.startsWith("[")) { - if (!line.endsWith("]")) { - throw new KrbException("Illegal config content:" - + line); - } - if (previous != null) { - v.add(previous); - v.add("}"); - } - String title = line.substring( - 1, line.length()-1).trim(); - if (title.isEmpty()) { - throw new KrbException("Illegal config content:" - + line); - } - previous = title + " = {"; - } else if (line.startsWith("{")) { - if (previous == null) { - throw new KrbException( - "Config file should not start with \"{\""); - } - previous += " {"; - if (line.length() > 1) { - // { and content on the same line - v.add(previous); - previous = line.substring(1).trim(); - } - } else { - // Lines before the first section are ignored - if (previous != null) { - v.add(previous); - previous = line; - } - } - } - if (previous != null) { - v.add(previous); - v.add("}"); - } - } - return v; + }, + null, + // include/includedir can go anywhere + new FilePermission("<>", "read")); } catch (java.security.PrivilegedActionException pe) { throw (IOException)pe.getException(); } + String previous = null; + for (String line: raw) { + if (line.startsWith("[")) { + if (!line.endsWith("]")) { + throw new KrbException("Illegal config content:" + + line); + } + if (previous != null) { + result.add(previous); + result.add("}"); + } + String title = line.substring( + 1, line.length()-1).trim(); + if (title.isEmpty()) { + throw new KrbException("Illegal config content:" + + line); + } + previous = title + " = {"; + } else if (line.startsWith("{")) { + if (previous == null) { + throw new KrbException( + "Config file should not start with \"{\""); + } + previous += " {"; + if (line.length() > 1) { + // { and content on the same line + result.add(previous); + previous = line.substring(1).trim(); + } + } else { + if (previous == null) { + // This won't happen, because before a section + // all directives have been resolved + throw new KrbException( + "Config file must starts with a section"); + } + result.add(previous); + previous = line; + } + } + if (previous != null) { + result.add(previous); + result.add("}"); + } + return result; } /** - * Parses stanza names and values from configuration file to - * stanzaTable (Hashtable). Hashtable key would be stanza names, - * (libdefaults, realms, domain_realms, etc), and the hashtable value - * would be another hashtable which contains the key-value pairs under - * a stanza name. The value of this sub-hashtable can be another hashtable - * containing another sub-sub-section or a vector of strings for - * final values (even if there is only one value defined). + * Parses the input lines to a hashtable. The key would be section names + * (libdefaults, realms, domain_realms, etc), and the value would be + * another hashtable which contains the key-value pairs inside the section. + * The value of this sub-hashtable can be another hashtable containing + * another sub-sub-section or a non-empty vector of strings for final values + * (even if there is only one value defined). + *

+ * 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. *

- * For duplicates section names, the latter overwrites the former. For - * duplicate value names, the values are in a vector in its appearing order. - * - * Please note that this behavior is Java traditional. and it is - * not the same as the MIT krb5 behavior, where:

    - *
  1. Duplicated root sections will be merged - *
  2. For duplicated sub-sections, the former overwrites the latter - *
  3. Duplicate keys for values are always saved in a vector - *
- * @param v the strings in the file, never null, might be empty + * 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. + *

+ * The behavior described above is compatible to other krb5 implementations + * but it's not decumented 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 * @throws KrbException if there is a file format error */ @SuppressWarnings("unchecked") @@ -596,7 +667,7 @@ throws KrbException { Hashtable current = stanzaTable; for (String line: v) { - // There are 3 kinds of lines + // There are only 3 kinds of lines // 1. a = b // 2. a = { // 3. } @@ -612,14 +683,25 @@ throw new KrbException("Illegal config content:" + line); } String key = line.substring(0, pos).trim(); - String value = trimmed(line.substring(pos+1)); + String value = unquote(line.substring(pos + 1)); if (value.equals("{")) { Hashtable subTable; if (current == stanzaTable) { key = key.toLowerCase(Locale.US); } - subTable = new Hashtable<>(); - current.put(key, subTable); + // 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)current.get(key); + } else { // otherwise, ignored + // read and ignore it (do not put into current) + 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); @@ -628,20 +710,19 @@ Vector values; if (current.containsKey(key)) { Object obj = current.get(key); - // If a key first shows as a section and then a value, - // this is illegal. However, we haven't really forbid - // first value then section, which the final result - // is a section. - if (!(obj instanceof Vector)) { - throw new KrbException("Key " + key - + "used for both value and section"); + if (obj instanceof Vector) { + // String values are merged + values = (Vector)obj; + values.add(value); + } else { + // If a key shows as section first and then a value, + // ignore the value. } - values = (Vector)current.get(key); } else { values = new Vector(); + values.add(value); current.put(key, values); } - values.add(value); } } } @@ -764,7 +845,7 @@ return "/etc/krb5.conf"; } - private static String trimmed(String s) { + private static String unquote(String s) { s = s.trim(); if (s.isEmpty()) return s; if (s.charAt(0) == '"' && s.charAt(s.length()-1) == '"' || diff -r 14cc6eb021d5 -r 7a670121602e jdk/test/sun/security/krb5/auto/HttpNegotiateServer.java --- a/jdk/test/sun/security/krb5/auto/HttpNegotiateServer.java Thu Jun 19 13:12:08 2014 -0700 +++ b/jdk/test/sun/security/krb5/auto/HttpNegotiateServer.java Fri Jun 20 10:27:10 2014 +0800 @@ -55,6 +55,8 @@ import org.ietf.jgss.GSSManager; import sun.security.jgss.GSSUtil; import sun.security.krb5.Config; +import sun.util.logging.PlatformLogger; + import java.util.Base64; /** @@ -148,6 +150,10 @@ public static void main(String[] args) throws Exception { + String HTTPLOG = "sun.net.www.protocol.http.HttpURLConnection"; + System.setProperty("sun.security.krb5.debug", "true"); + PlatformLogger.getLogger(HTTPLOG).setLevel(PlatformLogger.Level.ALL); + KDC kdcw = KDC.create(REALM_WEB); kdcw.addPrincipal(WEB_USER, WEB_PASS); kdcw.addPrincipalRandKey("krbtgt/" + REALM_WEB); diff -r 14cc6eb021d5 -r 7a670121602e jdk/test/sun/security/krb5/config/Duplicates.java --- a/jdk/test/sun/security/krb5/config/Duplicates.java Thu Jun 19 13:12:08 2014 -0700 +++ b/jdk/test/sun/security/krb5/config/Duplicates.java Fri Jun 20 10:27:10 2014 +0800 @@ -38,22 +38,22 @@ config.listTable(); String s; - // Latter overwrites former for root section + // root section merged s = config.get("libdefaults", "default_realm"); - if (s != null) { + if (!s.equals("R1")) { throw new Exception(); } - // Latter overwrites former for strings + // Former is preferred to latter for strings and sections s = config.get("libdefaults", "default_tkt_enctypes"); - if (!s.equals("aes256-cts")) { + if (!s.equals("aes128-cts")) { throw new Exception(); } - // Latter overwrites former for sub-section s = config.get("realms", "R1", "kdc"); - if (!s.equals("k2")) { + if (!s.equals("k1")) { throw new Exception(s); } - // Duplicate keys in [realms] are merged + // Duplicate keys in [realms] are merged, and sections with the same + // name in between ignored s = config.getAll("realms", "R2", "kdc"); if (!s.equals("k1 k2 k3 k4")) { throw new Exception(s); diff -r 14cc6eb021d5 -r 7a670121602e jdk/test/sun/security/krb5/config/Include.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/jdk/test/sun/security/krb5/config/Include.java Fri Jun 20 10:27:10 2014 +0800 @@ -0,0 +1,142 @@ +/* + * Copyright (c) 2014, 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 8029994 + * @summary Support "include" and "includedir" in krb5.conf + * @compile -XDignore.symbol.file Include.java + * @run main/othervm Include + */ +import sun.security.krb5.Config; +import sun.security.krb5.KrbException; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +public class Include { + public static void main(String[] args) throws Exception { + + String krb5Conf = "[section]\nkey="; // Skeleton of a section + + Path conf = Paths.get("krb5.conf"); // base krb5.conf + + Path ifile = Paths.get("f"); // include f + Path idir = Paths.get("x"); // includedir fx + Path idirdir = Paths.get("x/xx"); // sub dir, will be ignored + Path idirdirfile = Paths.get("x/xx/ff"); // sub dir, will be ignored + Path idirfile1 = Paths.get("x/f1"); // one file + Path idirfile2 = Paths.get("x/f2"); // another file + Path idirfile3 = Paths.get("x/f.3"); // third file bad name + + // OK: The base file can be missing + System.setProperty("java.security.krb5.conf", "no-such-file"); + tryReload(true); + + System.setProperty("java.security.krb5.conf", conf.toString()); + + // Write base file + Files.write(conf, + ("include " + ifile.toAbsolutePath() + "\n" + + "includedir " + idir.toAbsolutePath() + "\n" + + krb5Conf + "base").getBytes() + ); + + // Error: Neither include nor includedir exists + tryReload(false); + + // Error: Only includedir exists + Files.createDirectory(idir); + tryReload(false); + + // Error: Both exists, but include is a cycle + Files.write(ifile, + ("include " + conf.toAbsolutePath() + "\n" + + krb5Conf + "incfile").getBytes()); + tryReload(false); + + // Error: A good include exists, but no includedir + Files.delete(idir); + Files.write(ifile, (krb5Conf + "incfile").getBytes()); + tryReload(false); + + // OK: Everything is set + Files.createDirectory(idir); + tryReload(true); // Now OK + + // fx1 and fx2 will be loaded + Files.write(idirfile1, (krb5Conf + "incdir1").getBytes()); + Files.write(idirfile2, (krb5Conf + "incdir2").getBytes()); + // fx3 and fxs (and file inside it) will be ignored + Files.write(idirfile3, (krb5Conf + "incdir3").getBytes()); + Files.createDirectory(idirdir); + Files.write(idirdirfile, (krb5Conf + "incdirdir").getBytes()); + + // OK: All good files read + tryReload(true); + + String v = Config.getInstance().getAll("section", "key"); + // The order of files in includedir could be either + if (!v.equals("incfile incdir1 incdir2 base") && + !v.equals("incfile incdir2 incdir1 base")) { + throw new Exception(v); + } + + // Error: include file not absolute + Files.write(conf, + ("include " + ifile + "\n" + + "includedir " + idir.toAbsolutePath() + "\n" + + krb5Conf + "base").getBytes() + ); + tryReload(false); + + // Error: includedir not absolute + Files.write(conf, + ("include " + ifile.toAbsolutePath() + "\n" + + "includedir " + idir + "\n" + + krb5Conf + "base").getBytes() + ); + tryReload(false); + + // OK: unsupported directive + Files.write(conf, + ("module /lib/lib.so\n" + + krb5Conf + "base").getBytes() + ); + tryReload(true); + } + + private static void tryReload(boolean expected) throws Exception { + if (expected) { + Config.refresh(); + } else { + try { + Config.refresh(); + throw new Exception("Should be illegal"); + } catch (KrbException ke) { + // OK + } + } + } +} diff -r 14cc6eb021d5 -r 7a670121602e jdk/test/sun/security/krb5/config/k1.conf --- a/jdk/test/sun/security/krb5/config/k1.conf Thu Jun 19 13:12:08 2014 -0700 +++ b/jdk/test/sun/security/krb5/config/k1.conf Fri Jun 20 10:27:10 2014 +0800 @@ -9,11 +9,15 @@ R1 = { kdc = k1 } +R1 = hello R1 = { kdc = k2 } R2 = { kdc = k1 + kdc = { + foo = bar + } kdc = k2 k3 admin_server = a1 kdc = k4