Lucene - Core
  1. Lucene - Core
  2. LUCENE-6542

FSDirectory throws AccessControlException unless you grant write access to the index

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 5.1
    • Fix Version/s: 5.3, 6.0
    • Component/s: core/store
    • Labels:
    • Lucene Fields:
      New

      Description

      Hit this during my attempted upgrade to Lucene 5.1.0. (Yeah, I know 5.2.0 is out, and we'll be using that in production anyway, but the merge takes time.)

      Various tests of ours test Directory stuff against methods which the security policy won't allow tests to write to. Changes in FSDirectory mean that it now demands write access to the directory. 4.10.4 permitted read-only access.

      1. LUCENE-6542.patch
        12 kB
        Uwe Schindler
      2. LUCENE-6542.patch
        12 kB
        Uwe Schindler
      3. LUCENE-6542.patch
        11 kB
        Uwe Schindler
      4. LUCENE-6542.patch
        7 kB
        Uwe Schindler
      5. LUCENE-6542.patch
        7 kB
        Uwe Schindler
      6. LUCENE-6542.patch
        8 kB
        Hoss Man
      7. patch.txt
        0.7 kB
        Trejkaz

        Issue Links

          Activity

          Hide
          Trejkaz added a comment -

          Patch guards the Files.createDirectories call with a Files.exists check. Files.exists only demands read access, so it succeeds if you only have read access.

          Show
          Trejkaz added a comment - Patch guards the Files.createDirectories call with a Files.exists check. Files.exists only demands read access, so it succeeds if you only have read access.
          Hide
          Hoss Man added a comment -

          Patch looks fine to me at first glance, but i'm not exactly sure why the createDirectories call was introduced at that point in the code path in the first place.

          Looking at the logs, it seems to have been added as part of LUCENE-5945 / r1624784 – prior to that we only checked for existence and throw an error if it didn't exist or wasn't a directory (not sure if/where it was created automaticaly if it didn't exist)


          Tangentially: do we even have a way to write a test for this sort of thing? can we mock the security manager so that a test could say "during my setUp method, i want to write a bunch of dirs/files, but during the test method, please trigger a failure if any lucene code asks the security manager for the write access to those files?"

          Show
          Hoss Man added a comment - Patch looks fine to me at first glance, but i'm not exactly sure why the createDirectories call was introduced at that point in the code path in the first place. Looking at the logs, it seems to have been added as part of LUCENE-5945 / r1624784 – prior to that we only checked for existence and throw an error if it didn't exist or wasn't a directory (not sure if/where it was created automaticaly if it didn't exist) Tangentially: do we even have a way to write a test for this sort of thing? can we mock the security manager so that a test could say "during my setUp method, i want to write a bunch of dirs/files, but during the test method, please trigger a failure if any lucene code asks the security manager for the write access to those files?"
          Hide
          Trejkaz added a comment -

          It works something like this:

          import org.junit.After;
          import org.junit.Before;
          import org.junit.Test;
          
          import java.io.FilePermission;
          import java.nio.file.Files;
          import java.nio.file.Path;
          import java.security.Permission;
          import java.security.Permissions;
          
          import static org.hamcrest.Matchers.is;
          import static org.junit.Assert.assertThat;
          import static org.junit.Assert.fail;
          
          public class TestSecurity {
              private Path file;
              private SecurityManager oldSecurityManager;
          
              @Test
              public void test() {
                  assertThat(Files.isReadable(file), is(true));
                  try {
                      Files.isWritable(file);
                      fail("Expected SecurityException");
                  } catch (SecurityException e) {
                      // Expected
                  }
              }
          
              @Before
              public void setUp() throws Exception {
                  file = Files.createTempFile("temp", ".dat");
                  assertThat(Files.isReadable(file), is(true));
                  assertThat(Files.isWritable(file), is(true));
          
                  oldSecurityManager = System.getSecurityManager();
                  System.setSecurityManager(new CustomSecurityManager());
              }
          
              @After
              public void tearDown() {
                  System.setSecurityManager(oldSecurityManager);
              }
          
              private static class CustomSecurityManager extends SecurityManager {
                  Permissions permitted = new Permissions();
          
                  private CustomSecurityManager() {
                      permitted.add(new RuntimePermission("setSecurityManager"));
                      permitted.add(new FilePermission("<<ALL FILES>>", "read"));
                  }
          
                  @Override
                  public void checkPermission(Permission permission) {
                      if (!permitted.implies(permission)) {
                          // let super's implementation throw the AccessControlException
                          super.checkPermission(permission);
                      }
                  }
              }
          }
          
          Show
          Trejkaz added a comment - It works something like this: import org.junit.After; import org.junit.Before; import org.junit.Test; import java.io.FilePermission; import java.nio.file.Files; import java.nio.file.Path; import java.security.Permission; import java.security.Permissions; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; public class TestSecurity { private Path file; private SecurityManager oldSecurityManager; @Test public void test() { assertThat(Files.isReadable(file), is( true )); try { Files.isWritable(file); fail( "Expected SecurityException" ); } catch (SecurityException e) { // Expected } } @Before public void setUp() throws Exception { file = Files.createTempFile( "temp" , ".dat" ); assertThat(Files.isReadable(file), is( true )); assertThat(Files.isWritable(file), is( true )); oldSecurityManager = System .getSecurityManager(); System .setSecurityManager( new CustomSecurityManager()); } @After public void tearDown() { System .setSecurityManager(oldSecurityManager); } private static class CustomSecurityManager extends SecurityManager { Permissions permitted = new Permissions(); private CustomSecurityManager() { permitted.add( new RuntimePermission( "setSecurityManager" )); permitted.add( new FilePermission( "<<ALL FILES>>" , "read" )); } @Override public void checkPermission(Permission permission) { if (!permitted.implies(permission)) { // let super 's implementation throw the AccessControlException super .checkPermission(permission); } } } }
          Hide
          Hoss Man added a comment -

          Trejkaz,

          I couldn't really make sense of your "TestSecurity" example – nothing in it seems to be enforcing the "deny write access" part of the issue, it's just allowing more things (notably: read access for all files) then the default policy ... at first I thought maybe you were assuming the default policy was automatically going to deny writes, and had just meant "write" instead of "read" in your FilePermission to start allowing it during the test – but that didn't match up with the assertions you had, and the exceptions you were expecting from Files.isWritable.

          In any case – the key to my question wasn't just "how can any junit test change the SecurityManager" it was specifically about lucene tests, and whether we already had any helper code for this in the lucene test-framework to aid in this.

          I couldn't find any, so i tried to create a test that used a custom SecurityManager after building an index in an FSDirectory. My hope was that i could get it working, and then refactor the SecurityManager into LuceneTestCase with helper methods to specify Permission objects at runtime that should be used to "deny" Permission checks before defaulting to the system configured access policy.

          Unfortunately, the results have been confusing.

          Some seeds pass as expected – but also (confusingly) pass even if i revert your suggested changes to FSDirectory....

          ant test  -Dtestcase=TestReadOnlyIndex  -Dtests.seed=32CF43854EF86591 -Dtests.slow=true -Dtests.locale=tr_TR -Dtests.timezone=NST -Dtests.asserts=true -Dtests.file.encoding=UTF-8
          

          Other seeds fail consistently with strange access errors relating to reading JVM resource bundles – even though nothing about the custom security manager should be affecting these permissions...

          ant test  -Dtestcase=TestReadOnlyIndex -Dtests.seed=1DF78A66DF6175D3 -Dtests.slow=true -Dtests.locale=es_US -Dtests.timezone=Africa/Johannesburg -Dtests.asserts=true -Dtests.file.encoding=UTF-8
          

          ...

             [junit4] Started J0 PID(8937@localhost).
             [junit4] Suite: org.apache.lucene.index.TestReadOnlyIndex
             [junit4]   2> NOTE: reproduce with: ant test  -Dtestcase=TestReadOnlyIndex -Dtests.method=testReadOnlyIndex -Dtests.seed=1DF78A66DF6175D3 -Dtests.slow=true -Dtests.locale=es_US -Dtests.timezone=Africa/Johannesburg -Dtests.asserts=true -Dtests.file.encoding=UTF-8
             [junit4] ERROR   0.16s | TestReadOnlyIndex.testReadOnlyIndex <<<
             [junit4]    > Throwable #1: java.lang.BootstrapMethodError: call site initialization exception
             [junit4]    > 	at __randomizedtesting.SeedInfo.seed([1DF78A66DF6175D3:A472513897A23BC1]:0)
             [junit4]    > 	at java.lang.invoke.CallSite.makeSite(CallSite.java:341)
             [junit4]    > 	at java.lang.invoke.MethodHandleNatives.linkCallSiteImpl(MethodHandleNatives.java:307)
             [junit4]    > 	at java.lang.invoke.MethodHandleNatives.linkCallSite(MethodHandleNatives.java:297)
             [junit4]    > 	at org.apache.lucene.store.MMapDirectory.<clinit>(MMapDirectory.java:165)
             [junit4]    > 	at org.apache.lucene.store.FSDirectory.open(FSDirectory.java:163)
             [junit4]    > 	at org.apache.lucene.store.FSDirectory.open(FSDirectory.java:157)
             [junit4]    > 	at org.apache.lucene.index.TestReadOnlyIndex.testReadOnlyIndex(TestReadOnlyIndex.java:82)
             [junit4]    > 	at java.lang.Thread.run(Thread.java:745)
             [junit4]    > Caused by: java.util.MissingResourceException: Can't find bundle for base name sun.util.resources.CurrencyNames, locale es_US
             [junit4]    > 	at java.util.ResourceBundle.throwMissingResourceException(ResourceBundle.java:1564)
             [junit4]    > 	at java.util.ResourceBundle.getBundleImpl(ResourceBundle.java:1387)
             [junit4]    > 	at java.util.ResourceBundle.getBundle(ResourceBundle.java:890)
             [junit4]    > 	at sun.util.resources.LocaleData$1.run(LocaleData.java:164)
             [junit4]    > 	at sun.util.resources.LocaleData$1.run(LocaleData.java:160)
             [junit4]    > 	at java.security.AccessController.doPrivileged(Native Method)
             [junit4]    > 	at sun.util.resources.LocaleData.getBundle(LocaleData.java:160)
             [junit4]    > 	at sun.util.resources.LocaleData.getCurrencyNames(LocaleData.java:84)
             [junit4]    > 	at sun.util.locale.provider.LocaleResources.getCurrencyName(LocaleResources.java:216)
             [junit4]    > 	at sun.util.locale.provider.CurrencyNameProviderImpl.getString(CurrencyNameProviderImpl.java:122)
             [junit4]    > 	at sun.util.locale.provider.CurrencyNameProviderImpl.getSymbol(CurrencyNameProviderImpl.java:90)
             [junit4]    > 	at java.util.Currency$CurrencyNameGetter.getObject(Currency.java:640)
             [junit4]    > 	at java.util.Currency$CurrencyNameGetter.getObject(Currency.java:625)
             [junit4]    > 	at sun.util.locale.provider.LocaleServiceProviderPool.getLocalizedObjectImpl(LocaleServiceProviderPool.java:281)
             [junit4]    > 	at sun.util.locale.provider.LocaleServiceProviderPool.getLocalizedObject(LocaleServiceProviderPool.java:265)
             [junit4]    > 	at java.util.Currency.getSymbol(Currency.java:505)
             [junit4]    > 	at java.text.DecimalFormatSymbols.initialize(DecimalFormatSymbols.java:648)
             [junit4]    > 	at java.text.DecimalFormatSymbols.<init>(DecimalFormatSymbols.java:113)
             [junit4]    > 	at sun.util.locale.provider.DecimalFormatSymbolsProviderImpl.getInstance(DecimalFormatSymbolsProviderImpl.java:85)
             [junit4]    > 	at java.text.DecimalFormatSymbols.getInstance(DecimalFormatSymbols.java:180)
             [junit4]    > 	at java.util.Formatter.getZero(Formatter.java:2283)
             [junit4]    > 	at java.util.Formatter.<init>(Formatter.java:1892)
             [junit4]    > 	at java.util.Formatter.<init>(Formatter.java:1914)
             [junit4]    > 	at java.lang.String.format(String.java:2928)
             [junit4]    > 	at java.lang.invoke.TypeConvertingMethodAdapter.boxingDescriptor(TypeConvertingMethodAdapter.java:134)
             [junit4]    > 	at java.lang.invoke.TypeConvertingMethodAdapter.box(TypeConvertingMethodAdapter.java:161)
             [junit4]    > 	at java.lang.invoke.TypeConvertingMethodAdapter.convertType(TypeConvertingMethodAdapter.java:236)
             [junit4]    > 	at java.lang.invoke.InnerClassLambdaMetafactory$ForwardingMethodGenerator.generate(InnerClassLambdaMetafactory.java:476)
             [junit4]    > 	at java.lang.invoke.InnerClassLambdaMetafactory.spinInnerClass(InnerClassLambdaMetafactory.java:288)
             [junit4]    > 	at java.lang.invoke.InnerClassLambdaMetafactory.buildCallSite(InnerClassLambdaMetafactory.java:194)
             [junit4]    > 	at java.lang.invoke.LambdaMetafactory.metafactory(LambdaMetafactory.java:304)
             [junit4]    > 	at java.lang.invoke.CallSite.makeSite(CallSite.java:302)
             [junit4]    > 	... 42 more
             [junit4]    > Caused by: java.security.AccessControlException: access denied ("java.lang.RuntimePermission" "accessClassInPackage.sun.util.resources")
             [junit4]    > 	at java.security.AccessControlContext.checkPermission(AccessControlContext.java:457)
             [junit4]    > 	at java.security.AccessController.checkPermission(AccessController.java:884)
             [junit4]    > 	at java.lang.SecurityManager.checkPermission(SecurityManager.java:549)
             [junit4]    > 	at org.apache.lucene.index.TestReadOnlyIndex$RestrictiveSecurityManager.checkPermission(TestReadOnlyIndex.java:153)
             [junit4]    > 	at java.lang.SecurityManager.checkPackageAccess(SecurityManager.java:1564)
             [junit4]    > 	at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:311)
             [junit4]    > 	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
             [junit4]    > 	at java.util.ResourceBundle$RBClassLoader.loadClass(ResourceBundle.java:503)
             [junit4]    > 	at java.util.ResourceBundle$Control.newBundle(ResourceBundle.java:2640)
             [junit4]    > 	at java.util.ResourceBundle.loadBundle(ResourceBundle.java:1501)
             [junit4]    > 	at java.util.ResourceBundle.findBundle(ResourceBundle.java:1465)
             [junit4]    > 	at java.util.ResourceBundle.findBundle(ResourceBundle.java:1419)
             [junit4]    > 	at java.util.ResourceBundle.findBundle(ResourceBundle.java:1419)
             [junit4]    > 	at java.util.ResourceBundle.getBundleImpl(ResourceBundle.java:1361)
             [junit4]    > 	... 72 more
             [junit4] OK      0.00s | TestReadOnlyIndex.testSanityCheckSecurityManager
             [junit4]   2> NOTE: test params are: codec=Asserting(Lucene53): {fieldname=Lucene50(blocksize=128)}, docValues:{}, sim=RandomSimilarityProvider(queryNorm=true,coord=no): {fieldname=DFR GL2}, locale=es_US, timezone=Africa/Johannesburg
             [junit4]   2> NOTE: Linux 3.19.0-18-generic amd64/Oracle Corporation 1.8.0_45 (64-bit)/cpus=4,threads=1,free=207954744,total=249561088
             [junit4]   2> NOTE: All tests run in this JVM: [TestReadOnlyIndex]
             [junit4] Completed [1/1] in 0.78s, 2 tests, 1 error <<< FAILURES!
             [junit4] 
             [junit4] 
             [junit4] Tests with failures:
             [junit4]   - org.apache.lucene.index.TestReadOnlyIndex.testReadOnlyIndex
             [junit4] 
             [junit4] 
             [junit4] JVM J0:     0.73 ..     1.74 =     1.01s
             [junit4] Execution time total: 1.77 sec.
             [junit4] Tests summary: 1 suite, 2 tests, 1 error
          

          ...from what i can tell, these AccessControlException will happen just by having a Security Manager subclass that overrides checKPermission, even if that override does nothing but delegate to super...

          @Override
          public void checkPermission(Permission permission) {
            // first see if the top level policy allows/prevents
            super.checkPermission(permission);
            // now explicitly deny things we're configured to deny
            // if (deny.implies(permission)) {
            //   throw new SecurityException("explicitly denied by test");
            // }
          }
          

          ...so i'm really not sure what's going on here.

          Show
          Hoss Man added a comment - Trejkaz, I couldn't really make sense of your "TestSecurity" example – nothing in it seems to be enforcing the "deny write access" part of the issue, it's just allowing more things (notably: read access for all files) then the default policy ... at first I thought maybe you were assuming the default policy was automatically going to deny writes, and had just meant "write" instead of "read" in your FilePermission to start allowing it during the test – but that didn't match up with the assertions you had, and the exceptions you were expecting from Files.isWritable. In any case – the key to my question wasn't just "how can any junit test change the SecurityManager" it was specifically about lucene tests, and whether we already had any helper code for this in the lucene test-framework to aid in this. I couldn't find any, so i tried to create a test that used a custom SecurityManager after building an index in an FSDirectory. My hope was that i could get it working, and then refactor the SecurityManager into LuceneTestCase with helper methods to specify Permission objects at runtime that should be used to "deny" Permission checks before defaulting to the system configured access policy. Unfortunately, the results have been confusing. Some seeds pass as expected – but also (confusingly) pass even if i revert your suggested changes to FSDirectory.... ant test -Dtestcase=TestReadOnlyIndex -Dtests.seed=32CF43854EF86591 -Dtests.slow=true -Dtests.locale=tr_TR -Dtests.timezone=NST -Dtests.asserts=true -Dtests.file.encoding=UTF-8 Other seeds fail consistently with strange access errors relating to reading JVM resource bundles – even though nothing about the custom security manager should be affecting these permissions... ant test -Dtestcase=TestReadOnlyIndex -Dtests.seed=1DF78A66DF6175D3 -Dtests.slow=true -Dtests.locale=es_US -Dtests.timezone=Africa/Johannesburg -Dtests.asserts=true -Dtests.file.encoding=UTF-8 ... [junit4] Started J0 PID(8937@localhost). [junit4] Suite: org.apache.lucene.index.TestReadOnlyIndex [junit4] 2> NOTE: reproduce with: ant test -Dtestcase=TestReadOnlyIndex -Dtests.method=testReadOnlyIndex -Dtests.seed=1DF78A66DF6175D3 -Dtests.slow=true -Dtests.locale=es_US -Dtests.timezone=Africa/Johannesburg -Dtests.asserts=true -Dtests.file.encoding=UTF-8 [junit4] ERROR 0.16s | TestReadOnlyIndex.testReadOnlyIndex <<< [junit4] > Throwable #1: java.lang.BootstrapMethodError: call site initialization exception [junit4] > at __randomizedtesting.SeedInfo.seed([1DF78A66DF6175D3:A472513897A23BC1]:0) [junit4] > at java.lang.invoke.CallSite.makeSite(CallSite.java:341) [junit4] > at java.lang.invoke.MethodHandleNatives.linkCallSiteImpl(MethodHandleNatives.java:307) [junit4] > at java.lang.invoke.MethodHandleNatives.linkCallSite(MethodHandleNatives.java:297) [junit4] > at org.apache.lucene.store.MMapDirectory.<clinit>(MMapDirectory.java:165) [junit4] > at org.apache.lucene.store.FSDirectory.open(FSDirectory.java:163) [junit4] > at org.apache.lucene.store.FSDirectory.open(FSDirectory.java:157) [junit4] > at org.apache.lucene.index.TestReadOnlyIndex.testReadOnlyIndex(TestReadOnlyIndex.java:82) [junit4] > at java.lang.Thread.run(Thread.java:745) [junit4] > Caused by: java.util.MissingResourceException: Can't find bundle for base name sun.util.resources.CurrencyNames, locale es_US [junit4] > at java.util.ResourceBundle.throwMissingResourceException(ResourceBundle.java:1564) [junit4] > at java.util.ResourceBundle.getBundleImpl(ResourceBundle.java:1387) [junit4] > at java.util.ResourceBundle.getBundle(ResourceBundle.java:890) [junit4] > at sun.util.resources.LocaleData$1.run(LocaleData.java:164) [junit4] > at sun.util.resources.LocaleData$1.run(LocaleData.java:160) [junit4] > at java.security.AccessController.doPrivileged(Native Method) [junit4] > at sun.util.resources.LocaleData.getBundle(LocaleData.java:160) [junit4] > at sun.util.resources.LocaleData.getCurrencyNames(LocaleData.java:84) [junit4] > at sun.util.locale.provider.LocaleResources.getCurrencyName(LocaleResources.java:216) [junit4] > at sun.util.locale.provider.CurrencyNameProviderImpl.getString(CurrencyNameProviderImpl.java:122) [junit4] > at sun.util.locale.provider.CurrencyNameProviderImpl.getSymbol(CurrencyNameProviderImpl.java:90) [junit4] > at java.util.Currency$CurrencyNameGetter.getObject(Currency.java:640) [junit4] > at java.util.Currency$CurrencyNameGetter.getObject(Currency.java:625) [junit4] > at sun.util.locale.provider.LocaleServiceProviderPool.getLocalizedObjectImpl(LocaleServiceProviderPool.java:281) [junit4] > at sun.util.locale.provider.LocaleServiceProviderPool.getLocalizedObject(LocaleServiceProviderPool.java:265) [junit4] > at java.util.Currency.getSymbol(Currency.java:505) [junit4] > at java.text.DecimalFormatSymbols.initialize(DecimalFormatSymbols.java:648) [junit4] > at java.text.DecimalFormatSymbols.<init>(DecimalFormatSymbols.java:113) [junit4] > at sun.util.locale.provider.DecimalFormatSymbolsProviderImpl.getInstance(DecimalFormatSymbolsProviderImpl.java:85) [junit4] > at java.text.DecimalFormatSymbols.getInstance(DecimalFormatSymbols.java:180) [junit4] > at java.util.Formatter.getZero(Formatter.java:2283) [junit4] > at java.util.Formatter.<init>(Formatter.java:1892) [junit4] > at java.util.Formatter.<init>(Formatter.java:1914) [junit4] > at java.lang.String.format(String.java:2928) [junit4] > at java.lang.invoke.TypeConvertingMethodAdapter.boxingDescriptor(TypeConvertingMethodAdapter.java:134) [junit4] > at java.lang.invoke.TypeConvertingMethodAdapter.box(TypeConvertingMethodAdapter.java:161) [junit4] > at java.lang.invoke.TypeConvertingMethodAdapter.convertType(TypeConvertingMethodAdapter.java:236) [junit4] > at java.lang.invoke.InnerClassLambdaMetafactory$ForwardingMethodGenerator.generate(InnerClassLambdaMetafactory.java:476) [junit4] > at java.lang.invoke.InnerClassLambdaMetafactory.spinInnerClass(InnerClassLambdaMetafactory.java:288) [junit4] > at java.lang.invoke.InnerClassLambdaMetafactory.buildCallSite(InnerClassLambdaMetafactory.java:194) [junit4] > at java.lang.invoke.LambdaMetafactory.metafactory(LambdaMetafactory.java:304) [junit4] > at java.lang.invoke.CallSite.makeSite(CallSite.java:302) [junit4] > ... 42 more [junit4] > Caused by: java.security.AccessControlException: access denied ("java.lang.RuntimePermission" "accessClassInPackage.sun.util.resources") [junit4] > at java.security.AccessControlContext.checkPermission(AccessControlContext.java:457) [junit4] > at java.security.AccessController.checkPermission(AccessController.java:884) [junit4] > at java.lang.SecurityManager.checkPermission(SecurityManager.java:549) [junit4] > at org.apache.lucene.index.TestReadOnlyIndex$RestrictiveSecurityManager.checkPermission(TestReadOnlyIndex.java:153) [junit4] > at java.lang.SecurityManager.checkPackageAccess(SecurityManager.java:1564) [junit4] > at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:311) [junit4] > at java.lang.ClassLoader.loadClass(ClassLoader.java:357) [junit4] > at java.util.ResourceBundle$RBClassLoader.loadClass(ResourceBundle.java:503) [junit4] > at java.util.ResourceBundle$Control.newBundle(ResourceBundle.java:2640) [junit4] > at java.util.ResourceBundle.loadBundle(ResourceBundle.java:1501) [junit4] > at java.util.ResourceBundle.findBundle(ResourceBundle.java:1465) [junit4] > at java.util.ResourceBundle.findBundle(ResourceBundle.java:1419) [junit4] > at java.util.ResourceBundle.findBundle(ResourceBundle.java:1419) [junit4] > at java.util.ResourceBundle.getBundleImpl(ResourceBundle.java:1361) [junit4] > ... 72 more [junit4] OK 0.00s | TestReadOnlyIndex.testSanityCheckSecurityManager [junit4] 2> NOTE: test params are: codec=Asserting(Lucene53): {fieldname=Lucene50(blocksize=128)}, docValues:{}, sim=RandomSimilarityProvider(queryNorm=true,coord=no): {fieldname=DFR GL2}, locale=es_US, timezone=Africa/Johannesburg [junit4] 2> NOTE: Linux 3.19.0-18-generic amd64/Oracle Corporation 1.8.0_45 (64-bit)/cpus=4,threads=1,free=207954744,total=249561088 [junit4] 2> NOTE: All tests run in this JVM: [TestReadOnlyIndex] [junit4] Completed [1/1] in 0.78s, 2 tests, 1 error <<< FAILURES! [junit4] [junit4] [junit4] Tests with failures: [junit4] - org.apache.lucene.index.TestReadOnlyIndex.testReadOnlyIndex [junit4] [junit4] [junit4] JVM J0: 0.73 .. 1.74 = 1.01s [junit4] Execution time total: 1.77 sec. [junit4] Tests summary: 1 suite, 2 tests, 1 error ...from what i can tell, these AccessControlException will happen just by having a Security Manager subclass that overrides checKPermission, even if that override does nothing but delegate to super... @Override public void checkPermission(Permission permission) { // first see if the top level policy allows/prevents super .checkPermission(permission); // now explicitly deny things we're configured to deny // if (deny.implies(permission)) { // throw new SecurityException( "explicitly denied by test" ); // } } ...so i'm really not sure what's going on here.
          Hide
          Uwe Schindler added a comment -

          Hi,
          this was officially documented on the issue converting to NIO.2, but I forget it to add to MIGRATE.txt!

          For correct Java 7 implementation and lock factory correctly working after the change (needs fully resolved absolute path), we have to ensure the directory exists before the FSDirectory ctor returns, other wise race conditions occur! The makeDirectories() enforces this.

          Please don't warp this with Files.exists() because this is not really atomic and not a good check, because in fact we need write access to the directory at a later stage, too - because we call fsync on the directory itsself to ensure commits are written into the directory file itsself.

          Please don't add any crazy semantics to TestSecurityManager: The reason why it does not work for you with this hack is because of the internal backwards compatibility in SecurityManager to handle pre-Java 1.2 (or like that), where SecurityManager did not know about permissions. At that time it used to call checkWrite(), which now delegates in strange ways - they would need VirtualMethod class from Lucene to handle this correctly

          I don't see this as a bug: Lucene "owns" the directory, so it must be able to create/delete/fsync/whatever it. If the directory is on a read-only FS and you just open DirectoryReader, its perfectly fine if the directory exists (no OS error). It is just FilePermission that complains.

          Show
          Uwe Schindler added a comment - Hi, this was officially documented on the issue converting to NIO.2, but I forget it to add to MIGRATE.txt! For correct Java 7 implementation and lock factory correctly working after the change (needs fully resolved absolute path), we have to ensure the directory exists before the FSDirectory ctor returns, other wise race conditions occur! The makeDirectories() enforces this. Please don't warp this with Files.exists() because this is not really atomic and not a good check, because in fact we need write access to the directory at a later stage, too - because we call fsync on the directory itsself to ensure commits are written into the directory file itsself. Please don't add any crazy semantics to TestSecurityManager: The reason why it does not work for you with this hack is because of the internal backwards compatibility in SecurityManager to handle pre-Java 1.2 (or like that), where SecurityManager did not know about permissions. At that time it used to call checkWrite(), which now delegates in strange ways - they would need VirtualMethod class from Lucene to handle this correctly I don't see this as a bug: Lucene "owns" the directory, so it must be able to create/delete/fsync/whatever it. If the directory is on a read-only FS and you just open DirectoryReader, its perfectly fine if the directory exists (no OS error). It is just FilePermission that complains.
          Hide
          Uwe Schindler added a comment -

          And your second patch: Please don't do this. This is all not worth a test. It just compromises our security manager. A SecurityManager that allows to override/delete itsself just makes itsself broken.

          Show
          Uwe Schindler added a comment - And your second patch: Please don't do this. This is all not worth a test. It just compromises our security manager. A SecurityManager that allows to override/delete itsself just makes itsself broken.
          Hide
          Robert Muir added a comment -

          Yes, if you really want to do this in a test:

          Guideline 9-4 / ACCESS-4: Know how to restrict privileges through doPrivileged
          http://www.oracle.com/technetwork/java/seccodeguide-139067.html#9

          Show
          Robert Muir added a comment - Yes, if you really want to do this in a test: Guideline 9-4 / ACCESS-4: Know how to restrict privileges through doPrivileged http://www.oracle.com/technetwork/java/seccodeguide-139067.html#9
          Hide
          Uwe Schindler added a comment -

          Good tip!

          Show
          Uwe Schindler added a comment - Good tip!
          Hide
          Hoss Man added a comment - - edited

          Uwe: I'm still not following why my attempts at Runtime control over the allowed FilePermissions don't work, but that may all be moot so let's ignore it for now...

          EDIT: ah ... just saw rmuir's reply ... still wrapping my head arround 9-1 and 9-4, but i think i'm getting the gist -not going to worry about it until/unless we decide any of the rest of the objectives below make sense...

          I don't see this as a bug: Lucene "owns" the directory, so it must be able to create/delete/fsync/whatever it. If the directory is on a read-only FS and you just open DirectoryReader, its perfectly fine if the directory exists (no OS error). It is just FilePermission that complains.

          ...my main concern is this: For all the same reasons rmuir mentioned / put the effort into LUCENE-6238, we should try to be as restrictive as possible in what Permissions the various Lucene classes require to run. Testing wit ha "no writes allowed" security policy like Trejkaz describes seems like a totally legitimate thing for a user to want to do. If that's no supported – then how can/should users like Trejkaz write tests to verify that their usage of lucene will not attempt to write to a (logically) read only index w/o using restrictive FilePermissions in their tests?

          You say a read only FileSystem will work, and i believe you – but how can we prove that in a lucene test so that we don't inadvertently add some silly code to IndexReader that writes a file to the index Directory? Just depending on something like Files.setPosixFilePermissions in the test doesn't really seem like enough – since the silly code might force the file to be writable first.

          So short of a test that requires you have a prebuilt index on a filesystem that's mounted read only, how do we (or our users) write/run a test to be sure that the "read only" code in lucene is truely "read only" ?

          Show
          Hoss Man added a comment - - edited Uwe: I'm still not following why my attempts at Runtime control over the allowed FilePermissions don't work, but that may all be moot so let's ignore it for now... EDIT: ah ... just saw rmuir's reply ... still wrapping my head arround 9-1 and 9-4, but i think i'm getting the gist -not going to worry about it until/unless we decide any of the rest of the objectives below make sense... I don't see this as a bug: Lucene "owns" the directory, so it must be able to create/delete/fsync/whatever it. If the directory is on a read-only FS and you just open DirectoryReader, its perfectly fine if the directory exists (no OS error). It is just FilePermission that complains. ...my main concern is this: For all the same reasons rmuir mentioned / put the effort into LUCENE-6238 , we should try to be as restrictive as possible in what Permissions the various Lucene classes require to run. Testing wit ha "no writes allowed" security policy like Trejkaz describes seems like a totally legitimate thing for a user to want to do. If that's no supported – then how can/should users like Trejkaz write tests to verify that their usage of lucene will not attempt to write to a (logically) read only index w/o using restrictive FilePermissions in their tests? You say a read only FileSystem will work, and i believe you – but how can we prove that in a lucene test so that we don't inadvertently add some silly code to IndexReader that writes a file to the index Directory? Just depending on something like Files.setPosixFilePermissions in the test doesn't really seem like enough – since the silly code might force the file to be writable first. So short of a test that requires you have a prebuilt index on a filesystem that's mounted read only, how do we (or our users) write/run a test to be sure that the "read only" code in lucene is truely "read only" ?
          Hide
          Uwe Schindler added a comment -

          I have the test working here. Will post updated patch.

          Show
          Uwe Schindler added a comment - I have the test working here. Will post updated patch.
          Hide
          Robert Muir added a comment -

          maybe add a helper method, so tests can do something like:

          assertNeedsPrivs(new WithPrivs(new FilePermission("/foo/bar", "read") {
            void run() {
              new MMapDirectory();
            }
          });
          

          it could take Permission varargs... since in most cases it should only be 1 or 2 for unit testing.

          Show
          Robert Muir added a comment - maybe add a helper method, so tests can do something like: assertNeedsPrivs( new WithPrivs( new FilePermission( "/foo/bar" , "read" ) { void run() { new MMapDirectory(); } }); it could take Permission varargs... since in most cases it should only be 1 or 2 for unit testing.
          Hide
          Uwe Schindler added a comment -

          Here is the test. I had to f*ck with the last Permission, because to run the code with restricted permissions you need the additional permission, otherwise it runs with no permissions at all (see javadocs)

          I also added a helper method, we may move this to LTC later

          Show
          Uwe Schindler added a comment - Here is the test. I had to f*ck with the last Permission, because to run the code with restricted permissions you need the additional permission, otherwise it runs with no permissions at all (see javadocs) I also added a helper method, we may move this to LTC later
          Hide
          Uwe Schindler added a comment -

          Slightly improved generics in the patch to make the helper method more universal (to move to LTC).

          Show
          Uwe Schindler added a comment - Slightly improved generics in the patch to make the helper method more universal (to move to LTC).
          Hide
          Hoss Man added a comment -

          I also added a helper method, we may move this to LTC later

          +1

          This patch looks great Uwe, generalizing that method into LTC sounds good to me.

          Show
          Hoss Man added a comment - I also added a helper method, we may move this to LTC later +1 This patch looks great Uwe, generalizing that method into LTC sounds good to me.
          Hide
          Uwe Schindler added a comment -

          More neat helper method (Java 8). I will blow this up to ten times its size when backporting to 5.x

          Show
          Uwe Schindler added a comment - More neat helper method (Java 8). I will blow this up to ten times its size when backporting to 5.x
          Hide
          Uwe Schindler added a comment -

          This patch looks great Uwe, generalizing that method into LTC sounds good to me.

          Will look into this tomorrow. Have to sleep now, its already 4 am. The time when Mike McCandless starts coding

          Show
          Uwe Schindler added a comment - This patch looks great Uwe, generalizing that method into LTC sounds good to me. Will look into this tomorrow. Have to sleep now, its already 4 am. The time when Mike McCandless starts coding
          Hide
          Uwe Schindler added a comment -

          Moved the runWithLowerPermissions to LTC.

          I also fixed the FSDirectory() ctor to do a mich simpler Files.isDirectory() check, because otherwise the basic TestDirectory test fails on Windows (of couse it fails...).

          Could somebody with Linux or MacOSX test the patch or state any other complaints with it?

          Show
          Uwe Schindler added a comment - Moved the runWithLowerPermissions to LTC. I also fixed the FSDirectory() ctor to do a mich simpler Files.isDirectory() check, because otherwise the basic TestDirectory test fails on Windows (of couse it fails...). Could somebody with Linux or MacOSX test the patch or state any other complaints with it?
          Hide
          Uwe Schindler added a comment -

          More tests that we cannot escape our sandbox!

          Show
          Uwe Schindler added a comment - More tests that we cannot escape our sandbox!
          Hide
          Uwe Schindler added a comment -

          After discussion with Hoss Man, I added an assume to the runWithRestrictedPermissions, so it cancels test execution if no security manager is available. Running those tests without a security manager makes no sense, because they would assert nothing (because they have all permissions).

          Show
          Uwe Schindler added a comment - After discussion with Hoss Man , I added an assume to the runWithRestrictedPermissions, so it cancels test execution if no security manager is available. Running those tests without a security manager makes no sense, because they would assert nothing (because they have all permissions).
          Hide
          ASF subversion and git services added a comment -

          Commit 1688537 from Uwe Schindler in branch 'dev/trunk'
          [ https://svn.apache.org/r1688537 ]

          LUCENE-6542: FSDirectory's ctor now works with security policies or file systems that restrict write access

          Show
          ASF subversion and git services added a comment - Commit 1688537 from Uwe Schindler in branch 'dev/trunk' [ https://svn.apache.org/r1688537 ] LUCENE-6542 : FSDirectory's ctor now works with security policies or file systems that restrict write access
          Hide
          ASF subversion and git services added a comment -

          Commit 1688541 from Uwe Schindler in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1688541 ]

          Merged revision(s) 1688537 from lucene/dev/trunk:
          LUCENE-6542: FSDirectory's ctor now works with security policies or file systems that restrict write access

          Show
          ASF subversion and git services added a comment - Commit 1688541 from Uwe Schindler in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1688541 ] Merged revision(s) 1688537 from lucene/dev/trunk: LUCENE-6542 : FSDirectory's ctor now works with security policies or file systems that restrict write access
          Hide
          Uwe Schindler added a comment -

          Committed and backported the lambdas to Java 7 in branch_5x.

          Show
          Uwe Schindler added a comment - Committed and backported the lambdas to Java 7 in branch_5x.
          Hide
          Shalin Shekhar Mangar added a comment -

          Bulk close for 5.3.0 release

          Show
          Shalin Shekhar Mangar added a comment - Bulk close for 5.3.0 release

            People

            • Assignee:
              Uwe Schindler
              Reporter:
              Trejkaz
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development