1

I have a Java code where I created a SSLSocket. My design looks like this:

Super class: Server.java. Member variable: socket of type Socket.
My derived class: TLSServer.java. Member variables - SSLSocket sslSock;
                                                     SSLServerSocket sslSrvrSock;


sslSock = sslSrvrSock.accept();

Now, we have a code like this:

processData(Socket sock)
{
   read from socket
   close socket
}

Server server;
This is how we call processData - processData(server.sock);

Now the object server of type Server itself could be either Server or TLSServer. If TLSServer, we still pass it as an object of type Socket and not SSLSocket.

Now, when we finally close the socket, how do I ensure that SSL functionality is actually shut down for this socket? In C++ openssl lib, I can do: SSL_shutdown()

Here, since I am passing an object that is of type SSLSocket to a function that accepts Socket, I suspect that SSL connection is not getting cleaned up during shutdown. Only the underlying TCP layer is shutting down.

Pls suggest. Also, is there a way for me to verify via Java code or any other means if a SSL shutdown has happened for a given socket?

I have an app that is talking to this server which is not getting the close notify when the socket closes. Which makes me suspect that SSL shutdown is not happening.

Thanks, Om.

Omi
  • 976
  • 2
  • 20
  • 35

1 Answers1

0

;Here, since I am passing an object that is of type SSLSocket to a function that accepts Socket, I suspect that SSL connection is not getting cleaned up during shutdown. Only the underlying TCP layer is shutting down.

This doesn't make much sense - both concepts (SSLSocket and non-SSLed sockets) are resources that require close()ing.

There are 2 options:

  • The code you are calling into does not do it right. Which generally means: Does not use try-with-resources. In which case that code is broken. SSL has nothing to do with it - failure to close() non-SSL sockets safely is just as much of an issue.
  • The code you are calling into does it right. In which case, the fact that it's an SSLSocket instance is irrelevant: It'll be cleaned up appropriately.

Also, is there a way for me to verify via Java code or any other means if a SSL shutdown has happened for a given socket?

Check the code. Is it safe-closing? There aren't any runtime hooks I'm aware of that you can add.

You have a style issue!

processData(Socket sock)
{
   read from socket
   close socket
}

This isn't normal. As in, it's not a bug directly nor is it invalid java code but this runs counter to how the community expects resource-using code to operate.

Normally, whatever makes the resource is also responsible for closing the resource. It is abnormal for a thing that makes a resource not to also close it, and it is similarly abnormal for a thing that is handed_ a resource (vs. creating it itself) to close it.

