Accumulo
  1. Accumulo
  2. ACCUMULO-2936 Running Accumulo on different JVMs
  3. ACCUMULO-2943

Usage of SecureRandom with "SUN" as RNG provider causes lots of failures

    Details

    • Type: Sub-task Sub-task
    • Status: In Progress
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.6.0
    • Fix Version/s: 1.7.0, 1.6.3
    • Component/s: build
    • Labels:
      None
    • Environment:

      IBM JVM

      Description

      Both org.apache.accumulo.core.security.crypto.CrypoTest & org.apache.accumulo.core.file.rfile.RFileTest have lots of failures due to calls to SEcureRandom with Random Number Generator Provider hard-coded as Sun. The IBM JVM has it's own built in RNG Provider called IBMJCE. 2 issues - hard-coded calls to SecureRandom.getInstance(<algo>,"SUN") and also default value in Property class is "SUN".

      Options for proposals:

      1. Add mechanism to override default Property through System property through new annotator in Property class. Only usage will be by Property.CRYPTO_SECURE_RNG_PROVIDER

      The default is SUN, as coded in the Property class.

      Some of the unit tests explicitly call SecureRandom.getInstance() with SUN as provider. Others rely on the default value, which is SUN (and do not rely on accumulo-site.xml in which this can be overridden). For the system tests, I propose adding an override through a System property. For regular execution of accumulo, accumulo-site.xml can include the adjusted default. However, system tests will also fall on same problem.

      Another solution would be to have a factory class which supplies JVM specific behaviour, such as default NRG provider - this could be used by the Property class to retrieve this value based on the System.property for java.vendor etc.....

        Activity

        Hide
        ASF GitHub Bot added a comment -

        GitHub user haydenmarchant opened a pull request:

        https://github.com/apache/accumulo/pull/11

        ACCUMULO-2943 Fixing failures where no RNG "SUN" provider

        Both org.apache.accumulo.core.security.crypto.CrypoTest &
        org.apache.accumulo.core.file.rfile.RFileTest have lots of failures
        due to calls to SecureRandom with Random Number Generator Provider
        hard-coded as Sun. The IBM JVM has it's own built in RNG Provider
        called IBMJCE. 2 issues - hard-coded calls to
        SecureRandom.getInstance(<algo>,"SUN") and also default value in
        Property class is "SUN".

        Most failures are due to the CryptoModuleParameters instance being
        populated with default value of Crypto Secure RNG Provider, in
        particular, the following line from CryptoModelFactory.fillParamsObjectFromStringMap():

        params.setRandomNumberGeneratorProvider(cryptoOpts.get(Property.CRYPTO_SECURE_RNG_PROVIDER.getKey()));

        Since the default as described in Property class for RNG provider
        is "SUN", I have made an override mechanism in which a default
        property can be overidden by passing System property of same name.
        Any property with annotation @SystemOverride has this functionality
        enabled. So, when using a JVM which does not have the "SUN" RNG
        Provider, a system property (-Dcrypto.secure.rng.provider=

        {provname})
        can be added to the parent pom.xml in the surefire plugin definition
        (same location as the max memory for tests).

        In addition, CryptoTest.testCryptoModuleParamsParsing() has been
        changed to read from a separate config file since it just focuses on
        parsing of params and not the actual instantiation of providers etc...

        You can merge this pull request into a Git repository by running:

        $ git pull https://github.com/haydenmarchant/accumulo ACCUMULO-2943

        Alternatively you can review and apply these changes as the patch at:

        https://github.com/apache/accumulo/pull/11.patch

        To close this pull request, make a commit to your master/trunk branch
        with (at least) the following in the commit message:

        This closes #11

        ----
        commit cc9ab93aa31f517fca4fe7ccfd7caf7160e07603
        Author: haydenmarchant <hayden.marchant@gmail.com>
        Date: 2014-07-01T12:12:32Z

        ACCUMULO-2943 Fixing failures where no RNG "SUN" provider

        Both org.apache.accumulo.core.security.crypto.CrypoTest &
        org.apache.accumulo.core.file.rfile.RFileTest have lots of failures
        due to calls to SecureRandom with Random Number Generator Provider
        hard-coded as Sun. The IBM JVM has it's own built in RNG Provider
        called IBMJCE. 2 issues - hard-coded calls to
        SecureRandom.getInstance(<algo>,"SUN") and also default value in
        Property class is "SUN".

        Most failures are due to the CryptoModuleParameters instance being
        populated with default value of Crypto Secure RNG Provider, in
        particular, the following line from CryptoModelFactory.fillParamsObjectFromStringMap():

        params.setRandomNumberGeneratorProvider(cryptoOpts.get(Property.CRYPTO_SECURE_RNG_PROVIDER.getKey()));

        Since the default as described in Property class for RNG provider
        is "SUN", I have made an override mechanism in which a default
        property can be overidden by passing System property of same name.
        Any property with annotation @SystemOverride has this functionality
        enabled. So, when using a JVM which does not have the "SUN" RNG
        Provider, a system property (-Dcrypto.secure.rng.provider={provname}

        )
        can be added to the parent pom.xml in the surefire plugin definition
        (same location as the max memory for tests).

        In addition, CryptoTest.testCryptoModuleParamsParsing() has been
        changed to read from a separate config file so the on/off variants,
        since it just focuses on parsing of params.


        Show
        ASF GitHub Bot added a comment - GitHub user haydenmarchant opened a pull request: https://github.com/apache/accumulo/pull/11 ACCUMULO-2943 Fixing failures where no RNG "SUN" provider Both org.apache.accumulo.core.security.crypto.CrypoTest & org.apache.accumulo.core.file.rfile.RFileTest have lots of failures due to calls to SecureRandom with Random Number Generator Provider hard-coded as Sun. The IBM JVM has it's own built in RNG Provider called IBMJCE. 2 issues - hard-coded calls to SecureRandom.getInstance(<algo>,"SUN") and also default value in Property class is "SUN". Most failures are due to the CryptoModuleParameters instance being populated with default value of Crypto Secure RNG Provider, in particular, the following line from CryptoModelFactory.fillParamsObjectFromStringMap(): params.setRandomNumberGeneratorProvider(cryptoOpts.get(Property.CRYPTO_SECURE_RNG_PROVIDER.getKey())); Since the default as described in Property class for RNG provider is "SUN", I have made an override mechanism in which a default property can be overidden by passing System property of same name. Any property with annotation @SystemOverride has this functionality enabled. So, when using a JVM which does not have the "SUN" RNG Provider, a system property (-Dcrypto.secure.rng.provider= {provname}) can be added to the parent pom.xml in the surefire plugin definition (same location as the max memory for tests). In addition, CryptoTest.testCryptoModuleParamsParsing() has been changed to read from a separate config file since it just focuses on parsing of params and not the actual instantiation of providers etc... You can merge this pull request into a Git repository by running: $ git pull https://github.com/haydenmarchant/accumulo ACCUMULO-2943 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/accumulo/pull/11.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #11 ---- commit cc9ab93aa31f517fca4fe7ccfd7caf7160e07603 Author: haydenmarchant <hayden.marchant@gmail.com> Date: 2014-07-01T12:12:32Z ACCUMULO-2943 Fixing failures where no RNG "SUN" provider Both org.apache.accumulo.core.security.crypto.CrypoTest & org.apache.accumulo.core.file.rfile.RFileTest have lots of failures due to calls to SecureRandom with Random Number Generator Provider hard-coded as Sun. The IBM JVM has it's own built in RNG Provider called IBMJCE. 2 issues - hard-coded calls to SecureRandom.getInstance(<algo>,"SUN") and also default value in Property class is "SUN". Most failures are due to the CryptoModuleParameters instance being populated with default value of Crypto Secure RNG Provider, in particular, the following line from CryptoModelFactory.fillParamsObjectFromStringMap(): params.setRandomNumberGeneratorProvider(cryptoOpts.get(Property.CRYPTO_SECURE_RNG_PROVIDER.getKey())); Since the default as described in Property class for RNG provider is "SUN", I have made an override mechanism in which a default property can be overidden by passing System property of same name. Any property with annotation @SystemOverride has this functionality enabled. So, when using a JVM which does not have the "SUN" RNG Provider, a system property (-Dcrypto.secure.rng.provider={provname} ) can be added to the parent pom.xml in the surefire plugin definition (same location as the max memory for tests). In addition, CryptoTest.testCryptoModuleParamsParsing() has been changed to read from a separate config file so the on/off variants, since it just focuses on parsing of params.
        Hide
        ASF GitHub Bot added a comment -

        Github user madrob commented on a diff in the pull request:

        https://github.com/apache/accumulo/pull/11#discussion_r14403671

        — Diff: core/src/main/java/org/apache/accumulo/core/conf/Property.java —
        @@ -504,7 +508,11 @@ public boolean isDeprecated() {
        }

        public boolean isSensitive() {

        • return hasAnnotation(Sensitive.class) || hasPrefixWithAnnotation(getKey(), Sensitive.class);
          + return hasAnnotation(Sensitive.class) || hasPrefixWithAnnotation(getKey(), Sensitive.class);
            • End diff –

        Please try to minimize your patch set to only the affected lines. If there are outstanding formatting issues, we can address those in another patch.

        Show
        ASF GitHub Bot added a comment - Github user madrob commented on a diff in the pull request: https://github.com/apache/accumulo/pull/11#discussion_r14403671 — Diff: core/src/main/java/org/apache/accumulo/core/conf/Property.java — @@ -504,7 +508,11 @@ public boolean isDeprecated() { } public boolean isSensitive() { return hasAnnotation(Sensitive.class) || hasPrefixWithAnnotation(getKey(), Sensitive.class); + return hasAnnotation(Sensitive.class) || hasPrefixWithAnnotation(getKey(), Sensitive.class); End diff – Please try to minimize your patch set to only the affected lines. If there are outstanding formatting issues, we can address those in another patch.
        Hide
        ASF GitHub Bot added a comment -

        Github user busbey commented on the pull request:

        https://github.com/apache/accumulo/pull/11#issuecomment-47675001

        please make sure your commit is against the current head of the earliest dev branch the jira is filed against. It looks like some of your pom changes are out of date.

        Show
        ASF GitHub Bot added a comment - Github user busbey commented on the pull request: https://github.com/apache/accumulo/pull/11#issuecomment-47675001 please make sure your commit is against the current head of the earliest dev branch the jira is filed against. It looks like some of your pom changes are out of date.
        Hide
        ASF GitHub Bot added a comment -

        Github user haydenmarchant commented on the pull request:

        https://github.com/apache/accumulo/pull/11#issuecomment-48521194

        rebased with latest 1.6.1-SNAPSHOT, and removed pom.xml that accidentally sneaked into previous commit.

        Show
        ASF GitHub Bot added a comment - Github user haydenmarchant commented on the pull request: https://github.com/apache/accumulo/pull/11#issuecomment-48521194 rebased with latest 1.6.1-SNAPSHOT, and removed pom.xml that accidentally sneaked into previous commit.
        Hide
        ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

        https://github.com/apache/accumulo/pull/11

        Show
        ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/accumulo/pull/11

          People

          • Assignee:
            Hayden Marchant
            Reporter:
            Hayden Marchant
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:

              Development