OpenJPA
  1. OpenJPA
  2. OPENJPA-1089

Provide for password encryption within persistence.xml

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.3.0, 2.0.0
    • Fix Version/s: 1.3.0, 2.0.0-M3
    • Component/s: jpa
    • Labels:
      None
    • Patch Info:
      Patch Available

      Description

      A recent discussion on our users forum [1] has surfaced (again) the need to encrypt the password fields in the persistence.xml. In the particular scenario outlined in the posting, this user wanted to encrypt the password sent into Apache DBCP via the url string. In my mind, that's a separate problem related to DBCP.

      But, OpenJPA has openjpa.Connection*Password properties that could be encrypted. And, the new JPA 2 spec outlines a javax.persistence.jdbc.password property that would be nice to encrypt.

      I'm opening this Issue as a Feature request, but it could also be considered a bug since a non-jndi environment is crippled from a security standpoint.

      [1] http://n2.nabble.com/How-to-encrypt-DB-password-in-persistence.xml-td2868212.html

      1. OPENJPA-1089-2.patch
        16 kB
        Rick Curtis
      2. OPENJPA-1089.PATCH
        11 kB
        Rick Curtis

        Activity

        Hide
        Donald Woods added a comment -

        We have a similar feature in Apache Geronimo for our config.xml and deployment plans. The only downside of adding this to OpenJPA, is we would then have to follow the ASF Cryptography release guidelines at -
        http://www.apache.org/dev/crypto.html
        since we would be using encryption/decryption (even if provided by the JVM). Not a biggie, but adds a few steps to the release process...

        Show
        Donald Woods added a comment - We have a similar feature in Apache Geronimo for our config.xml and deployment plans. The only downside of adding this to OpenJPA, is we would then have to follow the ASF Cryptography release guidelines at - http://www.apache.org/dev/crypto.html since we would be using encryption/decryption (even if provided by the JVM). Not a biggie, but adds a few steps to the release process...
        Hide
        Rick Curtis added a comment -

        I created a plugin that allows a user to implement a EncryptionProvider interface so the OpenJPA runtime will call their interface to decrypt openjpa.Connection(2)Password values. Please take a look and let me know what you think. If everything looks good I'll fix up the user manual.

        -Rick

        Show
        Rick Curtis added a comment - I created a plugin that allows a user to implement a EncryptionProvider interface so the OpenJPA runtime will call their interface to decrypt openjpa.Connection(2)Password values. Please take a look and let me know what you think. If everything looks good I'll fix up the user manual. -Rick
        Hide
        Rick Curtis added a comment -

        I'd also like to comment that DBCP has a JIRA [1.] open for the same issue.

        -Rick

        [1.] http://issues.apache.org/jira/browse/DBCP-297

        Show
        Rick Curtis added a comment - I'd also like to comment that DBCP has a JIRA [1.] open for the same issue. -Rick [1.] http://issues.apache.org/jira/browse/DBCP-297
        Hide
        Donald Woods added a comment -

        Geronimo has been working on something similar for its config files - https://issues.apache.org/jira/browse/GERONIMO-3003

        We've had encrypt/decrypt for password stores and deployer connections for awhile, so wondering if we need to say that encrypted passwords must be Base64 encoded, so they can always be passed in as a String (whereas some encrypted data could include 0x00 and quotes by default)?

        Show
        Donald Woods added a comment - Geronimo has been working on something similar for its config files - https://issues.apache.org/jira/browse/GERONIMO-3003 We've had encrypt/decrypt for password stores and deployer connections for awhile, so wondering if we need to say that encrypted passwords must be Base64 encoded, so they can always be passed in as a String (whereas some encrypted data could include 0x00 and quotes by default)?
        Hide
        Donald Woods added a comment -

        This would be a definite value-add for OpenJPA...

        Show
        Donald Woods added a comment - This would be a definite value-add for OpenJPA...
        Hide
        Rick Curtis added a comment -

        Maybe I'm being a bit dense here... but isn't it enough to state that encrypted passwords must be a non-null, valid string?

        Show
        Rick Curtis added a comment - Maybe I'm being a bit dense here... but isn't it enough to state that encrypted passwords must be a non-null, valid string?
        Hide
        Pinaki Poddar added a comment -

        Rick's patch looked good. Why do not you commit it, Rick?

        Show
        Pinaki Poddar added a comment - Rick's patch looked good. Why do not you commit it, Rick?
        Hide
        Rick Curtis added a comment -

        Pinaki – I'll be glad to commit the patch as soon as I'm a committer

        Donald - Do you think we need to be more explicit in stating what are valid characters for encrypted passwords? Will the requirement that an encrypted password must be a string be strict enough?

        OPENJPA-1089-2.patch includes a minor update to the javadoc on the EncryptionProvider interface and it also includes new docs.

        Show
        Rick Curtis added a comment - Pinaki – I'll be glad to commit the patch as soon as I'm a committer Donald - Do you think we need to be more explicit in stating what are valid characters for encrypted passwords? Will the requirement that an encrypted password must be a string be strict enough? OPENJPA-1089 -2.patch includes a minor update to the javadoc on the EncryptionProvider interface and it also includes new docs.
        Hide
        Michael Dick added a comment -

        I ran into a problem with the testcase:
        TestPersistenceProductDerivation:109
        assertTrue(actual.containsAll(Arrays.asList(expectedPUs))); // fails
        assertTrue(actual.containsAll(expectedPUs)); // passes

        Otherwise the patch looks good, running the full regression bucket now.

        Show
        Michael Dick added a comment - I ran into a problem with the testcase: TestPersistenceProductDerivation:109 assertTrue(actual.containsAll(Arrays.asList(expectedPUs))); // fails assertTrue(actual.containsAll(expectedPUs)); // passes Otherwise the patch looks good, running the full regression bucket now.
        Hide
        Michael Dick added a comment -

        Thanks for the patch Rick

        Show
        Michael Dick added a comment - Thanks for the patch Rick
        Hide
        Michael Dick added a comment -

        Reopening to port to 1.3.0

        Show
        Michael Dick added a comment - Reopening to port to 1.3.0

          People

          • Assignee:
            Michael Dick
            Reporter:
            Kevin Sutter
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development