Derby
  1. Derby
  2. DERBY-2356

Make SSL server authentication optional

    Details

    • Bug behavior facts:
      Security

      Description

      Default SSL behaviour is to require serer authentication. For a database application this is not as important as it is for web browsers and also creates som extra hassle for the user/application programmer. Since the main objective for SSL in Derby is encryption on the wire, server authentication should be optional (the same way client authentication is).

      This also creates some symmetry which can be exploited to simplify the user interfce somewhat. This improvement to DERBY-2108 is described in the attached functional specification. See the attachment for details.

      1. derby-2356-v1.diff
        21 kB
        Bernt M. Johnsen
      2. derby-2356-v1.stat
        0.7 kB
        Bernt M. Johnsen
      3. derby-2356-v2.diff
        23 kB
        Bernt M. Johnsen
      4. derby-2356-v2.stat
        0.7 kB
        Bernt M. Johnsen
      5. derby-2356-v3.diff
        23 kB
        Bernt M. Johnsen
      6. SSLFuncSpect.txt
        6 kB
        Bernt M. Johnsen
      7. SSLFuncSpect.txt
        5 kB
        Bernt M. Johnsen

        Issue Links

          Activity

          Hide
          Bernt M. Johnsen added a comment -

          Uploaded patch for this improvement. Comments are welcome

          Show
          Bernt M. Johnsen added a comment - Uploaded patch for this improvement. Comments are welcome
          Hide
          Kristian Waagan added a comment -

          Just a few quick comments!

          1) Typo in Property.java
          [drda.NaiveTrustManager]
          2) Strange sentence in JavaDoc for getSocketFactory()
          3) No JavaDoc for the other methods. Are these documented elsewhere? For instance, what form/value is authType supposed to have?
          4) Could getAcceptedIssuers() return an empty array instead of null?
          [NetworkServerControlImpl]
          5) I guess SSL_OFF and default are the same for now. It would be nice with a comment to document the fall-through.
          6) Long line/missing newline in getSSLModeValue
          [net.NaiveTrustManager]
          7) See items 2-4.
          [NetConnection]
          8) Several long lines. Was long before, but maybe it is possible to break some of them?
          [OpenSocketAction]
          9) Document fall-through in switch?

          There also seems to be a mix of tabs and spaces in the changed code. Even though the code already contains both, it would be nice to stick to one alternative when adding/changing lines.

          I'll give the functionality a try tomorrow!
          Thanks,

          Show
          Kristian Waagan added a comment - Just a few quick comments! 1) Typo in Property.java [drda.NaiveTrustManager] 2) Strange sentence in JavaDoc for getSocketFactory() 3) No JavaDoc for the other methods. Are these documented elsewhere? For instance, what form/value is authType supposed to have? 4) Could getAcceptedIssuers() return an empty array instead of null? [NetworkServerControlImpl] 5) I guess SSL_OFF and default are the same for now. It would be nice with a comment to document the fall-through. 6) Long line/missing newline in getSSLModeValue [net.NaiveTrustManager] 7) See items 2-4. [NetConnection] 8) Several long lines. Was long before, but maybe it is possible to break some of them? [OpenSocketAction] 9) Document fall-through in switch? There also seems to be a mix of tabs and spaces in the changed code. Even though the code already contains both, it would be nice to stick to one alternative when adding/changing lines. I'll give the functionality a try tomorrow! Thanks,
          Hide
          John H. Embretsen added a comment -

          A few comments after my first take at trying out the (v1) patch:

          (I have only tried ssl=basic so far...)

          1) No server commands (e.g. shutdown, ping, runtimeinfo) worked after the server was started with SSL on (basic) . The message I'm getting is:

          Invalid reply header from network server: Invalid string .

          2) Using -Dderby.drda.sslMode=basic (and ssl=basic in the client URL) seemed to work fine, although I did not actually inspect the network traffic to verify encryption.

          3) Using ssl=basic as an option to the NetworkServerControl start command did not work:

          Command line: java <properties> -jar derbyrun.jar server start ssl=basic
          Result: Invalid number of arguments for command start.

          Command line: java <properties> -jar derbyrun.jar server start -ssl=basic
          Result: Argument -ssl=basic is unknown.

          I tried both with and without the -unsecure option/plain-text authentication.

          4) The funcSpec says:

          SSL at the server side is activated with the property
          derby.drda.sslMode (default off) or the -ssl option for the server
          command.

          By "the server command", do you mean the start command of the server? This should perhaps be clarified in the funcSpec?

          5) The funcSpec also says:

          The property may have three values: "off", "basic" and
          "peerAuthentication"

          However, the example in section 2.3 is using ssl=authenticate. Also, comments in the patch seem to indicate that "false", "true" and "auth" are also valid property values. What is (or should be) the correct set of valid values?

          6) I verified that connection attempts against a server started with SSL off, but with ssl=basic in the client URL, resulted in an informative error message on the client side.

          Show
          John H. Embretsen added a comment - A few comments after my first take at trying out the (v1) patch: (I have only tried ssl=basic so far...) 1) No server commands (e.g. shutdown, ping, runtimeinfo) worked after the server was started with SSL on (basic) . The message I'm getting is: Invalid reply header from network server: Invalid string . 2) Using -Dderby.drda.sslMode=basic (and ssl=basic in the client URL) seemed to work fine, although I did not actually inspect the network traffic to verify encryption. 3) Using ssl=basic as an option to the NetworkServerControl start command did not work: Command line: java <properties> -jar derbyrun.jar server start ssl=basic Result: Invalid number of arguments for command start. Command line: java <properties> -jar derbyrun.jar server start -ssl=basic Result: Argument -ssl=basic is unknown. I tried both with and without the -unsecure option/plain-text authentication. 4) The funcSpec says: SSL at the server side is activated with the property derby.drda.sslMode (default off) or the -ssl option for the server command. By "the server command", do you mean the start command of the server? This should perhaps be clarified in the funcSpec? 5) The funcSpec also says: The property may have three values: "off", "basic" and "peerAuthentication" However, the example in section 2.3 is using ssl=authenticate. Also, comments in the patch seem to indicate that "false", "true" and "auth" are also valid property values. What is (or should be) the correct set of valid values? 6) I verified that connection attempts against a server started with SSL off, but with ssl=basic in the client URL, resulted in an informative error message on the client side.
          Hide
          Bernt M. Johnsen added a comment -

          Thanks John!
          1) You need -ssl basic on these too, since they are clients. My mistake, it's not mentioned in the func spec, but it is implemented. This is also another argument for DERBY-2362.

          3) This is in line with the other options, e.g.

          java -jar derbyrun.jar server start p=1234
          Invalid number of arguments for command start.

          or

          java -jar derbyrun.jar server start -p=1234
          Argument -p=1234 is unknown.

          4) I will clarify this

          5) Typo. I will fix them (some are already fixed in my sandbox)

          Show
          Bernt M. Johnsen added a comment - Thanks John! 1) You need -ssl basic on these too, since they are clients. My mistake, it's not mentioned in the func spec, but it is implemented. This is also another argument for DERBY-2362 . 3) This is in line with the other options, e.g. java -jar derbyrun.jar server start p=1234 Invalid number of arguments for command start. or java -jar derbyrun.jar server start -p=1234 Argument -p=1234 is unknown. 4) I will clarify this 5) Typo. I will fix them (some are already fixed in my sandbox)
          Hide
          John H. Embretsen added a comment -

          Yes, using the -ssl basic option for the other NetworkServerControl commands as well worked just great, makes sense. So 4) would essentially be clear enough by just appending an "s" to "the server command"

          Regarding 3), I only followed the examples in the funcSpec, thinking this was somehow implemented differently. The regular variant
          <command> -ssl <sslMode> [otherOptions]
          works just fine.

          Show
          John H. Embretsen added a comment - Yes, using the -ssl basic option for the other NetworkServerControl commands as well worked just great, makes sense. So 4) would essentially be clear enough by just appending an "s" to "the server command" Regarding 3), I only followed the examples in the funcSpec, thinking this was somehow implemented differently. The regular variant <command> -ssl <sslMode> [otherOptions] works just fine.
          Hide
          Bernt M. Johnsen added a comment -

          Uploaded new funcspec and V2 of the patch. Thanks to John and Kristian for their comments.

          Show
          Bernt M. Johnsen added a comment - Uploaded new funcspec and V2 of the patch. Thanks to John and Kristian for their comments.
          Hide
          Bernt M. Johnsen added a comment -

          Uploaded v3. Small change from v2: Avoid creating a new TrustManager for each connection.

          Show
          Bernt M. Johnsen added a comment - Uploaded v3. Small change from v2: Avoid creating a new TrustManager for each connection.
          Hide
          Bernt M. Johnsen added a comment -

          Committed revision 511785.

          Show
          Bernt M. Johnsen added a comment - Committed revision 511785.

            People

            • Assignee:
              Bernt M. Johnsen
              Reporter:
              Bernt M. Johnsen
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development