From 9544f5225c9f16f4d9eb08d32d1f6bce12e5b139 Mon Sep 17 00:00:00 2001 From: Weijun Wang Date: Thu, 17 Sep 2015 18:41:05 -0700 Subject: [PATCH 1/9] 8134297: NPE in GSSNameElement nameType check Reviewed-by: xuelei --- .../classes/sun/security/jgss/wrapper/GSSNameElement.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jdk/src/java.security.jgss/share/classes/sun/security/jgss/wrapper/GSSNameElement.java b/jdk/src/java.security.jgss/share/classes/sun/security/jgss/wrapper/GSSNameElement.java index d09ca8d6a16..3463fd69dfa 100644 --- a/jdk/src/java.security.jgss/share/classes/sun/security/jgss/wrapper/GSSNameElement.java +++ b/jdk/src/java.security.jgss/share/classes/sun/security/jgss/wrapper/GSSNameElement.java @@ -159,7 +159,9 @@ public class GSSNameElement implements GSSNameSpi { int atPos = krbName.lastIndexOf('@'); if (atPos != -1) { String atRealm = krbName.substring(atPos); - if (nameType.equals(GSSUtil.NT_GSS_KRB5_PRINCIPAL) + // getNativeNameType() can modify NT_GSS_KRB5_PRINCIPAL to null + if ((nameType == null + || nameType.equals(GSSUtil.NT_GSS_KRB5_PRINCIPAL)) && new String(nameBytes).endsWith(atRealm)) { // Created from Kerberos name with realm, no need to check } else { From 0edc09be0ce2f4c2dd3a96298e09d4735ab6e890 Mon Sep 17 00:00:00 2001 From: Vadim Pakhnushev Date: Tue, 20 Oct 2015 12:08:44 +0300 Subject: [PATCH 2/9] 8139008: Better state table management Reviewed-by: prr, srl, mschoene --- .../share/native/libfontmanager/layout/StateTableProcessor2.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/jdk/src/java.desktop/share/native/libfontmanager/layout/StateTableProcessor2.cpp b/jdk/src/java.desktop/share/native/libfontmanager/layout/StateTableProcessor2.cpp index 9aa097a6f52..ab74b239d76 100644 --- a/jdk/src/java.desktop/share/native/libfontmanager/layout/StateTableProcessor2.cpp +++ b/jdk/src/java.desktop/share/native/libfontmanager/layout/StateTableProcessor2.cpp @@ -60,6 +60,7 @@ StateTableProcessor2::StateTableProcessor2(const LEReferenceToentryTableOffset); classTable = LEReferenceTo(stHeader, success, classTableOffset); + if (LE_FAILURE(success)) return; format = SWAPW(classTable->format); stateArray = LEReferenceToArrayOf(stHeader, success, stateArrayOffset, LE_UNBOUNDED_ARRAY); From fc6a5d3bd2bb830fda0949944c1ba734d0deed1f Mon Sep 17 00:00:00 2001 From: Anthony Scarpino Date: Mon, 21 Dec 2015 10:43:40 -0800 Subject: [PATCH 3/9] 8143945: Better GCM validation Reviewed-by: xuelei, mullan --- .../com/sun/crypto/provider/GaloisCounterMode.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/jdk/src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java b/jdk/src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java index 9f8f54a1823..87fa4a383f3 100644 --- a/jdk/src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java +++ b/jdk/src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java @@ -512,11 +512,17 @@ final class GaloisCounterMode extends FeedbackCipher { byte[] sOut = new byte[s.length]; GCTR gctrForSToTag = new GCTR(embeddedCipher, this.preCounterBlock); gctrForSToTag.doFinal(s, 0, s.length, sOut, 0); + + // check entire authentication tag for time-consistency + int mismatch = 0; for (int i = 0; i < tagLenBytes; i++) { - if (tag[i] != sOut[i]) { - throw new AEADBadTagException("Tag mismatch!"); - } + mismatch |= tag[i] ^ sOut[i]; } + + if (mismatch != 0) { + throw new AEADBadTagException("Tag mismatch!"); + } + return len; } From e171854b1c74dfa2ea9ab5ff9982ff8050247b58 Mon Sep 17 00:00:00 2001 From: Valerie Peng Date: Wed, 23 Dec 2015 02:31:34 +0000 Subject: [PATCH 4/9] 8138593: Make DSA more fair Changed nounce K generation to FIPS 186-4 B2.1 Reviewed-by: mullan --- .../classes/sun/security/provider/DSA.java | 252 ++---------------- .../TestInitSignWithMyOwnRandom.java | 6 +- .../sun/security/provider/DSA/TestDSA2.java | 4 +- 3 files changed, 30 insertions(+), 232 deletions(-) diff --git a/jdk/src/java.base/share/classes/sun/security/provider/DSA.java b/jdk/src/java.base/share/classes/sun/security/provider/DSA.java index 6f8c27a38c0..a25949742bb 100644 --- a/jdk/src/java.base/share/classes/sun/security/provider/DSA.java +++ b/jdk/src/java.base/share/classes/sun/security/provider/DSA.java @@ -106,6 +106,18 @@ abstract class DSA extends SignatureSpi { this.p1363Format = p1363Format; } + private static void checkKey(DSAParams params, int digestLen, String mdAlgo) + throws InvalidKeyException { + // FIPS186-3 states in sec4.2 that a hash function which provides + // a lower security strength than the (L, N) pair ordinarily should + // not be used. + int valueN = params.getQ().bitLength(); + if (valueN > digestLen) { + throw new InvalidKeyException("The security strength of " + + mdAlgo + " digest algorithm is not sufficient for this key size"); + } + } + /** * Initialize the DSA object with a DSA private key. * @@ -130,6 +142,12 @@ abstract class DSA extends SignatureSpi { throw new InvalidKeyException("DSA private key lacks parameters"); } + // check key size against hash output size for signing + // skip this check for verification to minimize impact on existing apps + if (md.getAlgorithm() != "NullDigest20") { + checkKey(params, md.getDigestLength()*8, md.getAlgorithm()); + } + this.params = params; this.presetX = priv.getX(); this.presetY = null; @@ -160,7 +178,6 @@ abstract class DSA extends SignatureSpi { if (params == null) { throw new InvalidKeyException("DSA public key lacks parameters"); } - this.params = params; this.presetY = pub.getY(); this.presetX = null; @@ -406,20 +423,13 @@ abstract class DSA extends SignatureSpi { return t5.mod(q); } - // NOTE: This following impl is defined in FIPS 186-3 AppendixB.2.2. - // Original DSS algos such as SHA1withDSA and RawDSA uses a different - // algorithm defined in FIPS 186-1 Sec3.2, and thus need to override this. + // NOTE: This following impl is defined in FIPS 186-4 AppendixB.2.1. protected BigInteger generateK(BigInteger q) { SecureRandom random = getSigningRandom(); - byte[] kValue = new byte[q.bitLength()/8]; + byte[] kValue = new byte[(q.bitLength() + 7)/8 + 8]; - while (true) { - random.nextBytes(kValue); - BigInteger k = new BigInteger(1, kValue).mod(q); - if (k.signum() > 0 && k.compareTo(q) < 0) { - return k; - } - } + random.nextBytes(kValue); + return new BigInteger(1, kValue).mod(q.subtract(BigInteger.ONE)).add(BigInteger.ONE); } // Use the application-specified SecureRandom Object if provided. @@ -504,222 +514,10 @@ abstract class DSA extends SignatureSpi { } } - static class LegacyDSA extends DSA { - /* The random seed used to generate k */ - private int[] kSeed; - /* The random seed used to generate k (specified by application) */ - private byte[] kSeedAsByteArray; - /* - * The random seed used to generate k - * (prevent the same Kseed from being used twice in a row - */ - private int[] kSeedLast; - - public LegacyDSA(MessageDigest md) throws NoSuchAlgorithmException { - this(md, false); - } - - private LegacyDSA(MessageDigest md, boolean p1363Format) - throws NoSuchAlgorithmException { - super(md, p1363Format); - } - - @Deprecated - protected void engineSetParameter(String key, Object param) { - if (key.equals("KSEED")) { - if (param instanceof byte[]) { - kSeed = byteArray2IntArray((byte[])param); - kSeedAsByteArray = (byte[])param; - } else { - debug("unrecognized param: " + key); - throw new InvalidParameterException("kSeed not a byte array"); - } - } else { - throw new InvalidParameterException("Unsupported parameter"); - } - } - - @Deprecated - protected Object engineGetParameter(String key) { - if (key.equals("KSEED")) { - return kSeedAsByteArray; - } else { - return null; - } - } - - /* - * Please read bug report 4044247 for an alternative, faster, - * NON-FIPS approved method to generate K - */ - @Override - protected BigInteger generateK(BigInteger q) { - BigInteger k = null; - - // The application specified a kSeed for us to use. - // Note: we dis-allow usage of the same Kseed twice in a row - if (kSeed != null && !Arrays.equals(kSeed, kSeedLast)) { - k = generateKUsingKSeed(kSeed, q); - if (k.signum() > 0 && k.compareTo(q) < 0) { - kSeedLast = kSeed.clone(); - return k; - } - } - - // The application did not specify a Kseed for us to use. - // We'll generate a new Kseed by getting random bytes from - // a SecureRandom object. - SecureRandom random = getSigningRandom(); - - while (true) { - int[] seed = new int[5]; - - for (int i = 0; i < 5; i++) seed[i] = random.nextInt(); - - k = generateKUsingKSeed(seed, q); - if (k.signum() > 0 && k.compareTo(q) < 0) { - kSeedLast = seed; - return k; - } - } - } - - /** - * Compute k for the DSA signature as defined in the original DSS, - * i.e. FIPS186. - * - * @param seed the seed for generating k. This seed should be - * secure. This is what is referred to as the KSEED in the DSA - * specification. - * - * @param g the g parameter from the DSA key pair. - */ - private BigInteger generateKUsingKSeed(int[] seed, BigInteger q) { - - // check out t in the spec. - int[] t = { 0xEFCDAB89, 0x98BADCFE, 0x10325476, - 0xC3D2E1F0, 0x67452301 }; - // - int[] tmp = SHA_7(seed, t); - byte[] tmpBytes = new byte[tmp.length * 4]; - for (int i = 0; i < tmp.length; i++) { - int k = tmp[i]; - for (int j = 0; j < 4; j++) { - tmpBytes[(i * 4) + j] = (byte) (k >>> (24 - (j * 8))); - } - } - BigInteger k = new BigInteger(1, tmpBytes).mod(q); - return k; - } - - // Constants for each round - private static final int round1_kt = 0x5a827999; - private static final int round2_kt = 0x6ed9eba1; - private static final int round3_kt = 0x8f1bbcdc; - private static final int round4_kt = 0xca62c1d6; - - /** - * Computes set 1 thru 7 of SHA-1 on m1. */ - static int[] SHA_7(int[] m1, int[] h) { - - int[] W = new int[80]; - System.arraycopy(m1,0,W,0,m1.length); - int temp = 0; - - for (int t = 16; t <= 79; t++){ - temp = W[t-3] ^ W[t-8] ^ W[t-14] ^ W[t-16]; - W[t] = ((temp << 1) | (temp >>>(32 - 1))); - } - - int a = h[0],b = h[1],c = h[2], d = h[3], e = h[4]; - for (int i = 0; i < 20; i++) { - temp = ((a<<5) | (a>>>(32-5))) + - ((b&c)|((~b)&d))+ e + W[i] + round1_kt; - e = d; - d = c; - c = ((b<<30) | (b>>>(32-30))); - b = a; - a = temp; - } - - // Round 2 - for (int i = 20; i < 40; i++) { - temp = ((a<<5) | (a>>>(32-5))) + - (b ^ c ^ d) + e + W[i] + round2_kt; - e = d; - d = c; - c = ((b<<30) | (b>>>(32-30))); - b = a; - a = temp; - } - - // Round 3 - for (int i = 40; i < 60; i++) { - temp = ((a<<5) | (a>>>(32-5))) + - ((b&c)|(b&d)|(c&d)) + e + W[i] + round3_kt; - e = d; - d = c; - c = ((b<<30) | (b>>>(32-30))); - b = a; - a = temp; - } - - // Round 4 - for (int i = 60; i < 80; i++) { - temp = ((a<<5) | (a>>>(32-5))) + - (b ^ c ^ d) + e + W[i] + round4_kt; - e = d; - d = c; - c = ((b<<30) | (b>>>(32-30))); - b = a; - a = temp; - } - int[] md = new int[5]; - md[0] = h[0] + a; - md[1] = h[1] + b; - md[2] = h[2] + c; - md[3] = h[3] + d; - md[4] = h[4] + e; - return md; - } - - /* - * Utility routine for converting a byte array into an int array - */ - private int[] byteArray2IntArray(byte[] byteArray) { - - int j = 0; - byte[] newBA; - int mod = byteArray.length % 4; - - // guarantee that the incoming byteArray is a multiple of 4 - // (pad with 0's) - switch (mod) { - case 3: newBA = new byte[byteArray.length + 1]; break; - case 2: newBA = new byte[byteArray.length + 2]; break; - case 1: newBA = new byte[byteArray.length + 3]; break; - default: newBA = new byte[byteArray.length + 0]; break; - } - System.arraycopy(byteArray, 0, newBA, 0, byteArray.length); - - // copy each set of 4 bytes in the byte array into an integer - int[] newSeed = new int[newBA.length / 4]; - for (int i = 0; i < newBA.length; i += 4) { - newSeed[j] = newBA[i + 3] & 0xFF; - newSeed[j] |= (newBA[i + 2] << 8) & 0xFF00; - newSeed[j] |= (newBA[i + 1] << 16) & 0xFF0000; - newSeed[j] |= (newBA[i + 0] << 24) & 0xFF000000; - j++; - } - - return newSeed; - } - } - /** * Standard SHA1withDSA implementation. */ - public static final class SHA1withDSA extends LegacyDSA { + public static final class SHA1withDSA extends DSA { public SHA1withDSA() throws NoSuchAlgorithmException { super(MessageDigest.getInstance("SHA-1")); } @@ -728,7 +526,7 @@ abstract class DSA extends SignatureSpi { /** * SHA1withDSA implementation that uses the IEEE P1363 format. */ - public static final class SHA1withDSAinP1363Format extends LegacyDSA { + public static final class SHA1withDSAinP1363Format extends DSA { public SHA1withDSAinP1363Format() throws NoSuchAlgorithmException { super(MessageDigest.getInstance("SHA-1"), true); } @@ -741,7 +539,7 @@ abstract class DSA extends SignatureSpi { * not, a SignatureException is thrown when sign()/verify() is called * per JCA spec. */ - static class Raw extends LegacyDSA { + static class Raw extends DSA { // Internal special-purpose MessageDigest impl for RawDSA // Only override whatever methods used // NOTE: no clone support diff --git a/jdk/test/java/security/Signature/TestInitSignWithMyOwnRandom.java b/jdk/test/java/security/Signature/TestInitSignWithMyOwnRandom.java index 2a2121e249d..0d2d7fd35a4 100644 --- a/jdk/test/java/security/Signature/TestInitSignWithMyOwnRandom.java +++ b/jdk/test/java/security/Signature/TestInitSignWithMyOwnRandom.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2002, 2003, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2002, 2015, 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 @@ -55,9 +55,9 @@ class TestRandomSource extends SecureRandom { int count = 0; - public int nextInt() { + @Override + public void nextBytes(byte[] rs) { count++; - return 0; } public boolean isUsed() { diff --git a/jdk/test/sun/security/provider/DSA/TestDSA2.java b/jdk/test/sun/security/provider/DSA/TestDSA2.java index 00691535ac1..320acce4880 100644 --- a/jdk/test/sun/security/provider/DSA/TestDSA2.java +++ b/jdk/test/sun/security/provider/DSA/TestDSA2.java @@ -60,8 +60,8 @@ public class TestDSA2 { boolean[] expectedToPass = { true, true, true, true, true, true, true, true }; test(1024, expectedToPass); - boolean[] expectedToPass2 = { true, true, true, true, - true, true, true, true }; + boolean[] expectedToPass2 = { true, false, true, true, + true, false, true, true }; test(2048, expectedToPass2); } From 29aeb056925592e0c463a46149d12df9b7884cbc Mon Sep 17 00:00:00 2001 From: Phil Race Date: Wed, 13 Jan 2016 11:23:25 -0800 Subject: [PATCH 5/9] 8146498: Better device table adjustments Reviewed-by: vadim, mschoene --- .../share/native/libfontmanager/layout/DeviceTables.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/jdk/src/java.desktop/share/native/libfontmanager/layout/DeviceTables.cpp b/jdk/src/java.desktop/share/native/libfontmanager/layout/DeviceTables.cpp index df4df194fcf..7ea3032fe97 100644 --- a/jdk/src/java.desktop/share/native/libfontmanager/layout/DeviceTables.cpp +++ b/jdk/src/java.desktop/share/native/libfontmanager/layout/DeviceTables.cpp @@ -45,9 +45,12 @@ const le_uint16 DeviceTable::fieldBits[] = { 2, 4, 8}; le_int16 DeviceTable::getAdjustment(const LEReferenceTo&base, le_uint16 ppem, LEErrorCode &success) const { + le_int16 result = 0; + if (LE_FAILURE(success)) { + return result; + } le_uint16 start = SWAPW(startSize); le_uint16 format = SWAPW(deltaFormat) - 1; - le_int16 result = 0; if (ppem >= start && ppem <= SWAPW(endSize) && format < FORMAT_COUNT) { le_uint16 sizeIndex = ppem - start; From 11920c3315e20a6948bea7bb7ccad45601271fca Mon Sep 17 00:00:00 2001 From: Phil Race Date: Wed, 13 Jan 2016 11:24:11 -0800 Subject: [PATCH 6/9] 8146494: Better ligature substitution Reviewed-by: vadim, mschoene --- .../layout/LigatureSubstProc.cpp | 20 +++++++++++++++++++ .../layout/LigatureSubstProc2.cpp | 20 +++++++++++++++++-- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/jdk/src/java.desktop/share/native/libfontmanager/layout/LigatureSubstProc.cpp b/jdk/src/java.desktop/share/native/libfontmanager/layout/LigatureSubstProc.cpp index 5a94563fa5b..76131fd2193 100644 --- a/jdk/src/java.desktop/share/native/libfontmanager/layout/LigatureSubstProc.cpp +++ b/jdk/src/java.desktop/share/native/libfontmanager/layout/LigatureSubstProc.cpp @@ -71,6 +71,10 @@ ByteOffset LigatureSubstitutionProcessor::processStateEntry(LEGlyphStorage &glyp { LEErrorCode success = LE_NO_ERROR; const LigatureSubstitutionStateEntry *entry = entryTable.getAlias(index, success); + if (LE_FAILURE(success)) { + currGlyph++; + return 0; + } ByteOffset newState = SWAPW(entry->newStateOffset); le_uint16 flags = SWAPW(entry->flags); @@ -91,6 +95,10 @@ ByteOffset LigatureSubstitutionProcessor::processStateEntry(LEGlyphStorage &glyp if (actionOffset != 0) { LEReferenceTo ap(stHeader, success, actionOffset); + if (LE_FAILURE(success)) { + currGlyph++; + return newState; + } LigatureActionEntry action; le_int32 offset, i = 0, j = 0; le_int32 stack[nComponents]; @@ -101,6 +109,10 @@ ByteOffset LigatureSubstitutionProcessor::processStateEntry(LEGlyphStorage &glyp if (j++ > 0) { ap.addObject(success); + if (LE_FAILURE(success)) { + currGlyph++; + return newState; + } } action = SWAPL(*ap.getAlias()); @@ -124,9 +136,17 @@ ByteOffset LigatureSubstitutionProcessor::processStateEntry(LEGlyphStorage &glyp return newState; // get out! bad font } i += SWAPW(offsetTable.getObject(LE_GET_GLYPH(glyphStorage[componentGlyph]), success)); + if (LE_FAILURE(success)) { + currGlyph++; + return newState; + } if (action & (lafLast | lafStore)) { LEReferenceTo ligatureOffset(stHeader, success, i); + if (LE_FAILURE(success)) { + currGlyph++; + return newState; + } TTGlyphID ligatureGlyph = SWAPW(*ligatureOffset.getAlias()); glyphStorage[componentGlyph] = LE_SET_GLYPH(glyphStorage[componentGlyph], ligatureGlyph); diff --git a/jdk/src/java.desktop/share/native/libfontmanager/layout/LigatureSubstProc2.cpp b/jdk/src/java.desktop/share/native/libfontmanager/layout/LigatureSubstProc2.cpp index 77e907322ed..1a4709e1d81 100644 --- a/jdk/src/java.desktop/share/native/libfontmanager/layout/LigatureSubstProc2.cpp +++ b/jdk/src/java.desktop/share/native/libfontmanager/layout/LigatureSubstProc2.cpp @@ -95,6 +95,10 @@ le_uint16 LigatureSubstitutionProcessor2::processStateEntry(LEGlyphStorage &glyp if (actionOffset != 0) { LEReferenceTo ap(stHeader, success, ligActionOffset); // byte offset + if (LE_FAILURE(success)) { + currGlyph+= dir; + return nextStateIndex; + } ap.addObject(ligActionIndex, success); LEReferenceToArrayOf ligatureTable(stHeader, success, ligatureOffset, LE_UNBOUNDED_ARRAY); LigatureActionEntry action; @@ -104,8 +108,8 @@ le_uint16 LigatureSubstitutionProcessor2::processStateEntry(LEGlyphStorage &glyp LEReferenceToArrayOf componentTable(stHeader, success, componentOffset, LE_UNBOUNDED_ARRAY); if(LE_FAILURE(success)) { - currGlyph+= dir; - return nextStateIndex; // get out! bad font + currGlyph+= dir; + return nextStateIndex; // get out! bad font } do { @@ -114,6 +118,10 @@ le_uint16 LigatureSubstitutionProcessor2::processStateEntry(LEGlyphStorage &glyp if (j++ > 0) { ap.addObject(success); } + if (LE_FAILURE(success)) { + currGlyph+= dir; + return nextStateIndex; + } action = SWAPL(*ap.getAlias()); @@ -129,9 +137,17 @@ le_uint16 LigatureSubstitutionProcessor2::processStateEntry(LEGlyphStorage &glyp return nextStateIndex; // get out! bad font } i += SWAPW(componentTable(LE_GET_GLYPH(glyphStorage[componentGlyph]) + (SignExtend(offset, lafComponentOffsetMask)),success)); + if (LE_FAILURE(success)) { + currGlyph+= dir; + return nextStateIndex; + } if (action & (lafLast | lafStore)) { TTGlyphID ligatureGlyph = SWAPW(ligatureTable(i,success)); + if (LE_FAILURE(success)) { + currGlyph+= dir; + return nextStateIndex; + } glyphStorage[componentGlyph] = LE_SET_GLYPH(glyphStorage[componentGlyph], ligatureGlyph); if(mm==nComponents) { LE_DEBUG_BAD_FONT("exceeded nComponents"); From 0aa59442eb94dc59551dbebca4ee4504cd0d7d37 Mon Sep 17 00:00:00 2001 From: Chris Hegarty Date: Wed, 20 Jan 2016 20:51:45 +0000 Subject: [PATCH 7/9] 8129952: Ensure thread consistency Reviewed-by: alanb, ahgross, skoivu --- .../classes/java/io/ObjectInputStream.java | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/jdk/src/java.base/share/classes/java/io/ObjectInputStream.java b/jdk/src/java.base/share/classes/java/io/ObjectInputStream.java index dbf10bd757d..60fc7947435 100644 --- a/jdk/src/java.base/share/classes/java/io/ObjectInputStream.java +++ b/jdk/src/java.base/share/classes/java/io/ObjectInputStream.java @@ -1915,6 +1915,8 @@ public class ObjectInputStream if (obj == null || handles.lookupException(passHandle) != null) { defaultReadFields(null, slotDesc); // skip field values } else if (slotDesc.hasReadObjectMethod()) { + ThreadDeath t = null; + boolean reset = false; SerialCallbackContext oldContext = curContext; if (oldContext != null) oldContext.check(); @@ -1933,10 +1935,19 @@ public class ObjectInputStream */ handles.markException(passHandle, ex); } finally { - curContext.setUsed(); - if (oldContext!= null) - oldContext.check(); - curContext = oldContext; + do { + try { + curContext.setUsed(); + if (oldContext!= null) + oldContext.check(); + curContext = oldContext; + reset = true; + } catch (ThreadDeath x) { + t = x; // defer until reset is true + } + } while (!reset); + if (t != null) + throw t; } /* From 83e0997bbda9529859c37fc302f665ad5ac047c9 Mon Sep 17 00:00:00 2001 From: Shanliang Jiang Date: Fri, 22 Jan 2016 13:27:09 +0100 Subject: [PATCH 8/9] 8144430: Improve JMX connections Reviewed-by: dfuchs, jbachorik, skoivu, ahgross --- .../classes/java/io/ObjectInputStream.java | 40 ++++++-- .../misc/JavaObjectInputStreamAccess.java | 41 ++++++++ .../misc/ObjectStreamClassValidator.java | 42 ++++++++ .../jdk/internal/misc/SharedSecrets.java | 16 +++- .../remote/rmi/RMIConnectorServer.java | 18 +++- .../remote/rmi/RMIJRMPServerImpl.java | 88 ++++++++++++++++- .../jmxremote/ConnectorBootstrap.java | 6 ++ .../rmi/server/DeserializationChecker.java | 93 ++++++++++++++++++ .../sun/rmi/server/MarshalInputStream.java | 42 +++++++- .../sun/rmi/server/UnicastServerRef.java | 95 +++++++++++++++++-- 10 files changed, 455 insertions(+), 26 deletions(-) create mode 100644 jdk/src/java.base/share/classes/jdk/internal/misc/JavaObjectInputStreamAccess.java create mode 100644 jdk/src/java.base/share/classes/jdk/internal/misc/ObjectStreamClassValidator.java create mode 100644 jdk/src/java.rmi/share/classes/sun/rmi/server/DeserializationChecker.java diff --git a/jdk/src/java.base/share/classes/java/io/ObjectInputStream.java b/jdk/src/java.base/share/classes/java/io/ObjectInputStream.java index 60fc7947435..cd040d3d7e5 100644 --- a/jdk/src/java.base/share/classes/java/io/ObjectInputStream.java +++ b/jdk/src/java.base/share/classes/java/io/ObjectInputStream.java @@ -40,6 +40,9 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import static java.io.ObjectStreamClass.processQueue; +import jdk.internal.misc.JavaObjectInputStreamAccess; +import jdk.internal.misc.ObjectStreamClassValidator; +import jdk.internal.misc.SharedSecrets; import jdk.internal.misc.Unsafe; import sun.reflect.misc.ReflectUtil; @@ -1509,23 +1512,28 @@ public class ObjectInputStream throws IOException { byte tc = bin.peekByte(); + ObjectStreamClass descriptor; switch (tc) { case TC_NULL: - return (ObjectStreamClass) readNull(); - + descriptor = (ObjectStreamClass) readNull(); + break; case TC_REFERENCE: - return (ObjectStreamClass) readHandle(unshared); - + descriptor = (ObjectStreamClass) readHandle(unshared); + break; case TC_PROXYCLASSDESC: - return readProxyDesc(unshared); - + descriptor = readProxyDesc(unshared); + break; case TC_CLASSDESC: - return readNonProxyDesc(unshared); - + descriptor = readNonProxyDesc(unshared); + break; default: throw new StreamCorruptedException( String.format("invalid type code: %02X", tc)); } + if (descriptor != null) { + validateDescriptor(descriptor); + } + return descriptor; } private boolean isCustomSubclass() { @@ -3658,4 +3666,20 @@ public class ObjectInputStream } } + private void validateDescriptor(ObjectStreamClass descriptor) { + ObjectStreamClassValidator validating = validator; + if (validating != null) { + validating.validateDescriptor(descriptor); + } + } + + // controlled access to ObjectStreamClassValidator + private volatile ObjectStreamClassValidator validator; + + private static void setValidator(ObjectInputStream ois, ObjectStreamClassValidator validator) { + ois.validator = validator; + } + static { + SharedSecrets.setJavaObjectInputStreamAccess(ObjectInputStream::setValidator); + } } diff --git a/jdk/src/java.base/share/classes/jdk/internal/misc/JavaObjectInputStreamAccess.java b/jdk/src/java.base/share/classes/jdk/internal/misc/JavaObjectInputStreamAccess.java new file mode 100644 index 00000000000..c344f8adc7c --- /dev/null +++ b/jdk/src/java.base/share/classes/jdk/internal/misc/JavaObjectInputStreamAccess.java @@ -0,0 +1,41 @@ +/* + * Copyright (c) 2016, 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. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * 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. + */ + +package jdk.internal.misc; + +import java.io.ObjectInputStream; + +/** + * The interface to specify methods for accessing {@code ObjectInputStream} + * @author sjiang + */ +public interface JavaObjectInputStreamAccess { + /** + * Sets a descriptor validating. + * @param ois stream to have the descriptors validated + * @param validator validator used to validate a descriptor. + */ + public void setValidator(ObjectInputStream ois, ObjectStreamClassValidator validator); +} diff --git a/jdk/src/java.base/share/classes/jdk/internal/misc/ObjectStreamClassValidator.java b/jdk/src/java.base/share/classes/jdk/internal/misc/ObjectStreamClassValidator.java new file mode 100644 index 00000000000..2b543a30721 --- /dev/null +++ b/jdk/src/java.base/share/classes/jdk/internal/misc/ObjectStreamClassValidator.java @@ -0,0 +1,42 @@ +/* + * Copyright (c) 2016, 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. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * 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. + */ +package jdk.internal.misc; + +import java.io.ObjectStreamClass; + +/** + * A callback used by {@code ObjectInputStream} to do descriptor validation. + * + * @author sjiang + */ +public interface ObjectStreamClassValidator { + /** + * This method will be called by ObjectInputStream to + * check a descriptor just before creating an object described by this descriptor. + * The object will not be created if this method throws a {@code RuntimeException}. + * @param descriptor descriptor to be checked. + */ + public void validateDescriptor(ObjectStreamClass descriptor); +} diff --git a/jdk/src/java.base/share/classes/jdk/internal/misc/SharedSecrets.java b/jdk/src/java.base/share/classes/jdk/internal/misc/SharedSecrets.java index 3246bde5be1..24dc4ce43a7 100644 --- a/jdk/src/java.base/share/classes/jdk/internal/misc/SharedSecrets.java +++ b/jdk/src/java.base/share/classes/jdk/internal/misc/SharedSecrets.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2002, 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2002, 2016, 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 @@ -29,9 +29,9 @@ import java.lang.module.ModuleDescriptor; import java.util.jar.JarFile; import java.io.Console; import java.io.FileDescriptor; +import java.io.ObjectInputStream; import java.security.ProtectionDomain; import java.security.AccessController; -import jdk.internal.misc.Unsafe; /** A repository of "shared secrets", which are a mechanism for calling implementation-private methods in another package without @@ -63,6 +63,7 @@ public class SharedSecrets { private static JavaAWTAccess javaAWTAccess; private static JavaAWTFontAccess javaAWTFontAccess; private static JavaBeansAccess javaBeansAccess; + private static JavaObjectInputStreamAccess javaObjectInputStreamAccess; public static JavaUtilJarAccess javaUtilJarAccess() { if (javaUtilJarAccess == null) { @@ -262,4 +263,15 @@ public class SharedSecrets { public static void setJavaUtilResourceBundleAccess(JavaUtilResourceBundleAccess access) { javaUtilResourceBundleAccess = access; } + + public static JavaObjectInputStreamAccess getJavaObjectInputStreamAccess() { + if (javaObjectInputStreamAccess == null) { + unsafe.ensureClassInitialized(ObjectInputStream.class); + } + return javaObjectInputStreamAccess; + } + + public static void setJavaObjectInputStreamAccess(JavaObjectInputStreamAccess access) { + javaObjectInputStreamAccess = access; + } } diff --git a/jdk/src/java.management/share/classes/javax/management/remote/rmi/RMIConnectorServer.java b/jdk/src/java.management/share/classes/javax/management/remote/rmi/RMIConnectorServer.java index fe90422c193..a71bef5b403 100644 --- a/jdk/src/java.management/share/classes/javax/management/remote/rmi/RMIConnectorServer.java +++ b/jdk/src/java.management/share/classes/javax/management/remote/rmi/RMIConnectorServer.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2002, 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2002, 2016, 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 @@ -45,6 +45,7 @@ import java.util.Set; import javax.management.InstanceNotFoundException; import javax.management.MBeanServer; +import javax.management.remote.JMXAuthenticator; import javax.management.remote.JMXConnectionNotification; import javax.management.remote.JMXConnector; @@ -99,6 +100,21 @@ public class RMIConnectorServer extends JMXConnectorServer { public static final String RMI_SERVER_SOCKET_FACTORY_ATTRIBUTE = "jmx.remote.rmi.server.socket.factory"; + /** + * Name of the attribute that specifies a list of class names acceptable + * as parameters to the {@link RMIServer#newClient(java.lang.Object) RMIServer.newClient()} + * remote method call. + *

+ * This list of classes should correspond to the transitive closure of the + * credentials class (or classes) used by the installed {@linkplain JMXAuthenticator} + * associated with the {@linkplain RMIServer} implementation. + *

+ * If the attribute is not set, or is null, then any class is + * deemed acceptable. + */ + public static final String CREDENTIAL_TYPES = + "jmx.remote.rmi.server.credential.types"; + /** *

Makes an RMIConnectorServer. * This is equivalent to calling {@link #RMIConnectorServer( diff --git a/jdk/src/java.management/share/classes/javax/management/remote/rmi/RMIJRMPServerImpl.java b/jdk/src/java.management/share/classes/javax/management/remote/rmi/RMIJRMPServerImpl.java index 0e45c966c9c..838c092ca48 100644 --- a/jdk/src/java.management/share/classes/javax/management/remote/rmi/RMIJRMPServerImpl.java +++ b/jdk/src/java.management/share/classes/javax/management/remote/rmi/RMIJRMPServerImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2002, 2007, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2002, 2016, 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 @@ -39,6 +39,13 @@ import javax.security.auth.Subject; import com.sun.jmx.remote.internal.RMIExporter; import com.sun.jmx.remote.util.EnvHelp; +import java.io.ObjectStreamClass; +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import sun.reflect.misc.ReflectUtil; +import sun.rmi.server.DeserializationChecker; import sun.rmi.server.UnicastServerRef; import sun.rmi.server.UnicastServerRef2; @@ -52,6 +59,9 @@ import sun.rmi.server.UnicastServerRef2; * @since 1.5 */ public class RMIJRMPServerImpl extends RMIServerImpl { + + private final ExportedWrapper exportedWrapper; + /** *

Creates a new {@link RMIServer} object that will be exported * on the given port using the given socket factories.

@@ -89,10 +99,31 @@ public class RMIJRMPServerImpl extends RMIServerImpl { this.csf = csf; this.ssf = ssf; this.env = (env == null) ? Collections.emptyMap() : env; + + String[] credentialsTypes + = (String[]) this.env.get(RMIConnectorServer.CREDENTIAL_TYPES); + List types = null; + if (credentialsTypes != null) { + types = new ArrayList<>(); + for (String type : credentialsTypes) { + if (type == null) { + throw new IllegalArgumentException("A credential type is null."); + } + ReflectUtil.checkPackageAccess(type); + types.add(type); + } + } + exportedWrapper = types != null ? + new ExportedWrapper(this, types) : + null; } protected void export() throws IOException { - export(this); + if (exportedWrapper != null) { + export(exportedWrapper); + } else { + export(this); + } } private void export(Remote obj) throws RemoteException { @@ -142,7 +173,11 @@ public class RMIJRMPServerImpl extends RMIServerImpl { * RMIJRMPServerImpl has not been exported yet. */ public Remote toStub() throws IOException { - return RemoteObject.toStub(this); + if (exportedWrapper != null) { + return RemoteObject.toStub(exportedWrapper); + } else { + return RemoteObject.toStub(this); + } } /** @@ -189,11 +224,56 @@ public class RMIJRMPServerImpl extends RMIServerImpl { * server failed. */ protected void closeServer() throws IOException { - unexport(this, true); + if (exportedWrapper != null) { + unexport(exportedWrapper, true); + } else { + unexport(this, true); + } } private final int port; private final RMIClientSocketFactory csf; private final RMIServerSocketFactory ssf; private final Map env; + + private static class ExportedWrapper implements RMIServer, DeserializationChecker { + private final RMIServer impl; + private final List allowedTypes; + + private ExportedWrapper(RMIServer impl, List credentialsTypes) { + this.impl = impl; + allowedTypes = credentialsTypes; + } + + @Override + public String getVersion() throws RemoteException { + return impl.getVersion(); + } + + @Override + public RMIConnection newClient(Object credentials) throws IOException { + return impl.newClient(credentials); + } + + @Override + public void check(Method method, ObjectStreamClass descriptor, + int paramIndex, int callID) { + String type = descriptor.getName(); + if (!allowedTypes.contains(type)) { + throw new ClassCastException("Unsupported type: " + type); + } + } + + @Override + public void checkProxyClass(Method method, String[] ifaces, + int paramIndex, int callID) { + if (ifaces != null && ifaces.length > 0) { + for (String iface : ifaces) { + if (!allowedTypes.contains(iface)) { + throw new ClassCastException("Unsupported type: " + iface); + } + } + } + } + } } diff --git a/jdk/src/java.management/share/classes/sun/management/jmxremote/ConnectorBootstrap.java b/jdk/src/java.management/share/classes/sun/management/jmxremote/ConnectorBootstrap.java index 5ef0f7f1708..477b7786421 100644 --- a/jdk/src/java.management/share/classes/sun/management/jmxremote/ConnectorBootstrap.java +++ b/jdk/src/java.management/share/classes/sun/management/jmxremote/ConnectorBootstrap.java @@ -510,6 +510,9 @@ public final class ConnectorBootstrap { // This RMI server should not keep the VM alive Map env = new HashMap<>(); env.put(RMIExporter.EXPORTER_ATTRIBUTE, new PermanentExporter()); + env.put(RMIConnectorServer.CREDENTIAL_TYPES, new String[]{ + String[].class.getName(), String.class.getName() + }); // The local connector server need only be available via the // loopback connection. @@ -740,6 +743,9 @@ public final class ConnectorBootstrap { PermanentExporter exporter = new PermanentExporter(); env.put(RMIExporter.EXPORTER_ATTRIBUTE, exporter); + env.put(RMIConnectorServer.CREDENTIAL_TYPES, new String[]{ + String[].class.getName(), String.class.getName() + }); boolean useSocketFactory = bindAddress != null && !useSsl; diff --git a/jdk/src/java.rmi/share/classes/sun/rmi/server/DeserializationChecker.java b/jdk/src/java.rmi/share/classes/sun/rmi/server/DeserializationChecker.java new file mode 100644 index 00000000000..8182b10bfbe --- /dev/null +++ b/jdk/src/java.rmi/share/classes/sun/rmi/server/DeserializationChecker.java @@ -0,0 +1,93 @@ +/* + * Copyright (c) 2016, 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. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * 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. + */ + +package sun.rmi.server; + +import java.io.ObjectStreamClass; +import java.lang.reflect.Method; + +/** + * Implementing this interface to have a deserialization control when RMI + * dispatches a remote request. If an exported object implements this interface, + * RMI dispatching mechanism will call the method {@code check} every time + * deserialising a remote object for invoking a method of the exported object. + * + * @author sjiang + */ +public interface DeserializationChecker { + /** + * Will be called to check a descriptor. + * This method may be called 2 times, the first time is when a descriptor is read + * from the stream, the second is just before creating an object described + * by this descriptor. + * + * @param method the method invoked from a remote request. + * @param descriptor The descriptor of the class of any object deserialised + * while deserialising the parameter. The first descriptor will be that of + * the top level object (the concrete class of the parameter itself); + * Subsequent calls with the same {@code method}, {@code paramIndex} and + * {@code callID} will correspond to objects contained in the parameter. + * @param paramIndex an index indicates the position of a parameter in the + * method. This index will be reused for deserialising all + * objects contained in the parameter object. For example, the parameter + * being deserialised is a {@code List}, all deserialisation calls for its + * elements will have same index. + * @param callID a unique ID identifying one + * time method invocation, the same ID is used for deserialization call of + * all parameters within the method. + */ + public void check(Method method, + ObjectStreamClass descriptor, + int paramIndex, + int callID); + + /** + * Will be called to validate a Proxy interfaces from a remote user before loading it. + * @param method the method invoked from a remote request. + * @param ifaces a string table of all interfaces implemented by the proxy to be checked. + * @param paramIndex an index indicates the position of a parameter in the + * method. This index will be reused for deserialising all + * objects contained in the parameter object. For example, the parameter + * being deserialised is a {@code List}, all deserialisation calls for its + * elements will have same index. + * @param callID a unique ID identifying one + * time method invocation, the same ID is used for deserialization call of + * all parameters within the method. + */ + public void checkProxyClass(Method method, + String[] ifaces, + int paramIndex, + int callID); + + /** + * Inform of the completion of parameter deserialisation for a method invocation. + * This is useful if the last parameter is a complex object, like a {@code List} + * which elements are complex object too. + * + * The default implementation does nothing. + * @param callID the ID identifying a method invocation. + */ + public default void end(int callID) {} +} diff --git a/jdk/src/java.rmi/share/classes/sun/rmi/server/MarshalInputStream.java b/jdk/src/java.rmi/share/classes/sun/rmi/server/MarshalInputStream.java index 3453e61f517..59c0be3360e 100644 --- a/jdk/src/java.rmi/share/classes/sun/rmi/server/MarshalInputStream.java +++ b/jdk/src/java.rmi/share/classes/sun/rmi/server/MarshalInputStream.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1996, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1996, 2016, 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 @@ -30,13 +30,13 @@ import java.io.InputStream; import java.io.ObjectInputStream; import java.io.ObjectStreamClass; import java.io.StreamCorruptedException; -import java.net.URL; import java.util.*; import java.security.AccessControlException; import java.security.Permission; - import java.rmi.server.RMIClassLoader; import java.security.PrivilegedAction; +import jdk.internal.misc.ObjectStreamClassValidator; +import jdk.internal.misc.SharedSecrets; /** * MarshalInputStream is an extension of ObjectInputStream. When resolving @@ -54,6 +54,11 @@ import java.security.PrivilegedAction; * @author Peter Jones */ public class MarshalInputStream extends ObjectInputStream { + interface StreamChecker extends ObjectStreamClassValidator { + void checkProxyInterfaceNames(String[] ifaces); + } + + private volatile StreamChecker streamChecker = null; /** * Value of "java.rmi.server.useCodebaseOnly" property, @@ -123,7 +128,7 @@ public class MarshalInputStream extends ObjectInputStream { throws IOException, StreamCorruptedException { super(in); - } + } /** * Returns a callback previously registered via the setDoneCallback @@ -240,6 +245,11 @@ public class MarshalInputStream extends ObjectInputStream { protected Class resolveProxyClass(String[] interfaces) throws IOException, ClassNotFoundException { + StreamChecker checker = streamChecker; + if (checker != null) { + checker.checkProxyInterfaceNames(interfaces); + } + /* * Always read annotation written by MarshalOutputStream. */ @@ -319,4 +329,28 @@ public class MarshalInputStream extends ObjectInputStream { void useCodebaseOnly() { useCodebaseOnly = true; } + + synchronized void setStreamChecker(StreamChecker checker) { + streamChecker = checker; + SharedSecrets.getJavaObjectInputStreamAccess().setValidator(this, checker); + } + @Override + protected ObjectStreamClass readClassDescriptor() throws IOException, + ClassNotFoundException { + ObjectStreamClass descriptor = super.readClassDescriptor(); + + validateDesc(descriptor); + + return descriptor; + } + + private void validateDesc(ObjectStreamClass descriptor) { + StreamChecker checker; + synchronized (this) { + checker = streamChecker; + } + if (checker != null) { + checker.validateDescriptor(descriptor); + } + } } diff --git a/jdk/src/java.rmi/share/classes/sun/rmi/server/UnicastServerRef.java b/jdk/src/java.rmi/share/classes/sun/rmi/server/UnicastServerRef.java index b608f237cf5..3c57aa4b2d2 100644 --- a/jdk/src/java.rmi/share/classes/sun/rmi/server/UnicastServerRef.java +++ b/jdk/src/java.rmi/share/classes/sun/rmi/server/UnicastServerRef.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1996, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1996, 2016, 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 @@ -28,7 +28,7 @@ package sun.rmi.server; import java.io.IOException; import java.io.ObjectInput; import java.io.ObjectOutput; -import java.io.PrintStream; +import java.io.ObjectStreamClass; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.rmi.MarshalException; @@ -52,6 +52,7 @@ import java.util.Date; import java.util.HashMap; import java.util.Map; import java.util.WeakHashMap; +import java.util.concurrent.atomic.AtomicInteger; import sun.rmi.runtime.Log; import sun.rmi.transport.LiveRef; import sun.rmi.transport.Target; @@ -116,6 +117,8 @@ public class UnicastServerRef extends UnicastRef private static final Map,?> withoutSkeletons = Collections.synchronizedMap(new WeakHashMap,Void>()); + private final AtomicInteger methodCallIDCount = new AtomicInteger(0); + /** * Create a new (empty) Unicast server remote reference. */ @@ -297,14 +300,11 @@ public class UnicastServerRef extends UnicastRef logCall(obj, method); // unmarshal parameters - Class[] types = method.getParameterTypes(); - Object[] params = new Object[types.length]; + Object[] params = null; try { unmarshalCustomCallData(in); - for (int i = 0; i < types.length; i++) { - params[i] = unmarshalValue(types[i], in); - } + params = unmarshalParameters(obj, method, marshalStream); } catch (java.io.IOException e) { throw new UnmarshalException( "error unmarshalling arguments", e); @@ -565,4 +565,85 @@ public class UnicastServerRef extends UnicastRef return map; } } + + /** + * Unmarshal parameters for the given method of the given instance over + * the given marshalinputstream. Perform any necessary checks. + */ + private Object[] unmarshalParameters(Object obj, Method method, MarshalInputStream in) + throws IOException, ClassNotFoundException { + return (obj instanceof DeserializationChecker) ? + unmarshalParametersChecked((DeserializationChecker)obj, method, in) : + unmarshalParametersUnchecked(method, in); + } + + /** + * Unmarshal parameters for the given method of the given instance over + * the given marshalinputstream. Do not perform any additional checks. + */ + private Object[] unmarshalParametersUnchecked(Method method, ObjectInput in) + throws IOException, ClassNotFoundException { + Class[] types = method.getParameterTypes(); + Object[] params = new Object[types.length]; + for (int i = 0; i < types.length; i++) { + params[i] = unmarshalValue(types[i], in); + } + return params; + } + + /** + * Unmarshal parameters for the given method of the given instance over + * the given marshalinputstream. Do perform all additional checks. + */ + private Object[] unmarshalParametersChecked( + DeserializationChecker checker, + Method method, MarshalInputStream in) + throws IOException, ClassNotFoundException { + int callID = methodCallIDCount.getAndIncrement(); + MyChecker myChecker = new MyChecker(checker, method, callID); + in.setStreamChecker(myChecker); + try { + Class[] types = method.getParameterTypes(); + Object[] values = new Object[types.length]; + for (int i = 0; i < types.length; i++) { + myChecker.setIndex(i); + values[i] = unmarshalValue(types[i], in); + } + myChecker.end(callID); + return values; + } finally { + in.setStreamChecker(null); + } + } + + private static class MyChecker implements MarshalInputStream.StreamChecker { + private final DeserializationChecker descriptorCheck; + private final Method method; + private final int callID; + private int parameterIndex; + + MyChecker(DeserializationChecker descriptorCheck, Method method, int callID) { + this.descriptorCheck = descriptorCheck; + this.method = method; + this.callID = callID; + } + + @Override + public void validateDescriptor(ObjectStreamClass descriptor) { + descriptorCheck.check(method, descriptor, parameterIndex, callID); + } + + @Override + public void checkProxyInterfaceNames(String[] ifaces) { + descriptorCheck.checkProxyClass(method, ifaces, parameterIndex, callID); + } + + void setIndex(int parameterIndex) { + this.parameterIndex = parameterIndex; + } + + void end(int callId) { + descriptorCheck.end(callId); + } + } } From 49b2db4ae7ff6499003679f87df594ba5947ffad Mon Sep 17 00:00:00 2001 From: Vladimir Ivanov Date: Fri, 18 Mar 2016 18:07:55 -0700 Subject: [PATCH 9/9] 8152335: Improve MethodHandle consistency Co-authored-by: Michael Haupt Reviewed-by: acorn, ahgross, jrose --- .../share/classes/java/lang/ClassLoader.java | 3 + .../classes/java/lang/invoke/MemberName.java | 29 ++++++-- .../java/lang/invoke/MethodHandleNatives.java | 2 +- .../classes/sun/invoke/util/VerifyAccess.java | 68 +++++++++++++++---- 4 files changed, 82 insertions(+), 20 deletions(-) diff --git a/jdk/src/java.base/share/classes/java/lang/ClassLoader.java b/jdk/src/java.base/share/classes/java/lang/ClassLoader.java index 1fcd03b760f..61123302a3c 100644 --- a/jdk/src/java.base/share/classes/java/lang/ClassLoader.java +++ b/jdk/src/java.base/share/classes/java/lang/ClassLoader.java @@ -817,6 +817,9 @@ public abstract class ClassLoader { if (!checkName(name)) throw new NoClassDefFoundError("IllegalName: " + name); + // Note: Checking logic in java.lang.invoke.MemberName.checkForTypeAlias + // relies on the fact that spoofing is impossible if a class has a name + // of the form "java.*" if ((name != null) && name.startsWith("java.") && this != getBuiltinPlatformClassLoader()) { throw new SecurityException diff --git a/jdk/src/java.base/share/classes/java/lang/invoke/MemberName.java b/jdk/src/java.base/share/classes/java/lang/invoke/MemberName.java index d141efcac6e..de4e3c15537 100644 --- a/jdk/src/java.base/share/classes/java/lang/invoke/MemberName.java +++ b/jdk/src/java.base/share/classes/java/lang/invoke/MemberName.java @@ -827,7 +827,7 @@ import java.util.Objects; assert(isResolved() == isResolved); } - void checkForTypeAlias() { + void checkForTypeAlias(Class refc) { if (isInvocable()) { MethodType type; if (this.type instanceof MethodType) @@ -835,16 +835,16 @@ import java.util.Objects; else this.type = type = getMethodType(); if (type.erase() == type) return; - if (VerifyAccess.isTypeVisible(type, clazz)) return; - throw new LinkageError("bad method type alias: "+type+" not visible from "+clazz); + if (VerifyAccess.isTypeVisible(type, refc)) return; + throw new LinkageError("bad method type alias: "+type+" not visible from "+refc); } else { Class type; if (this.type instanceof Class) type = (Class) this.type; else this.type = type = getFieldType(); - if (VerifyAccess.isTypeVisible(type, clazz)) return; - throw new LinkageError("bad field type alias: "+type+" not visible from "+clazz); + if (VerifyAccess.isTypeVisible(type, refc)) return; + throw new LinkageError("bad field type alias: "+type+" not visible from "+refc); } } @@ -1016,10 +1016,25 @@ import java.util.Objects; MemberName m = ref.clone(); // JVM will side-effect the ref assert(refKind == m.getReferenceKind()); try { + // There are 4 entities in play here: + // * LC: lookupClass + // * REFC: symbolic reference class (MN.clazz before resolution); + // * DEFC: resolved method holder (MN.clazz after resolution); + // * PTYPES: parameter types (MN.type) + // + // What we care about when resolving a MemberName is consistency between DEFC and PTYPES. + // We do type alias (TA) checks on DEFC to ensure that. DEFC is not known until the JVM + // finishes the resolution, so do TA checks right after MHN.resolve() is over. + // + // All parameters passed by a caller are checked against MH type (PTYPES) on every invocation, + // so it is safe to call a MH from any context. + // + // REFC view on PTYPES doesn't matter, since it is used only as a starting point for resolution and doesn't + // participate in method selection. m = MethodHandleNatives.resolve(m, lookupClass); - m.checkForTypeAlias(); + m.checkForTypeAlias(m.getDeclaringClass()); m.resolution = null; - } catch (LinkageError ex) { + } catch (ClassNotFoundException | LinkageError ex) { // JVM reports that the "bytecode behavior" would get an error assert(!m.isResolved()); m.resolution = ex; diff --git a/jdk/src/java.base/share/classes/java/lang/invoke/MethodHandleNatives.java b/jdk/src/java.base/share/classes/java/lang/invoke/MethodHandleNatives.java index 7a5d3f1f0b2..67c197dd709 100644 --- a/jdk/src/java.base/share/classes/java/lang/invoke/MethodHandleNatives.java +++ b/jdk/src/java.base/share/classes/java/lang/invoke/MethodHandleNatives.java @@ -49,7 +49,7 @@ class MethodHandleNatives { static native void init(MemberName self, Object ref); static native void expand(MemberName self); - static native MemberName resolve(MemberName self, Class caller) throws LinkageError; + static native MemberName resolve(MemberName self, Class caller) throws LinkageError, ClassNotFoundException; static native int getMembers(Class defc, String matchName, String matchSig, int matchFlags, Class caller, int skip, MemberName[] results); diff --git a/jdk/src/java.base/share/classes/sun/invoke/util/VerifyAccess.java b/jdk/src/java.base/share/classes/sun/invoke/util/VerifyAccess.java index 456909fde1c..37cbede912a 100644 --- a/jdk/src/java.base/share/classes/sun/invoke/util/VerifyAccess.java +++ b/jdk/src/java.base/share/classes/sun/invoke/util/VerifyAccess.java @@ -231,22 +231,66 @@ public class VerifyAccess { * @param refc the class attempting to make the reference */ public static boolean isTypeVisible(Class type, Class refc) { - if (type == refc) return true; // easy check + if (type == refc) { + return true; // easy check + } while (type.isArray()) type = type.getComponentType(); - if (type.isPrimitive() || type == Object.class) return true; - ClassLoader parent = type.getClassLoader(); - if (parent == null) return true; - ClassLoader child = refc.getClassLoader(); - if (child == null) return false; - if (parent == child || loadersAreRelated(parent, child, true)) + if (type.isPrimitive() || type == Object.class) { return true; - // Do it the hard way: Look up the type name from the refc loader. - try { - Class res = child.loadClass(type.getName()); - return (type == res); - } catch (ClassNotFoundException ex) { + } + ClassLoader typeLoader = type.getClassLoader(); + ClassLoader refcLoader = refc.getClassLoader(); + if (typeLoader == refcLoader) { + return true; + } + if (refcLoader == null && typeLoader != null) { return false; } + if (typeLoader == null && type.getName().startsWith("java.")) { + // Note: The API for actually loading classes, ClassLoader.defineClass, + // guarantees that classes with names beginning "java." cannot be aliased, + // because class loaders cannot load them directly. + return true; + } + + // Do it the hard way: Look up the type name from the refc loader. + // + // Force the refc loader to report and commit to a particular binding for this type name (type.getName()). + // + // In principle, this query might force the loader to load some unrelated class, + // which would cause this query to fail (and the original caller to give up). + // This would be wasted effort, but it is expected to be very rare, occurring + // only when an attacker is attempting to create a type alias. + // In the normal case, one class loader will simply delegate to the other, + // and the same type will be visible through both, with no extra loading. + // + // It is important to go through Class.forName instead of ClassLoader.loadClass + // because Class.forName goes through the JVM system dictionary, which records + // the class lookup once for all. This means that even if a not-well-behaved class loader + // would "change its mind" about the meaning of the name, the Class.forName request + // will use the result cached in the JVM system dictionary. Note that the JVM system dictionary + // will record the first successful result. Unsuccessful results are not stored. + // + // We use doPrivileged in order to allow an unprivileged caller to ask an arbitrary + // class loader about the binding of the proposed name (type.getName()). + // The looked up type ("res") is compared for equality against the proposed + // type ("type") and then is discarded. Thus, the worst that can happen to + // the "child" class loader is that it is bothered to load and report a class + // that differs from "type"; this happens once due to JVM system dictionary + // memoization. And the caller never gets to look at the alternate type binding + // ("res"), whether it exists or not. + final String name = type.getName(); + Class res = java.security.AccessController.doPrivileged( + new java.security.PrivilegedAction<>() { + public Class run() { + try { + return Class.forName(name, false, refcLoader); + } catch (ClassNotFoundException | LinkageError e) { + return null; // Assume the class is not found + } + } + }); + return (type == res); } /**