These rules are the obvious result of applying a bunch of axioms and obvious maxims:

  • Resources must be managed, as in, once created they must be closed or that JVM starts 'leaking resources' (not so much a memory leak, but more e.g. that you run out of file handles and therefore the JVM can't do much until it is shut down entirely). The garbage collector cannot be used as a fallback for resource management, because there is no guarantee the garbage collector is going to run in a timely fashion. It could (and this really happens) take days for the GC to kick in.

  • Given that they must be managed, making multiple unrelated snippets of code jointly responsible for managing it does not work - shared responsibility, is no responsibility. Or, everybody tries to program safely and closes everything in sight which means one snippet makes the work of another impossible to closing a resource it needs 'out from under its feet'. The solution is simple and obvious: Make sure only one snippet of code is universally agreed upon as being responsible for the resource. That code should ensure it is closed. Other code should ensure it does not close it.

This then simplifies to: So.. which code is responsible?

And that leads to: ... the one that made the resource, unless explicitly documented otherwise. This documentation is stated in terms of the passing of the responsibility. You can have a thing that makes a resource and fails to close it, as long as it documents that responsibility 'passes' to the caller of it. You can have a thing that takes a resource from outside of itself and closes it, as long as it documents that it takes away the responsibility from the caller (and, hence, that the caller must not also close it and must cease interacting with the resource after passing it).

Rust applies this principle as a compiler checked concept with all objects (due to not having a garbage collector, pretty much all objects in rust are 'resources that must be closed') - so if you're familiar with that language, you do the same thing in java, only you limit it to objects-representing-system-resources and you don't get the rust compiler's enforcement of responsibility and all that entails (such as refusing to compile code that interacts with an object after handing it over to another along with the responsibility for that object).

An example of a method that 'passes responsibility' is new FileInputStream() (which makes a resource but obviously doesn't also close it, as that would make the class useless), and Files.newInputStream. By convention, you should either use a constructor, or include new in the method's name. Older API is bad and breaks this rule (back then 'resource management is kinda hard' was less well known), such as Socket's .getInputStream() which should have been called .newInputStream(). Because it makes a resource and passes responsibility for it to the caller.

Your method here is a case of the reverse: It does not make a resource but nevertheless does 'take it over' and takes on the responsibility of closing it. As far as I'm aware there is no convention for this, so you need to go out of your way to document it. Or, preferably, rewrite this method. It should do one of three things:

  • Not close the resource and leave the responsibility with the caller (or rather, the maker of that resource).
  • If it is a long-lived thing where you end up storing this resource in a field or passing it to a thread/runnable, the thing that takes a resource should itself become a resource, and implement AutoClosable to signal this. By convention, passing resources to AutoClosables inherently passes responsibility. See: Scanner, BufferedInputStream, and every other filter stream/reader/writer in the core java libraries. Then the caller gets to 'transfer' the responsibility from 'close this socket' to 'close this SocketProcessor', which is a common occurrence in java.
  • Should document extremely carefully because you're off the common path here that this method adopts responsibility.

Given that it's so 'weird' and given that it sounds in your question like you don't directly control this code, you should assume that it does the common thing and therefore _does not close the resource, intentionally.

Safely closing resources

This is how you safely close resources:

try (Resource r = makeAndTakeResponsibilityForTheResource()) {
  // use r here
}

That's it. It's as simple as that: The compiler and runtime work together to ensure that no matter what happens, if code 'flows' out of this block the resource is closed. Whether naturally (runs to the closing brace), by control flow (return;, break;, etcetera), or by exception. Here is an example with sockets:

try (Socket s = serverSocket.accept()) {
  // do stuff with s
}
// s as a variable doesn't even exist anymore here, which is nice

In java code over 20 years old, this would have to look like the unwieldy:

Socket s = ...;
try {
  // use s here
} finally {
  s.close();
}

The try-with construct desugars to the above - bad decompilers will likely turn the first snippet into the second one, in case you're desperate enough to decompile classes to figure out how they work and if they work as you expected them to.

If you use your resources as follows:

  • "A resource" is any object that is instanceof AutoClosable.
  • Resource makers are either constructors, or methods that include the name new, or create them using try ( ... ) {}. And you are aware invoking a method whose name includes new or constructor that returns a resource means you are the 'resource maker' - you inherit the responsibility.
  • Objects that require a resource statefully (cannot be fully done with it within the confines of a single method call) are turned into resources themselves (give em a close() method and stick implements AutoClosable on it). Presume passing a resource to a thing that makes a resource transfers responsibility (e.g. new BufferedInputStream(someStream) transfers).
  • Do NOT close resources outside of these rules. Don't close resources you didn't make 'just in case', don't add finalizer() methods that close resources.

It can't go wrong. Hence, do it that way - it's convention, and that convention is good.

Checking it

linter tools (tools that check your source code as you develop - sometimes more fancifully called 'static code analysers') exist that check proper application of the above rules. Apply them to your codebase if you want to know.

There is, as far as I know, no real runtime equivalent. You could of course write AOP or agents or other JVM plugins that keep track of the creation of resources and simply start some timers to eventually start saying: Hmmm, that resource has been open for a very very long time. Bug? - or if you want to get real fancy, figure out a way to runtime-confirm the above behaviour.

That is very complicated (person-month+) and as far as I know, nobody is doing this. I guess folks trust the linters.

rzwitserloot
  • 85,357
  • 5
  • 51
  • 72