Derby
  1. Derby
  2. DERBY-5970

Check that connection attributes have legal values.

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 10.10.1.1
    • Fix Version/s: None
    • Component/s: Services
    • Urgency:
      Normal
    • Bug behavior facts:
      Security

      Description

      At boot time, Derby does not check whether connection attributes are set to legal values. This can cause them to be silently ignored. In the case of security operations like re(un)encryption, these silent failures deceive the DBO into thinking that the security behavior of the database has changed when, in fact, it hasn't. We should do the following:

      1) Prevent decryptDatabase from being set to an illegal value. Since this is a new attribute, there are no backward compatibility issues.

      2) Evaluate other attributes on a case-by-case basis to determine which ones should raise exceptions if they are set to illegal values. Technically, this may result in backwardly incompatible behavior. However, I think that for most attributes, we will decide that the incompatibility is minor and is a welcome bugfix.

      1. AttributeChecks.html
        14 kB
        Rick Hillegas
      2. AttributeChecks.html
        14 kB
        Rick Hillegas
      3. AttributeChecks.html
        14 kB
        Rick Hillegas
      4. AttributeChecks.html
        7 kB
        Rick Hillegas
      5. derby-5970-02-aa-vetDataEncryptionValue.diff
        2 kB
        Rick Hillegas
      6. derby-5970-01-ab-vetDecryptDatabaseValue.diff
        3 kB
        Rick Hillegas
      7. derby-5970-01-aa-vetDecryptDatabaseValue.diff
        3 kB
        Rick Hillegas

        Issue Links

          Activity

          Hide
          Rick Hillegas added a comment -

          Attaching derby-5970-01-aa-vetDecryptDatabaseValue.diff. This patch makes Derby raise an exception if the decryptDatabase attribute is set and it is set to some value other than "true". I am running regression tests now.

          Touches the following files

          ------------

          M java/engine/org/apache/derby/impl/jdbc/EmbedConnection.java

          Vet the value of decryptDatabase early on while processing the connection request.

          ------------

          M java/testing/org/apache/derbyTesting/functionTests/tests/store/DecryptDatabaseTest.java

          Add a test case.

          Show
          Rick Hillegas added a comment - Attaching derby-5970-01-aa-vetDecryptDatabaseValue.diff. This patch makes Derby raise an exception if the decryptDatabase attribute is set and it is set to some value other than "true". I am running regression tests now. Touches the following files ------------ M java/engine/org/apache/derby/impl/jdbc/EmbedConnection.java Vet the value of decryptDatabase early on while processing the connection request. ------------ M java/testing/org/apache/derbyTesting/functionTests/tests/store/DecryptDatabaseTest.java Add a test case.
          Hide
          Knut Anders Hatlen added a comment -

          Looks like a good improvement to me.

          I'm wondering if it would be slightly more robust to use the case-insensitive Boolean.valueOf(String) library method instead of converting to lower case manually, in case there should be some exotic locale where lower-casing "TRUE" doesn't behave as one would expect (something similar to Turkish, where lower case of "I" is not "i").

          Show
          Knut Anders Hatlen added a comment - Looks like a good improvement to me. I'm wondering if it would be slightly more robust to use the case-insensitive Boolean.valueOf(String) library method instead of converting to lower case manually, in case there should be some exotic locale where lower-casing "TRUE" doesn't behave as one would expect (something similar to Turkish, where lower case of "I" is not "i").
          Hide
          Rick Hillegas added a comment -

          Thanks for the quick review, Knut. Attaching derby-5970-01-ab-vetDecryptDatabaseValue.diff, which makes the improvement you suggested. Tests passed cleanly for me on the previous rev of the patch. Committed at subversion revision 1403735.

          Show
          Rick Hillegas added a comment - Thanks for the quick review, Knut. Attaching derby-5970-01-ab-vetDecryptDatabaseValue.diff, which makes the improvement you suggested. Tests passed cleanly for me on the previous rev of the patch. Committed at subversion revision 1403735.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-5970-02-aa-vetDataEncryptionValue.diff. This patch causes Derby to raise an exception if the dataEncryption attribute is set to a value other than true. I will run regression tests.

          Touches the following files:

          -------------------

          M java/engine/org/apache/derby/impl/jdbc/EmbedConnection.java

          Raise an exception of dataEncryption is set but is not set to true.

          -------------------

          M java/testing/org/apache/derbyTesting/functionTests/master/URLCheck.out

          Verify that an exception is raised if dataEncryption is set to something other than true.

          Show
          Rick Hillegas added a comment - Attaching derby-5970-02-aa-vetDataEncryptionValue.diff. This patch causes Derby to raise an exception if the dataEncryption attribute is set to a value other than true. I will run regression tests. Touches the following files: ------------------- M java/engine/org/apache/derby/impl/jdbc/EmbedConnection.java Raise an exception of dataEncryption is set but is not set to true. ------------------- M java/testing/org/apache/derbyTesting/functionTests/master/URLCheck.out Verify that an exception is raised if dataEncryption is set to something other than true.
          Hide
          Rick Hillegas added a comment -

          Tests passed cleanly for me on derby-5970-02-aa-vetDataEncryptionValue.diff.

          Show
          Rick Hillegas added a comment - Tests passed cleanly for me on derby-5970-02-aa-vetDataEncryptionValue.diff.
          Hide
          Rick Hillegas added a comment -

          Committed derby-5970-02-aa-vetDataEncryptionValue.diff at subversion revision 1404944.

          Show
          Rick Hillegas added a comment - Committed derby-5970-02-aa-vetDataEncryptionValue.diff at subversion revision 1404944.
          Hide
          Rick Hillegas added a comment -

          We might be able to use the optional ij URLCheck logic to help us raise a SQLWarning when attributes have illegal values but backward compatibility concerns prevent us from raising an exception.

          Show
          Rick Hillegas added a comment - We might be able to use the optional ij URLCheck logic to help us raise a SQLWarning when attributes have illegal values but backward compatibility concerns prevent us from raising an exception.
          Hide
          Rick Hillegas added a comment -

          Attaching AttributeChecks.html. This summarizes Derby's behavior when processing URL attributes with bad values. This may inform additional work done on this issue.

          Show
          Rick Hillegas added a comment - Attaching AttributeChecks.html. This summarizes Derby's behavior when processing URL attributes with bad values. This may inform additional work done on this issue.
          Hide
          Knut Anders Hatlen added a comment - - edited

          DERBY-5907 suggests that we move that logic out of ij. After support for warnings on connect calls is added to the client, that is.

          Show
          Knut Anders Hatlen added a comment - - edited DERBY-5907 suggests that we move that logic out of ij. After support for warnings on connect calls is added to the client, that is.
          Hide
          Rick Hillegas added a comment -

          Attaching rev 2 of AttributeChecks.html. In this second rev I offer recommendations for additional sanity-checking of attributes.

          Show
          Rick Hillegas added a comment - Attaching rev 2 of AttributeChecks.html. In this second rev I offer recommendations for additional sanity-checking of attributes.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks for writing this up so systematically, Rick. The proposed changes look reasonable to me.

          For the drop attribute, I think it would be OK to throw an exception rather than a warning if it's set to an invalid value. Applications that break because of that change have a bug and need fixing, so I wouldn't worry about backward compatibility in that particular case.

          failover: Should we raise an error when the value is not true/false, for consistency with the proposed changes for the other replication attributes?

          Show
          Knut Anders Hatlen added a comment - Thanks for writing this up so systematically, Rick. The proposed changes look reasonable to me. For the drop attribute, I think it would be OK to throw an exception rather than a warning if it's set to an invalid value. Applications that break because of that change have a bug and need fixing, so I wouldn't worry about backward compatibility in that particular case. failover: Should we raise an error when the value is not true/false, for consistency with the proposed changes for the other replication attributes?
          Hide
          Rick Hillegas added a comment -

          Thanks for those good suggestions, Knut. Attaching a third rev of the spec, incorporating your feedback.

          Show
          Rick Hillegas added a comment - Thanks for those good suggestions, Knut. Attaching a third rev of the spec, incorporating your feedback.
          Hide
          Knut Anders Hatlen added a comment -

          territory: The reference manual says one place that it can be used on creation and on upgrade, another place it says only on creation. I tried it and found that the territory attribute had no effect on upgrade (will file a JIRA to fix the docs). If that's correct, we should probably also raise a warning if territory is set on upgrade.

          Show
          Knut Anders Hatlen added a comment - territory: The reference manual says one place that it can be used on creation and on upgrade, another place it says only on creation. I tried it and found that the territory attribute had no effect on upgrade (will file a JIRA to fix the docs). If that's correct, we should probably also raise a warning if territory is set on upgrade.
          Hide
          Rick Hillegas added a comment -

          Thanks for that additional feedback, Knut. Attaching a fourth rev of the spec, which incorporates that suggestion.

          Show
          Rick Hillegas added a comment - Thanks for that additional feedback, Knut. Attaching a fourth rev of the spec, which incorporates that suggestion.
          Hide
          Mike Matrigali added a comment -

          this seems more like a improvement than a bug. not marking it as a regression.

          Show
          Mike Matrigali added a comment - this seems more like a improvement than a bug. not marking it as a regression.

            People

            • Assignee:
              Unassigned
              Reporter:
              Rick Hillegas
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:

                Development