8333772: Incorrect Kerberos behavior when udp_preference_limit = 0

Reviewed-by: ssahoo, mullan
This commit is contained in:
Weijun Wang 2024-07-22 16:00:40 +00:00
parent 0725eb1df2
commit c740e1e3b0
4 changed files with 211 additions and 32 deletions

View File

@ -62,15 +62,18 @@ public final class KdcComm {
// them can also be defined in a realm, which overrides value here.
/**
* max retry time for a single KDC, default Krb5.KDC_RETRY_LIMIT (3)
* max retry time for a single KDC, default Krb5.KDC_RETRY_LIMIT (3),
* Must be > 0.
*/
private static int defaultKdcRetryLimit;
/**
* timeout requesting a ticket from KDC, in millisec, default 30 sec
* timeout requesting a ticket from KDC, in millisec, default
* Krb5.KDC_TIMEOUT (30000). Must be > 0.
*/
private static int defaultKdcTimeout;
/**
* max UDP packet size, default unlimited (-1)
* max UDP packet size, default Krb5.KDC_DEFAULT_UDP_PREF_LIMIT (1465).
* Must be >= 0 and <= Krb5.KDC_HARD_UDP_LIMIT (32700).
*/
private static int defaultUdpPrefLimit;
@ -146,9 +149,9 @@ public final class KdcComm {
timeout = parseTimeString(temp);
temp = cfg.get("libdefaults", "max_retries");
max_retries = parsePositiveIntString(temp);
max_retries = parseNonNegativeIntString(temp);
temp = cfg.get("libdefaults", "udp_preference_limit");
udp_pref_limit = parsePositiveIntString(temp);
udp_pref_limit = parseNonNegativeIntString(temp);
} catch (Exception exc) {
// ignore any exceptions; use default values
if (DEBUG != null) {
@ -157,7 +160,7 @@ public final class KdcComm {
exc.getMessage());
}
}
defaultKdcTimeout = timeout > 0 ? timeout : 30*1000; // 30 seconds
defaultKdcTimeout = timeout > 0 ? timeout : Krb5.KDC_TIMEOUT;
defaultKdcRetryLimit =
max_retries > 0 ? max_retries : Krb5.KDC_RETRY_LIMIT;
@ -175,11 +178,11 @@ public final class KdcComm {
/**
* The instance fields
*/
private String realm;
private final String realm;
public KdcComm(String realm) throws KrbException {
if (realm == null) {
realm = Config.getInstance().getDefaultRealm();
realm = Config.getInstance().getDefaultRealm();
if (realm == null) {
throw new KrbException(Krb5.KRB_ERR_GENERIC,
"Cannot find default realm");
@ -191,11 +194,10 @@ public final class KdcComm {
public byte[] send(KrbKdcReq req)
throws IOException, KrbException {
int udpPrefLimit = getRealmSpecificValue(
realm, "udp_preference_limit", defaultUdpPrefLimit);
realm, "udp_preference_limit", defaultUdpPrefLimit, false);
byte[] obuf = req.encoding();
boolean useTCP = (udpPrefLimit > 0 &&
(obuf != null && obuf.length > udpPrefLimit));
boolean useTCP = obuf != null && obuf.length > udpPrefLimit;
return send(req, useTCP);
}
@ -207,14 +209,6 @@ public final class KdcComm {
return null;
Config cfg = Config.getInstance();
if (realm == null) {
realm = cfg.getDefaultRealm();
if (realm == null) {
throw new KrbException(Krb5.KRB_ERR_GENERIC,
"Cannot find default realm");
}
}
String kdcList = cfg.getKDCList(realm);
if (kdcList == null) {
throw new KrbException("Cannot get kdc for realm " + realm);
@ -296,9 +290,9 @@ public final class KdcComm {
int port = Krb5.KDC_INET_DEFAULT_PORT;
int retries = getRealmSpecificValue(
realm, "max_retries", defaultKdcRetryLimit);
realm, "max_retries", defaultKdcRetryLimit, true);
int timeout = getRealmSpecificValue(
realm, "kdc_timeout", defaultKdcTimeout);
realm, "kdc_timeout", defaultKdcTimeout, true);
if (badPolicy == BpType.TRY_LESS &&
KdcAccessibility.isBad(tempKdc)) {
if (retries > tryLessMaxRetries) {
@ -339,7 +333,7 @@ public final class KdcComm {
}
}
if (portStr != null) {
int tempPort = parsePositiveIntString(portStr);
int tempPort = parseNonNegativeIntString(portStr);
if (tempPort > 0)
port = tempPort;
}
@ -444,10 +438,10 @@ public final class KdcComm {
return -1;
}
if (s.endsWith("s")) {
int seconds = parsePositiveIntString(s.substring(0, s.length()-1));
int seconds = parseNonNegativeIntString(s.substring(0, s.length()-1));
return (seconds < 0) ? -1 : (seconds*1000);
} else {
return parsePositiveIntString(s);
return parseNonNegativeIntString(s);
}
}
@ -461,9 +455,11 @@ public final class KdcComm {
* the global setting if null
* @param key the key for the setting
* @param defValue default value
* @param mustBePositive true if value must be >0, false if value must be >=0
* @return a value for the key
*/
private int getRealmSpecificValue(String realm, String key, int defValue) {
private int getRealmSpecificValue(String realm, String key, int defValue,
boolean mustBePositive) {
int v = defValue;
if (realm == null) return v;
@ -475,18 +471,22 @@ public final class KdcComm {
if (key.equals("kdc_timeout")) {
temp = parseTimeString(value);
} else {
temp = parsePositiveIntString(value);
temp = parseNonNegativeIntString(value);
}
} catch (Exception exc) {
// Ignored, defValue will be picked up
}
if (temp > 0) v = temp;
if (mustBePositive) {
if (temp > 0) v = temp;
} else {
if (temp >= 0) v = temp;
}
return v;
}
private static int parsePositiveIntString(String intString) {
private static int parseNonNegativeIntString(String intString) {
if (intString == null)
return -1;

View File

@ -134,6 +134,7 @@ public class Krb5 {
// number of retries before giving up
public static final int KDC_RETRY_LIMIT = 3;
public static final int KDC_TIMEOUT = 30000;
public static final int KDC_DEFAULT_UDP_PREF_LIMIT = 1465;
public static final int KDC_HARD_UDP_LIMIT = 32700;

View File

@ -38,7 +38,7 @@ import sun.security.krb5.Config;
/*
* @test
* @bug 8164656 8181461 8194486
* @bug 8164656 8181461 8194486 8333772
* @summary krb5.kdc.bad.policy test
* @library /test/lib
* @run main jdk.test.lib.FileInstaller TestHosts TestHosts
@ -219,13 +219,13 @@ public class KdcPolicy {
inDefaults += "udp_preference_limit = 10000\n";
} else if (r.nextBoolean()) {
inRealm += " udp_preference_limit = 10000\n";
inDefaults += "udp_preference_limit = 1\n";
inDefaults += "udp_preference_limit = 0\n";
} // else no settings means UDP
} else {
if (r.nextBoolean()) {
inDefaults += "udp_preference_limit = 1\n";
inDefaults += "udp_preference_limit = 0\n";
} else {
inRealm += " udp_preference_limit = 1\n";
inRealm += " udp_preference_limit = 0\n";
inDefaults += "udp_preference_limit = 10000\n";
}
}

View File

@ -0,0 +1,178 @@
/*
* Copyright (c) 2024, 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 java.io.*;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import jdk.test.lib.Asserts;
import sun.security.krb5.Config;
/*
* @test
* @bug 8333772
* @summary check krb5.conf reading on default and realm-specific values
* @library /test/lib
*/
public class RealmSpecificValues {
static DebugMatcher cm = new DebugMatcher();
public static void main(String[] args) throws Exception {
System.setProperty("sun.security.krb5.debug", "true");
System.setProperty("java.security.krb5.conf", "alternative-krb5.conf");
// Defaults
writeConf(-1, -1, -1, -1, -1, -1);
test(true, 3, 30000);
// Below has settings. For each setting we provide 3 cases:
// 1. Set in defaults, 2, set in realms, 3, both
// udp = 0 is useful
writeConf(0, -1, -1, -1, -1, -1);
test(false, 3, 30000);
writeConf(-1, -1, -1, 0, -1, -1);
test(false, 3, 30000);
writeConf(1, -1, -1, 0, -1, -1);
test(false, 3, 30000);
// max_retries = 0 is ignored
writeConf(-1, 0, -1, -1, -1, -1);
test(true, 3, 30000);
writeConf(-1, -1, -1, -1, 0, -1);
test(true, 3, 30000);
writeConf(-1, 6, -1, -1, 0, -1); // Note: 0 is ignored, it does not reset to default
test(true, 6, 30000);
// max_retries = 1 is useful
writeConf(-1, 1, -1, -1, -1, -1);
test(true, 1, 30000);
writeConf(-1, -1, -1, -1, 1, -1);
test(true, 1, 30000);
writeConf(-1, 3, -1, -1, 1, -1);
test(true, 1, 30000);
// timeout = 0 is ignored
writeConf(-1, -1, 0, -1, -1, -1);
test(true, 3, 30000);
writeConf(-1, -1, -1, -1, -1, 0);
test(true, 3, 30000);
writeConf(-1, -1, 10000, -1, -1, 0);
test(true, 3, 10000);
// timeout > 0 is useful
writeConf(-1, -1, 10000, -1, -1, -1);
test(true, 3, 10000);
writeConf(-1, -1, -1, -1, -1, 10000);
test(true, 3, 10000);
writeConf(-1, -1, 20000, -1, -1, 10000);
test(true, 3, 10000);
}
static void writeConf(int limit, int retries, int timeout,
int limitR, int retriesR, int timeoutR) throws Exception {
String inDefaults = "";
if (limit >= 0) inDefaults += "udp_preference_limit = " + limit + "\n";
if (retries >= 0) inDefaults += "max_retries = " + retries + "\n";
if (timeout >= 0) inDefaults += "kdc_timeout = " + timeout + "\n";
String inRealm = "";
if (limitR >= 0) inRealm += "udp_preference_limit = " + limitR + "\n";
if (retriesR >= 0) inRealm += "max_retries = " + retriesR + "\n";
if (timeoutR >= 0) inRealm += "kdc_timeout = " + timeoutR + "\n";
String conf = "[libdefaults]\n" +
"default_realm = " + OneKDC.REALM + "\n" +
inDefaults +
"\n" +
"[realms]\n" +
OneKDC.REALM + " = {\n" +
"kdc = " + OneKDC.KDCHOST + ":12345\n" +
inRealm +
"}\n";
Files.writeString(Paths.get("alternative-krb5.conf"), conf);
}
static void test(boolean isUDP, int retries, int timeout) throws Exception {
PrintStream oldErr = System.err;
ByteArrayOutputStream bo = new ByteArrayOutputStream();
System.setErr(new PrintStream(bo));
try {
Config.refresh();
Context.fromUserPass(OneKDC.USER, OneKDC.PASS, false);
} catch (Exception e) {
// will happen
} finally {
System.setErr(oldErr);
}
String[] lines = new String(bo.toByteArray()).split("\n");
for (String line: lines) {
if (cm.match(line)) {
System.out.println(line);
Asserts.assertEQ(cm.isUDP(), isUDP);
Asserts.assertEQ(cm.timeout(), timeout);
Asserts.assertEQ(cm.retries(), retries);
return;
}
}
Asserts.fail("Should not reach here");
}
/**
* A helper class to match the krb5 debug output:
* >>> KrbKdcReq send: kdc=kdc.rabbit.hole TCP:12345, timeout=30000,
* number of retries =3, #bytes=141
*/
static class DebugMatcher {
static final Pattern re = Pattern.compile(
">>> KrbKdcReq send: kdc=\\S+ (TCP|UDP):\\d+, " +
"timeout=(\\d+), number of retries\\s*=(\\d+)");
Matcher matcher;
boolean match(String line) {
matcher = re.matcher(line);
return matcher.find();
}
boolean isUDP() {
return matcher.group(1).equals("UDP");
}
int timeout() {
return Integer.parseInt(matcher.group(2));
}
int retries() {
return Integer.parseInt(matcher.group(3));
}
}
}