Issue Details (XML | Word | Printable)

Key: DERBY-2356
Type: Improvement Improvement
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Bernt M. Johnsen
Reporter: Bernt M. Johnsen
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
Derby

Make SSL server authentication optional

Created: 19/Feb/07 02:21 PM   Updated: 30/Jun/09 12:14 AM
Return to search
Component/s: Network Client, Network Server
Affects Version/s: 10.3.1.4
Fix Version/s: 10.3.1.4

Time Tracking:
Not Specified

File Attachments:
  Size
File Licensed for inclusion in ASF works derby-2356-v1.diff 2007-02-20 01:21 PM Bernt M. Johnsen 21 kB
File Licensed for inclusion in ASF works derby-2356-v1.stat 2007-02-20 01:21 PM Bernt M. Johnsen 0.7 kB
File Licensed for inclusion in ASF works derby-2356-v2.diff 2007-02-23 12:40 PM Bernt M. Johnsen 23 kB
File Licensed for inclusion in ASF works derby-2356-v2.stat 2007-02-23 12:40 PM Bernt M. Johnsen 0.7 kB
File Licensed for inclusion in ASF works derby-2356-v3.diff 2007-02-26 09:23 AM Bernt M. Johnsen 23 kB
Text File Licensed for inclusion in ASF works SSLFuncSpect.txt 2007-02-23 12:40 PM Bernt M. Johnsen 6 kB
Text File Licensed for inclusion in ASF works SSLFuncSpect.txt 2007-02-19 02:21 PM Bernt M. Johnsen 5 kB
Issue Links:
Reference

Bug behavior facts: Security
Resolution Date: 26/Feb/07 07:05 PM


 Description  « Hide
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.

 All   Comments   Work Log   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Bernt M. Johnsen added a comment - 20/Feb/07 01:21 PM
Uploaded patch for this improvement. Comments are welcome

Kristian Waagan added a comment - 20/Feb/07 05:54 PM
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,

John H. Embretsen added a comment - 22/Feb/07 12:13 PM
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.


Bernt M. Johnsen added a comment - 22/Feb/07 08:56 PM
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)


John H. Embretsen added a comment - 23/Feb/07 07:19 AM
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.

Bernt M. Johnsen added a comment - 23/Feb/07 12:40 PM
Uploaded new funcspec and V2 of the patch. Thanks to John and Kristian for their comments.

Bernt M. Johnsen added a comment - 26/Feb/07 09:23 AM
Uploaded v3. Small change from v2: Avoid creating a new TrustManager for each connection.

Bernt M. Johnsen added a comment - 26/Feb/07 07:05 PM
Committed revision 511785.