Derby
  1. Derby
  2. DERBY-4483

Provide a way to change the hash algorithm used by BUILTIN authentication

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.5.3.0
    • Fix Version/s: 10.6.1.0
    • Component/s: Services
    • Labels:
      None
    • Issue & fix info:
      Release Note Needed
    • Bug behavior facts:
      Security

      Description

      The BUILTIN authentication scheme protects the passwords by hashing them with the SHA-1 algorithm. It would be nice to have way to specify a different algorithm so that users can take advantage of new, stronger algorithms provided by their JCE provider if so desired.

      This issue tracks our response to security vulnerability CVE-2009-4269, which Marcell Major identified. See http://marcellmajor.com/derbyhash.html

      1. upgrade-test.diff
        7 kB
        Knut Anders Hatlen
      2. toHexByte.diff
        5 kB
        Knut Anders Hatlen
      3. releaseNote.html
        7 kB
        Knut Anders Hatlen
      4. releaseNote.html
        7 kB
        Knut Anders Hatlen
      5. experiment.diff
        13 kB
        Knut Anders Hatlen
      6. errormsg.diff
        3 kB
        Knut Anders Hatlen
      7. derby-4483-2a.stat
        0.4 kB
        Knut Anders Hatlen
      8. derby-4483-2a.diff
        6 kB
        Knut Anders Hatlen
      9. derby-4483-1a.stat
        0.7 kB
        Knut Anders Hatlen
      10. derby-4483-1a.diff
        33 kB
        Knut Anders Hatlen
      11. comments.diff
        5 kB
        Knut Anders Hatlen

        Issue Links

          Activity

          Hide
          Knut Anders Hatlen added a comment -

          The work on this issue has been completed, so I'm marking it as resolved. If there are more problems, please file new JIRA issues to handle those.

          Show
          Knut Anders Hatlen added a comment - The work on this issue has been completed, so I'm marking it as resolved. If there are more problems, please file new JIRA issues to handle those.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks Rick. I'm fine with that approach, and have posted a patch on DERBY-4602 that implements what you suggested. I think this change warrants a release note in itself, though, as databases created on a more capable platform may now need some extra maintenance steps before being usable on a more restricted platform if authentication has been enabled.

          Show
          Knut Anders Hatlen added a comment - Thanks Rick. I'm fine with that approach, and have posted a patch on DERBY-4602 that implements what you suggested. I think this change warrants a release note in itself, though, as databases created on a more capable platform may now need some extra maintenance steps before being usable on a more restricted platform if authentication has been enabled.
          Hide
          Rick Hillegas added a comment -

          Hi Knut,

          I am not thrilled by using SHA-1 as the default algorithm. It is not considered secure enough for use by the U.S. government as of this year; see http://en.wikipedia.org/wiki/SHA_hash_functions I would prefer to see a solution like this:

          1) If the user specifies an algorithm, use it

          2) Otherwise, try to use SHA-256

          3) If SHA_256 isn't available, fall back on SHA-1

          4) If even that isn't available, then raise an error

          What do you think?

          Thanks,
          -Rick

          Show
          Rick Hillegas added a comment - Hi Knut, I am not thrilled by using SHA-1 as the default algorithm. It is not considered secure enough for use by the U.S. government as of this year; see http://en.wikipedia.org/wiki/SHA_hash_functions I would prefer to see a solution like this: 1) If the user specifies an algorithm, use it 2) Otherwise, try to use SHA-256 3) If SHA_256 isn't available, fall back on SHA-1 4) If even that isn't available, then raise an error What do you think? Thanks, -Rick
          Hide
          Knut Anders Hatlen added a comment -

          To make it easier to choose another algorithm than the default, we may want to handle derby.authentication.builtin.algorithm more like derby.database.sqlAuthorization during database creation. If d.d.sqlAuthorization is set as a system property when a database is created, its value will be persisted in the database. This way it will be possible to change the algorithm at database creation time without executing any SQL statements.

          Show
          Knut Anders Hatlen added a comment - To make it easier to choose another algorithm than the default, we may want to handle derby.authentication.builtin.algorithm more like derby.database.sqlAuthorization during database creation. If d.d.sqlAuthorization is set as a system property when a database is created, its value will be persisted in the database. This way it will be possible to change the algorithm at database creation time without executing any SQL statements.
          Hide
          Knut Anders Hatlen added a comment -

          It looks like we'll have to change the default algorithm to SHA-1, since SHA-256 is not supported on weme 6.2 (see DERBY-4602). It seems the only algorithms available on all supported platforms are SHA-1 and MD5.

          Show
          Knut Anders Hatlen added a comment - It looks like we'll have to change the default algorithm to SHA-1, since SHA-256 is not supported on weme 6.2 (see DERBY-4602 ). It seems the only algorithms available on all supported platforms are SHA-1 and MD5.
          Hide
          Rick Hillegas added a comment -

          Hi Knut,

          The (2a) patch looks good to me. +1 Thanks.

          Show
          Rick Hillegas added a comment - Hi Knut, The (2a) patch looks good to me. +1 Thanks.
          Hide
          Knut Anders Hatlen added a comment -

          I went ahead and committed errormsg.diff (revision 927367) and derby-4483-2a.diff (revision 927368) as they were, since the only unresolved issue now appears to be the wording of the error message, which could easily be changed once we reach agreement.

          I'm also attaching an updated release note which contains the error message used in the current code on trunk.

          Show
          Knut Anders Hatlen added a comment - I went ahead and committed errormsg.diff (revision 927367) and derby-4483-2a.diff (revision 927368) as they were, since the only unresolved issue now appears to be the wording of the error message, which could easily be changed once we reach agreement. I'm also attaching an updated release note which contains the error message used in the current code on trunk.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks Kim and Bryan.

          I'm not sure if the last sentence would have to be removed for the next release. Since users may skip versions when they upgrade, it may be just as relevant in future releases. But then there's the problem that we say "refer to the release note" although it won't be mentioned in the release notes for that future release, so I agree it may be somewhat confusing.

          Perhaps we should keep the sentence for now and file a JIRA issue to get it removed from trunk once the 10.6 branch has been cut? Then all 10.6.X releases will give this hint, hopefully helping most of those who'll be hit by the issue, while future feature releases won't have a reference to the release notes for an ancient release. And would it be better to say "refer to the release notes for Derby 10.6.1" instead of "release note for DERBY-4483"? That may make it clearer where to look if a user gets this message in a later release.

          Show
          Knut Anders Hatlen added a comment - Thanks Kim and Bryan. I'm not sure if the last sentence would have to be removed for the next release. Since users may skip versions when they upgrade, it may be just as relevant in future releases. But then there's the problem that we say "refer to the release note" although it won't be mentioned in the release notes for that future release, so I agree it may be somewhat confusing. Perhaps we should keep the sentence for now and file a JIRA issue to get it removed from trunk once the 10.6 branch has been cut? Then all 10.6.X releases will give this hint, hopefully helping most of those who'll be hit by the issue, while future feature releases won't have a reference to the release notes for an ancient release. And would it be better to say "refer to the release notes for Derby 10.6.1" instead of "release note for DERBY-4483 "? That may make it clearer where to look if a user gets this message in a later release.
          Hide
          Bryan Pendleton added a comment -

          That error message reads fine to me, thanks for putting it together. I agree that mentioning
          the issue in the error message is unusual, but I think it is worth the risk if it helps people
          track down the cause of what might appear to be a frustrating regression in behavior.

          Show
          Bryan Pendleton added a comment - That error message reads fine to me, thanks for putting it together. I agree that mentioning the issue in the error message is unusual, but I think it is worth the risk if it helps people track down the cause of what might appear to be a frustrating regression in behavior.
          Hide
          Kim Haase added a comment -

          Is there a precedent for having an error message that is specific to a particular release? It could be a maintenance headache because you'd have to remember to remove the last sentence for the next release.

          I would guess that if they got the message and were puzzled by it, they might go to the release notes anyway.

          Show
          Kim Haase added a comment - Is there a precedent for having an error message that is specific to a particular release? It could be a maintenance headache because you'd have to remember to remove the last sentence for the next release. I would guess that if they got the message and were puzzled by it, they might go to the release notes anyway.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks for the suggestions about the error message. That sounds like a good idea. The attached patch (errormsg.diff) makes BasicAuthenticationServiceImpl.authenticateUser() raise an exception with a custom error message if strong password substitution is enabled. Now it will look this on the client:

          ERROR 08004: DERBY SQL error: SQLCODE: -1, SQLSTATE: 08004, SQLERRMC: Connection authentication failure occurred. Either the supplied credentials were invalid, or the database uses a password encryption scheme which is not compatible with the strong password substitution security mechanism. If this error started after upgrade, refer to the release note for DERBY-4483 for options.

          I think this patch could go in independently of the patch that changes the default, if the wording in the new message sounds OK.

          Show
          Knut Anders Hatlen added a comment - Thanks for the suggestions about the error message. That sounds like a good idea. The attached patch (errormsg.diff) makes BasicAuthenticationServiceImpl.authenticateUser() raise an exception with a custom error message if strong password substitution is enabled. Now it will look this on the client: ERROR 08004: DERBY SQL error: SQLCODE: -1, SQLSTATE: 08004, SQLERRMC: Connection authentication failure occurred. Either the supplied credentials were invalid, or the database uses a password encryption scheme which is not compatible with the strong password substitution security mechanism. If this error started after upgrade, refer to the release note for DERBY-4483 for options. I think this patch could go in independently of the patch that changes the default, if the wording in the new message sounds OK.
          Hide
          Kathey Marsden added a comment -

          It might be nice even if the error message is somewhat prescriptive without being too wordy like:

          ERROR 08004: Connection authentication failure occurred. Reason: userid or password invalid, or the database may be using an alternate password encryption scheme. If this error started after upgrade refer to the DERBY-4483 release note for options.

          I don't know if there is a precedent for referring to Jira issues in messages, but it might help users with self service if they encounter the issue on upgrade.

          Show
          Kathey Marsden added a comment - It might be nice even if the error message is somewhat prescriptive without being too wordy like: ERROR 08004: Connection authentication failure occurred. Reason: userid or password invalid, or the database may be using an alternate password encryption scheme. If this error started after upgrade refer to the DERBY-4483 release note for options. I don't know if there is a precedent for referring to Jira issues in messages, but it might help users with self service if they encounter the issue on upgrade.
          Hide
          Bryan Pendleton added a comment -

          The release note is clear and well-written. Thanks for putting it together. +1

          Would there be any benefit to augmenting the client-side error message that is received when using strong password substitution against a conf-hash DB, so that the client-side message suggests this alternate possibility for the error? Something along the lines of:

          ERROR 08004: Connection authentication failure occurred. Reason: userid or password invalid, or the database may be using an alternate password encryption scheme.

          Show
          Bryan Pendleton added a comment - The release note is clear and well-written. Thanks for putting it together. +1 Would there be any benefit to augmenting the client-side error message that is received when using strong password substitution against a conf-hash DB, so that the client-side message suggests this alternate possibility for the error? Something along the lines of: ERROR 08004: Connection authentication failure occurred. Reason: userid or password invalid, or the database may be using an alternate password encryption scheme.
          Hide
          Knut Anders Hatlen added a comment -

          Attached is a new patch (2a) that enables the configurable hash scheme by default in new databases. The patch makes SHA-256 the default algorithm. SHA-256 is believed to be more secure than the currently used SHA-1 algorithm, and it's also one of the algorithms NIST recommended U.S. Government agencies to use instead of SHA-1 (see http://csrc.nist.gov/groups/ST/toolkit/secure_hashing.html#Approved%20Algorithms). The default algorithm can easily be changed, though, if someone thinks we should have another default. Also, it's possible to change the default in a future release just by changing the value of a constant, and that should not have any compatibility implications that I'm aware of, so we won't be stuck forever with the algorithm we pick here.

          Making the configurable hash authentication scheme the default authentication scheme has one known compatibility implication: Strong password substitution when exchanging credentials between the network client and the server will not work in new databases unless you manually disable the configurable hash authentication scheme first (by setting the derby.authentication.builtin.algorithm property to null). Because of this, I'm attaching a release note as well.

          Here's a description of the changes made by the patch:

          • iapi/reference/Property.java: added a constant for the default value (SHA-256) of the property that enables the new scheme
          • impl/sql/catalog/DataDictionaryImpl.java: set the database property when the database is created (note: only on database creation, so upgraded databases will continue working the same way as before)
          • tests/jdbcapi/AuthenticationTest.java: added test case to verify that the property was initialized to SHA-256
          • tests/upgradeTests/Changes10_6.java: added test case to verify that the authentication scheme does not change on upgrade
          • tests/derbynet/NSSecurityMechanismTest.java: disable the new scheme for the test case that tests strong password substitution together with BUILTIN authentication

          All the regression tests ran cleanly with the patch. Comments on the patch and the release note would be appreciated. Thanks.

          Show
          Knut Anders Hatlen added a comment - Attached is a new patch (2a) that enables the configurable hash scheme by default in new databases. The patch makes SHA-256 the default algorithm. SHA-256 is believed to be more secure than the currently used SHA-1 algorithm, and it's also one of the algorithms NIST recommended U.S. Government agencies to use instead of SHA-1 (see http://csrc.nist.gov/groups/ST/toolkit/secure_hashing.html#Approved%20Algorithms ). The default algorithm can easily be changed, though, if someone thinks we should have another default. Also, it's possible to change the default in a future release just by changing the value of a constant, and that should not have any compatibility implications that I'm aware of, so we won't be stuck forever with the algorithm we pick here. Making the configurable hash authentication scheme the default authentication scheme has one known compatibility implication: Strong password substitution when exchanging credentials between the network client and the server will not work in new databases unless you manually disable the configurable hash authentication scheme first (by setting the derby.authentication.builtin.algorithm property to null). Because of this, I'm attaching a release note as well. Here's a description of the changes made by the patch: iapi/reference/Property.java: added a constant for the default value (SHA-256) of the property that enables the new scheme impl/sql/catalog/DataDictionaryImpl.java: set the database property when the database is created (note: only on database creation, so upgraded databases will continue working the same way as before) tests/jdbcapi/AuthenticationTest.java: added test case to verify that the property was initialized to SHA-256 tests/upgradeTests/Changes10_6.java: added test case to verify that the authentication scheme does not change on upgrade tests/derbynet/NSSecurityMechanismTest.java: disable the new scheme for the test case that tests strong password substitution together with BUILTIN authentication All the regression tests ran cleanly with the patch. Comments on the patch and the release note would be appreciated. Thanks.
          Hide
          Knut Anders Hatlen added a comment -

          Committed toHexByte.diff to trunk with revision 926520.

          Show
          Knut Anders Hatlen added a comment - Committed toHexByte.diff to trunk with revision 926520.
          Hide
          Knut Anders Hatlen added a comment -

          Here's a patch that moves toHexByte() from StringUtil to AuthenticationServiceBase and makes it private. It also removes the offset and length parameters because all callers operate on the entire string, so they're not needed, and since the original method also seems to have a couple of bugs in the handling of offset/length, it's better to remove those parameters to prevent other code from starting using them.

          Show
          Knut Anders Hatlen added a comment - Here's a patch that moves toHexByte() from StringUtil to AuthenticationServiceBase and makes it private. It also removes the offset and length parameters because all callers operate on the entire string, so they're not needed, and since the original method also seems to have a couple of bugs in the handling of offset/length, it's better to remove those parameters to prevent other code from starting using them.
          Hide
          Knut Anders Hatlen added a comment -

          Committed comments.diff to trunk with revision 924746.

          Show
          Knut Anders Hatlen added a comment - Committed comments.diff to trunk with revision 924746.
          Hide
          Knut Anders Hatlen added a comment -

          Here's a patch that updates some comments with information about the extra requirement for strong password substitution (touches code in the client and the server, as well as in the engine). It also fixes up one more symbol name that refers to the SHA-1 based old scheme as the new scheme.

          Show
          Knut Anders Hatlen added a comment - Here's a patch that updates some comments with information about the extra requirement for strong password substitution (touches code in the client and the server, as well as in the engine). It also fixes up one more symbol name that refers to the SHA-1 based old scheme as the new scheme.
          Hide
          Knut Anders Hatlen added a comment -

          Hi Bryan,

          That sounds like a good idea, since we'd want to encourage the use of the more robust String.getBytes("UTF-8") in new code.

          Show
          Knut Anders Hatlen added a comment - Hi Bryan, That sounds like a good idea, since we'd want to encourage the use of the more robust String.getBytes("UTF-8") in new code.
          Hide
          Bryan Pendleton added a comment -

          It seems like the only call to StringUtil.toHexByte is now in AuthenticationServiceBase.

          Would it make sense to move the toHexByte method from StringUtil to AuthenticationServiceBase,
          and make it private there?

          Show
          Bryan Pendleton added a comment - It seems like the only call to StringUtil.toHexByte is now in AuthenticationServiceBase. Would it make sense to move the toHexByte method from StringUtil to AuthenticationServiceBase, and make it private there?
          Hide
          Knut Anders Hatlen added a comment -

          Thanks Rick! I've committed the patch with revision 922304.

          I plan to post a follow up patch which adds more details to the comments about strong password substitution. The current comments state that it can be used with BUILTIN authentication, but now that's only partially true.

          Show
          Knut Anders Hatlen added a comment - Thanks Rick! I've committed the patch with revision 922304. I plan to post a follow up patch which adds more details to the comments about strong password substitution. The current comments state that it can be used with BUILTIN authentication, but now that's only partially true.
          Hide
          Rick Hillegas added a comment -

          Thanks for these changes, Knut. +1

          Show
          Rick Hillegas added a comment - Thanks for these changes, Knut. +1
          Hide
          Knut Anders Hatlen added a comment -

          Attaching a new patch (derby-4483-1a) which merges the experiment patch and the upgrade test patch. Additionally, it contains the following changes:

          • Some test cases have been added to AuthenticationTest (suggestions for more tests are welcome!)
          • Comments have been expanded to contain most of the information in the writeup
          • Renamed the old authentication mechanism to prevent confusion (including changes to constant names and method names)
          • Added a more specific error message for the case where an invalid algorithm name has been specified:

          ij> call syscs_util.syscs_set_database_property('derby.user.kah', 'test');
          ERROR XBCXW: The message digest algorithm 'xyz' is not supported by any of the available cryptography providers. Please install a cryptography provider that supports that algorithm, or specify another algorithm in the derby.authentication.builtin.algorithm property.
          ERROR XJ001: Java exception: 'xyz MessageDigest not available: java.security.NoSuchAlgorithmException'.

          I believe this patch is ready for commit.

          Show
          Knut Anders Hatlen added a comment - Attaching a new patch (derby-4483-1a) which merges the experiment patch and the upgrade test patch. Additionally, it contains the following changes: Some test cases have been added to AuthenticationTest (suggestions for more tests are welcome!) Comments have been expanded to contain most of the information in the writeup Renamed the old authentication mechanism to prevent confusion (including changes to constant names and method names) Added a more specific error message for the case where an invalid algorithm name has been specified: ij> call syscs_util.syscs_set_database_property('derby.user.kah', 'test'); ERROR XBCXW: The message digest algorithm 'xyz' is not supported by any of the available cryptography providers. Please install a cryptography provider that supports that algorithm, or specify another algorithm in the derby.authentication.builtin.algorithm property. ERROR XJ001: Java exception: 'xyz MessageDigest not available: java.security.NoSuchAlgorithmException'. I believe this patch is ready for commit.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks for looking at the patch and providing these useful comments
          and suggestions, Rick! I've added my responses below.

          > o Thanks for the extensive write-up explaining how the new code
          > works. It would be helpful if that writeup were included in a
          > header comment somewhere.

          That's a good suggestion. Will do that.

          > o I did not understand why the prefixes 3b60 and 3b61 were chosen to
          > flag authentication schemes. Since you have been in there and
          > probably understand why those strings are used rather than some
          > other strings, it would be helpful if you could record that
          > reasoning in a comment.

          I'm afraid I have no idea where the prefix 3b60 comes from. I
          generated 3b61 just by adding 1 to the first prefix. The only
          requirement, as far as I can see, is that the prefixes must be
          unique. I'll add a comment saying that.

          > o The symbol name ID_PATTERN_NEW_SCHEME suggests that there is an
          > even older scheme which might still be used in really old
          > databases. Is that possible? If so, does
          > BasicAuthenticationServiceImpl.encryptPasswordUsingStoredAlgorithm()
          > need to handle another case? If not, it would be less confusing if
          > this symbol were renamed so that it did not suggest an impossibile
          > situation to unwary readers like me.

          There is no other scheme that must be handled in the current code. The
          naming is probably a remnant from an old Cloudscape release which
          contained another ("old") authentication scheme. I think I'll go ahead
          with the suggestion from the writeup and rename the "new
          authentication scheme" to "SHA-1 authentication scheme" and update
          symbol names accordingly.

          > o If AuthenticationServiceBase.encryptPassword() really is only used
          > by the newly introduced configurable scheme, it would be helpful
          > if the name of this method indicated that.

          There are two encryptPassword() methods with different signatures in
          that class; one for the the existing scheme and one for the
          configurable scheme. Adding the name of the scheme to those methods
          sounds like a good suggestion.

          > o I agree that it would be good to add a more specific error message
          > in that method.

          Will do that.

          Show
          Knut Anders Hatlen added a comment - Thanks for looking at the patch and providing these useful comments and suggestions, Rick! I've added my responses below. > o Thanks for the extensive write-up explaining how the new code > works. It would be helpful if that writeup were included in a > header comment somewhere. That's a good suggestion. Will do that. > o I did not understand why the prefixes 3b60 and 3b61 were chosen to > flag authentication schemes. Since you have been in there and > probably understand why those strings are used rather than some > other strings, it would be helpful if you could record that > reasoning in a comment. I'm afraid I have no idea where the prefix 3b60 comes from. I generated 3b61 just by adding 1 to the first prefix. The only requirement, as far as I can see, is that the prefixes must be unique. I'll add a comment saying that. > o The symbol name ID_PATTERN_NEW_SCHEME suggests that there is an > even older scheme which might still be used in really old > databases. Is that possible? If so, does > BasicAuthenticationServiceImpl.encryptPasswordUsingStoredAlgorithm() > need to handle another case? If not, it would be less confusing if > this symbol were renamed so that it did not suggest an impossibile > situation to unwary readers like me. There is no other scheme that must be handled in the current code. The naming is probably a remnant from an old Cloudscape release which contained another ("old") authentication scheme. I think I'll go ahead with the suggestion from the writeup and rename the "new authentication scheme" to "SHA-1 authentication scheme" and update symbol names accordingly. > o If AuthenticationServiceBase.encryptPassword() really is only used > by the newly introduced configurable scheme, it would be helpful > if the name of this method indicated that. There are two encryptPassword() methods with different signatures in that class; one for the the existing scheme and one for the configurable scheme. Adding the name of the scheme to those methods sounds like a good suggestion. > o I agree that it would be good to add a more specific error message > in that method. Will do that.
          Hide
          Rick Hillegas added a comment -

          Hi Knut,

          Thanks for the experiment.patch increment. I had a couple polishing issues:

          o Thanks for the extensive write-up explaining how the new code works. It would be helpful if that writeup were included in a header comment somewhere.

          o I did not understand why the prefixes 3b60 and 3b61 were chosen to flag authentication schemes. Since you have been in there and probably understand why those strings are used rather than some other strings, it would be helpful if you could record that reasoning in a comment.

          o The symbol name ID_PATTERN_NEW_SCHEME suggests that there is an even older scheme which might still be used in really old databases. Is that possible? If so, does BasicAuthenticationServiceImpl.encryptPasswordUsingStoredAlgorithm() need to handle another case? If not, it would be less confusing if this symbol were renamed so that it did not suggest an impossibile situation to unwary readers like me.

          o If AuthenticationServiceBase.encryptPassword() really is only used by the newly introduced configurable scheme, it would be helpful if the name of this method indicated that.

          o I agree that it would be good to add a more specific error message in that method.

          Thanks,
          -Rick

          Show
          Rick Hillegas added a comment - Hi Knut, Thanks for the experiment.patch increment. I had a couple polishing issues: o Thanks for the extensive write-up explaining how the new code works. It would be helpful if that writeup were included in a header comment somewhere. o I did not understand why the prefixes 3b60 and 3b61 were chosen to flag authentication schemes. Since you have been in there and probably understand why those strings are used rather than some other strings, it would be helpful if you could record that reasoning in a comment. o The symbol name ID_PATTERN_NEW_SCHEME suggests that there is an even older scheme which might still be used in really old databases. Is that possible? If so, does BasicAuthenticationServiceImpl.encryptPasswordUsingStoredAlgorithm() need to handle another case? If not, it would be less confusing if this symbol were renamed so that it did not suggest an impossibile situation to unwary readers like me. o If AuthenticationServiceBase.encryptPassword() really is only used by the newly introduced configurable scheme, it would be helpful if the name of this method indicated that. o I agree that it would be good to add a more specific error message in that method. Thanks, -Rick
          Hide
          Knut Anders Hatlen added a comment -

          Attaching an upgrade test that verifies that the new algorithm is only used in hard-upgraded databases. The upgrade test framework does not support tests that use authentication, but I found that the roles tests worked around this by creating a separate database and manually enabling authentication on it, so I followed that pattern.

          Show
          Knut Anders Hatlen added a comment - Attaching an upgrade test that verifies that the new algorithm is only used in hard-upgraded databases. The upgrade test framework does not support tests that use authentication, but I found that the roles tests worked around this by creating a separate database and manually enabling authentication on it, so I followed that pattern.
          Hide
          Knut Anders Hatlen added a comment -

          Hi Bryan,

          >> This means that a database may contain passwords that are hashed using different algorithms.
          >
          > Can the Derby administrator or DBA tell that they are in this state? Is there a way to know
          > which user/passwords are hashed with which algorithm, during a situation in which my database has
          > such a mixture?

          If the DBA has a list of all users in the system, he could query the derby.user.* database properties and look at their suffixes. For example:

          ij> select name, substr(pw, length(pw) - 9, 10) from
          > (select u, syscs_util.syscs_get_database_property(rtrim('derby.user.'||u))
          > from (values 'knut', 'bryan', 'dag') v(u)) s(name, pw);
          NAME |2
          ----------------
          knut |5de26b791b
          bryan|f4a77c:MD5
          dag |a2:SHA-512

          3 rows selected

          Here, the user 'knut' has a password that's hashed with the old scheme (there's no algorithm name in the suffix), 'bryan' has a password hashed with MD5, and 'dag' with SHA-512.

          I don't think we currently have any way of listing all database properties, so the DBA must know all users up front in order to perform this query. It might be useful (in a separate JIRA) to add a new diagnostic table function to improve the monitoring capabilities. Then we could simply have done something like this:

          select * from table ( syscs_diag.database_properties('derby.user.%') ) p;

          Show
          Knut Anders Hatlen added a comment - Hi Bryan, >> This means that a database may contain passwords that are hashed using different algorithms. > > Can the Derby administrator or DBA tell that they are in this state? Is there a way to know > which user/passwords are hashed with which algorithm, during a situation in which my database has > such a mixture? If the DBA has a list of all users in the system, he could query the derby.user.* database properties and look at their suffixes. For example: ij> select name, substr(pw, length(pw) - 9, 10) from > (select u, syscs_util.syscs_get_database_property(rtrim('derby.user.'||u)) > from (values 'knut', 'bryan', 'dag') v(u)) s(name, pw); NAME |2 ---------------- knut |5de26b791b bryan|f4a77c:MD5 dag |a2:SHA-512 3 rows selected Here, the user 'knut' has a password that's hashed with the old scheme (there's no algorithm name in the suffix), 'bryan' has a password hashed with MD5, and 'dag' with SHA-512. I don't think we currently have any way of listing all database properties, so the DBA must know all users up front in order to perform this query. It might be useful (in a separate JIRA) to add a new diagnostic table function to improve the monitoring capabilities. Then we could simply have done something like this: select * from table ( syscs_diag.database_properties('derby.user.%') ) p;
          Hide
          Bryan Pendleton added a comment -

          Knut Anders, thanks very much for the detailed writeup. It will be very helpful to people studying your proposal.

          > This means that a database may contain passwords that are hashed using different algorithms.

          Can the Derby administrator or DBA tell that they are in this state? Is there a way to know
          which user/passwords are hashed with which algorithm, during a situation in which my database has
          such a mixture?

          Show
          Bryan Pendleton added a comment - Knut Anders, thanks very much for the detailed writeup. It will be very helpful to people studying your proposal. > This means that a database may contain passwords that are hashed using different algorithms. Can the Derby administrator or DBA tell that they are in this state? Is there a way to know which user/passwords are hashed with which algorithm, during a situation in which my database has such a mixture?
          Hide
          Knut Anders Hatlen added a comment -

          One note about testing:

          There are no tests in experiment.diff. There will of course have to be tests in the final patch. In addition to tests of the basic functionality, there should also be upgrade tests to verify that it works as expected in full and soft upgrade, as well as that you can move back to older versions after a soft upgrade. I don't know if the upgrade tests currently support tests that use authentication. I will have to look at that.

          I did however run derbyall and suites.All with the patch and saw no failures (as expected since the old code path was still used in all tests).

          I also ran derbyall and suites.All with a variant of the patch that hard-coded the use of the new scheme with the SHA-256 algorithm. Only one test failed, NSSecurityMechanismTest. This was an expected failure because of the incompatibility with the strong password substitution mechanism mentioned in my previous comment.

          Show
          Knut Anders Hatlen added a comment - One note about testing: There are no tests in experiment.diff. There will of course have to be tests in the final patch. In addition to tests of the basic functionality, there should also be upgrade tests to verify that it works as expected in full and soft upgrade, as well as that you can move back to older versions after a soft upgrade. I don't know if the upgrade tests currently support tests that use authentication. I will have to look at that. I did however run derbyall and suites.All with the patch and saw no failures (as expected since the old code path was still used in all tests). I also ran derbyall and suites.All with a variant of the patch that hard-coded the use of the new scheme with the SHA-256 algorithm. Only one test failed, NSSecurityMechanismTest. This was an expected failure because of the incompatibility with the strong password substitution mechanism mentioned in my previous comment.
          Hide
          Knut Anders Hatlen added a comment -

          I've experimented with some code changes, see the attached patch
          experiment.diff, to see if it would work to implement this along the
          lines suggested above. Here's a description of how the patch works and
          the behaviour we will get if we go for that approach:

          • API and behaviour *

          A new property called derby.authentication.builtin.algorithm is
          introduced. This is a dynamic property that can be set at database or
          system level. The property names a digest/hash algorithm that must be
          supported by a security provider registered in the JVM.

          If this property is set to a non-empty string, calls to
          SYSCS_UTIL.SYSCS_SET_DATABASE_PROPERTY that change a password (that
          is, set a derby.user.* property) will use the algorithm specified by
          the property to hash the password before storing it in the
          database. Otherwise, the old algorithm (based on SHA-1) is used.

          Since the hashing happens when a derby.user.* property is set, a
          change in which algorithm to use will only take effect on passwords
          that are changed after the algorithm was changed. This means that a
          database may contain passwords that are hashed using different
          algorithms. To update all the stored passwords to use the new
          algorithm, one has to call SYSCS_SET_DATABASE_PROPERTY again on all
          derby.user.* properties after the
          derby.authentication.builtin.algorithm property has been set.

          • Errors *

          If the derby.authentication.builtin.algorithm specifies an algorithm
          that is not known by any of the registered security providers, an
          SQLException with SQLState XJ001 (java exception) will be raised if a
          password is attempted modified later. The SQLException is chained to a
          NoSuchAlgorithmException.

          If a stored password token for a user has been hashed with an
          algorithm that is not available on the current platform (could for
          example happen if the database was created on another platform with a
          different set of security providers), an SQLException with SQLState
          XJ001 (java exception) will be raised when validating the credentials
          for that user. The SQLException is chained to a
          NoSuchAlgorithmException.

          We may want more specific messages in these situations. Currently, the
          message looks like this if you specify a non-existing algorithm, for
          example DOES-NOT-EXIST:

          ij> call syscs_util.syscs_set_database_property('derby.user.kah', 'abc');
          ERROR XJ001: Java exception: 'java.security.NoSuchAlgorithmException: DOES-NOT-EXIST MessageDigest not available'.

          • Compatibility *

          If derby.authentication.builtin.algorithm is not set or an empty
          string, there should be no change in behaviour.

          Otherwise, you will get the new behaviour. The new behaviour should be
          mostly transparent to the users. However, you will no longer be able
          to connect from the client driver with securityMechanism=8 (strong
          password substitution). This is because the strong password
          substitution security mechanism needs to use the same algorithm itself
          to produce a password token that can be compared against the token
          stored in the database.

          Adding support for this on the client is not straightforward, since
          the client would need to be told by the server which algorithm to use,
          and this would probably require a protocol extension. It is further
          complicated by the fact that the passwords can be stored with
          different hash algorithms for different users. Anyways, that's for
          another issue to address, if someone wants that functionality.

          • Upgrade *

          Since earlier versions of Derby don't understand the new format of the
          stored password tokens, derby.authentication.builtin.algorithm is
          ignored if the data dictionary is not at version 10.6 or higher. The
          old algorithm will be used instead in that case. This allows you to
          change the password of a user while running in soft upgrade mode and
          still be able to log in as that user if you move back to an older
          version of Derby.

          If the database has been upgraded (with a full upgrade) so that the
          data dictionary is at version 10.6 or later, or if the database was
          created with Derby version 10.6 or later, setting the
          derby.authentication.builtin.algorithm will modify the behaviour as
          described in the sections above.

          • Implementation *

          The code changes are limited to two classes (with the exception of the
          definition of a constant in a third one).

          • AuthenticationServiceBase.java:

          The method map(), which does the mapping from plaintext password to
          hashed password, is extended with logic to check the
          derby.authentication.builtin.algorithm property and the data
          dictionary version in order to choose which algorithm to use.

          • BasicAuthenticationServiceImpl.java:

          The method authenticateUser() is extended with logic so that it
          understands which algorithm has been used to hash a stored
          password. It applies the same algorithm on the user-supplied password
          and compares it with the one stored in the database.

          The existing code in these two classes already had a mechanism to
          distinguish between different hashing approaches. It did this by
          prefixing the stored hash with an identifier telling which approach
          was used. The code in the patch piggybacked on that mechanism and
          prefixed the tokens with a different identifier. Also, the name of the
          algorithm (for instance SHA-256 or MD5) is appended to the token, with
          a colon separating the hash and the name of the algorithm.

          One somewhat confusing detail is that the old authentication scheme is
          called the new authentication scheme, both in comments and names of
          constants. We may want to rename it to something that's more neutral
          to where it is on the evolutionary path. SHA-1 authentication scheme,
          perhaps.

          For the new authentication scheme implemented here, I tentatively
          picked the name "configurable hash authentication scheme".

          • Internal differences between the schemes *

          There are some differences in how the hashes/digests are generated in
          the two schemes, apart from the obvious that the old scheme always
          uses SHA-1 and the new scheme can use a variety of algorithms.

          • Translating passwords into byte arrays

          The old scheme uses StringUtil.toHexByte() to convert the password to
          a byte array. This code however assumes that all characters are within
          the ISO-8859-1 range by only looking at the eight lower bits. It
          doesn't break when characters outside this range is used, but it does
          not give you any benefit from the extra bits/bytes you've provided
          either. Therefore, I used String.getBytes("UTF-8") instead, which is
          simpler and uses all bits regardless of which Unicode characters you
          have in your password.

          • Salting the hash

          The old scheme created a hash from the password alone. This means that
          if two users have the same password, they will have the same hashed
          password. In the new scheme, the user name and the password are
          concatenated before applying the hash function. This means that two
          users with the same password will (most likely) not have the same
          hashed password, which makes a dictionary attack more time
          consuming. Although it's not strictly necessary within the context of
          this issue, it sounded like a cheap way to enhance the security while
          we're first at it.

          For those interested, I found this blog entry well-written and an easy
          to understand introduction to the technique of salting password
          hashes:
          http://pentesterconfessions.blogspot.com/2008/05/secure-salt-for-tastey-hashes.html

          (The current scheme in Derby is similar to scenario 1, described in
          that entry, and the proposed new scheme is similar to scenario 2.)

          Comments and suggestions to improve this proposal are welcome!
          Thanks.

          Show
          Knut Anders Hatlen added a comment - I've experimented with some code changes, see the attached patch experiment.diff, to see if it would work to implement this along the lines suggested above. Here's a description of how the patch works and the behaviour we will get if we go for that approach: API and behaviour * A new property called derby.authentication.builtin.algorithm is introduced. This is a dynamic property that can be set at database or system level. The property names a digest/hash algorithm that must be supported by a security provider registered in the JVM. If this property is set to a non-empty string, calls to SYSCS_UTIL.SYSCS_SET_DATABASE_PROPERTY that change a password (that is, set a derby.user.* property) will use the algorithm specified by the property to hash the password before storing it in the database. Otherwise, the old algorithm (based on SHA-1) is used. Since the hashing happens when a derby.user.* property is set, a change in which algorithm to use will only take effect on passwords that are changed after the algorithm was changed. This means that a database may contain passwords that are hashed using different algorithms. To update all the stored passwords to use the new algorithm, one has to call SYSCS_SET_DATABASE_PROPERTY again on all derby.user.* properties after the derby.authentication.builtin.algorithm property has been set. Errors * If the derby.authentication.builtin.algorithm specifies an algorithm that is not known by any of the registered security providers, an SQLException with SQLState XJ001 (java exception) will be raised if a password is attempted modified later. The SQLException is chained to a NoSuchAlgorithmException. If a stored password token for a user has been hashed with an algorithm that is not available on the current platform (could for example happen if the database was created on another platform with a different set of security providers), an SQLException with SQLState XJ001 (java exception) will be raised when validating the credentials for that user. The SQLException is chained to a NoSuchAlgorithmException. We may want more specific messages in these situations. Currently, the message looks like this if you specify a non-existing algorithm, for example DOES-NOT-EXIST: ij> call syscs_util.syscs_set_database_property('derby.user.kah', 'abc'); ERROR XJ001: Java exception: 'java.security.NoSuchAlgorithmException: DOES-NOT-EXIST MessageDigest not available'. Compatibility * If derby.authentication.builtin.algorithm is not set or an empty string, there should be no change in behaviour. Otherwise, you will get the new behaviour. The new behaviour should be mostly transparent to the users. However, you will no longer be able to connect from the client driver with securityMechanism=8 (strong password substitution). This is because the strong password substitution security mechanism needs to use the same algorithm itself to produce a password token that can be compared against the token stored in the database. Adding support for this on the client is not straightforward, since the client would need to be told by the server which algorithm to use, and this would probably require a protocol extension. It is further complicated by the fact that the passwords can be stored with different hash algorithms for different users. Anyways, that's for another issue to address, if someone wants that functionality. Upgrade * Since earlier versions of Derby don't understand the new format of the stored password tokens, derby.authentication.builtin.algorithm is ignored if the data dictionary is not at version 10.6 or higher. The old algorithm will be used instead in that case. This allows you to change the password of a user while running in soft upgrade mode and still be able to log in as that user if you move back to an older version of Derby. If the database has been upgraded (with a full upgrade) so that the data dictionary is at version 10.6 or later, or if the database was created with Derby version 10.6 or later, setting the derby.authentication.builtin.algorithm will modify the behaviour as described in the sections above. Implementation * The code changes are limited to two classes (with the exception of the definition of a constant in a third one). AuthenticationServiceBase.java: The method map(), which does the mapping from plaintext password to hashed password, is extended with logic to check the derby.authentication.builtin.algorithm property and the data dictionary version in order to choose which algorithm to use. BasicAuthenticationServiceImpl.java: The method authenticateUser() is extended with logic so that it understands which algorithm has been used to hash a stored password. It applies the same algorithm on the user-supplied password and compares it with the one stored in the database. The existing code in these two classes already had a mechanism to distinguish between different hashing approaches. It did this by prefixing the stored hash with an identifier telling which approach was used. The code in the patch piggybacked on that mechanism and prefixed the tokens with a different identifier. Also, the name of the algorithm (for instance SHA-256 or MD5) is appended to the token, with a colon separating the hash and the name of the algorithm. One somewhat confusing detail is that the old authentication scheme is called the new authentication scheme, both in comments and names of constants. We may want to rename it to something that's more neutral to where it is on the evolutionary path. SHA-1 authentication scheme, perhaps. For the new authentication scheme implemented here, I tentatively picked the name "configurable hash authentication scheme". Internal differences between the schemes * There are some differences in how the hashes/digests are generated in the two schemes, apart from the obvious that the old scheme always uses SHA-1 and the new scheme can use a variety of algorithms. Translating passwords into byte arrays The old scheme uses StringUtil.toHexByte() to convert the password to a byte array. This code however assumes that all characters are within the ISO-8859-1 range by only looking at the eight lower bits. It doesn't break when characters outside this range is used, but it does not give you any benefit from the extra bits/bytes you've provided either. Therefore, I used String.getBytes("UTF-8") instead, which is simpler and uses all bits regardless of which Unicode characters you have in your password. Salting the hash The old scheme created a hash from the password alone. This means that if two users have the same password, they will have the same hashed password. In the new scheme, the user name and the password are concatenated before applying the hash function. This means that two users with the same password will (most likely) not have the same hashed password, which makes a dictionary attack more time consuming. Although it's not strictly necessary within the context of this issue, it sounded like a cheap way to enhance the security while we're first at it. For those interested, I found this blog entry well-written and an easy to understand introduction to the technique of salting password hashes: http://pentesterconfessions.blogspot.com/2008/05/secure-salt-for-tastey-hashes.html (The current scheme in Derby is similar to scenario 1, described in that entry, and the proposed new scheme is similar to scenario 2.) Comments and suggestions to improve this proposal are welcome! Thanks.
          Hide
          Francois Orsini added a comment -

          Sounds like a great approach, Knut - that should work indeed.

          Show
          Francois Orsini added a comment - Sounds like a great approach, Knut - that should work indeed.
          Hide
          Knut Anders Hatlen added a comment -

          One possibility for allowing changing hash algorithm at runtime, is to store the name of the algorithm along with the hash. Something like

          sha256:ad6f438f529e021f62b3c97be2b34c3b73dd444334de08fcaa7f54fd302fb92d
          md5:3865ab6a65c2d5a3be1733b4408d7b74
          ...

          Then, when you change the algorithm, you'll only use the new algorithm for passwords created/updated after the change, and the authentication service would know which hash algorithm to use for each user. If no algorithm name is specified, we could fall back to the SHA-1 algorithm to stay compatible with old databases.

          Show
          Knut Anders Hatlen added a comment - One possibility for allowing changing hash algorithm at runtime, is to store the name of the algorithm along with the hash. Something like sha256:ad6f438f529e021f62b3c97be2b34c3b73dd444334de08fcaa7f54fd302fb92d md5:3865ab6a65c2d5a3be1733b4408d7b74 ... Then, when you change the algorithm, you'll only use the new algorithm for passwords created/updated after the change, and the authentication service would know which hash algorithm to use for each user. If no algorithm name is specified, we could fall back to the SHA-1 algorithm to stay compatible with old databases.
          Hide
          Francois Orsini added a comment -

          Note: Allowing to set a different hash algorithm would have to be done at the time the database is created when the hash password is stored in the database (storing the user property) as otherwise during runtime, one would not be able to compare some hash produced by the new configured hash algorithm versus the stored hash value which would have been generated using some previous and different algorithm.

          Show
          Francois Orsini added a comment - Note: Allowing to set a different hash algorithm would have to be done at the time the database is created when the hash password is stored in the database (storing the user property) as otherwise during runtime, one would not be able to compare some hash produced by the new configured hash algorithm versus the stored hash value which would have been generated using some previous and different algorithm.

            People

            • Assignee:
              Knut Anders Hatlen
              Reporter:
              Knut Anders Hatlen
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development