From eb4a9355625105d1f3ad6e5d15552a97f5d20559 Mon Sep 17 00:00:00 2001 From: wcrisman Date: Mon, 29 Dec 2014 11:57:32 -0800 Subject: [PATCH] Changed key.channel().close() into just calling close on the SocketContext throughout the code. Comment changes and debugging additions and changes. --- .../web/server/PassThroughSocketContext.java | 39 +++--- .../foundation/web/server/SocketContext.java | 113 +++++++++--------- 2 files changed, 83 insertions(+), 69 deletions(-) diff --git a/Foundation Web Core/src/com/foundation/web/server/PassThroughSocketContext.java b/Foundation Web Core/src/com/foundation/web/server/PassThroughSocketContext.java index fec88a6..fb48689 100644 --- a/Foundation Web Core/src/com/foundation/web/server/PassThroughSocketContext.java +++ b/Foundation Web Core/src/com/foundation/web/server/PassThroughSocketContext.java @@ -205,24 +205,33 @@ private boolean writeClientBoundMessage(SocketChannel channel, MessageBuffer cur * @see com.foundation.web.server.WebServer.AbstractSocketContext#passThrough(java.nio.ByteBuffer) */ protected void passThrough(ByteBuffer buffer) { - synchronized(getLock()) { - ByteBuffer messageBytes = ByteBuffer.allocate(buffer.remaining()); - MessageBuffer message; + if(buffer != null) { + boolean notify = false; - //Create a new buffer to hold the data so we don't modify the passed buffer (other than to update its position).// - messageBytes = ByteBuffer.allocate(buffer.remaining()); - messageBytes.put(buffer); - message = new MessageBuffer(this, messageBytes); + synchronized(getLock()) { + ByteBuffer messageBytes = ByteBuffer.allocate(buffer.remaining()); + MessageBuffer message; + + //Create a new buffer to hold the data so we don't modify the passed buffer (other than to update its position).// + messageBytes = ByteBuffer.allocate(buffer.remaining()); + messageBytes.put(buffer); + message = new MessageBuffer(this, messageBytes); + + //Chain the message into the linked list. + if(lastOutboundMessage == null) { + currentOutboundMessage = lastOutboundMessage = message; + notify = true; + }//if// + else { + lastOutboundMessage.setNext(message); + lastOutboundMessage = message; + }//else// + }//synchronized// - //Chain the message into the linked list. - if(lastOutboundMessage == null) { - currentOutboundMessage = lastOutboundMessage = message; + if(notify) { + notifyListenerOfPendingWrite(); }//if// - else { - lastOutboundMessage.setNext(message); - lastOutboundMessage = message; - }//else// - }//synchronized// + }//if// }//passThrough()// protected synchronized void close() { synchronized(getLock()) { diff --git a/Foundation Web Core/src/com/foundation/web/server/SocketContext.java b/Foundation Web Core/src/com/foundation/web/server/SocketContext.java index b834a07..ef9cdf2 100644 --- a/Foundation Web Core/src/com/foundation/web/server/SocketContext.java +++ b/Foundation Web Core/src/com/foundation/web/server/SocketContext.java @@ -274,12 +274,10 @@ private void queueOutboundClientMessage(MessageBuffer messageBuffer) { }//queueOutboundClientMessage()// /** * Adds a HTTP response to the socket context. - *

Note: We must synchronize since a socket could be used to access multiple applications and thus mutliple sessions.

+ *

Note that this just queues a response. The actual sending is initiated by the web server's NetworkListener when the socket is flagged as being ready to write.

* @param response The response to be added. - * @result Whether request is in a receive state. Will be false if the request generated a response that could not be completely transmitted. */ public void sendHttpResponse(Response response) { - //prepareResponse(response); queueOutboundClientMessage(new HttpMessageBuffer(this, response)); request = null; }//sendHttpResponse()// @@ -298,7 +296,7 @@ protected void passThrough(ByteBuffer buffer) { queueOutboundClientMessage(message); }//passThrough()// /* (non-Javadoc) - * @see com.foundation.web.server.WebServer.AbstractSocketContext#processResponses() + * @see com.foundation.web.server.WebServer.AbstractSocketContext#writeOutgoingMessages() */ protected void writeOutgoingMessages() throws IOException { boolean keepSending; @@ -364,8 +362,10 @@ protected void writeOutgoingMessages() throws IOException { }//if// }//writeOutgoingMessages()// /** - * Sends a response to the client. - * @return Whether the response could be fully sent. This will be false if there is still more data to be written when the call returns. + * Sends a message/response to the client. + * @param channel The channel to use for the sending. + * @param currentOutboundMessage The message to be sending. This may be null if using SSL and sending handshake data (in which case the encryptedWriteBuffer should have data on it). + * @return Whether the response could be fully sent. This will be false if there is still more data to be written for the current message OR in the encrypted SSL write buffer (for SSL handshake messages) when the call returns. */ private boolean writeClientBoundMessage(SocketChannel channel, MessageBuffer currentOutboundMessage) { boolean sendMore = true; @@ -380,8 +380,10 @@ private boolean writeClientBoundMessage(SocketChannel channel, MessageBuffer cur return sendMore; }//writeClientBoundMessage()// /** - * Sends a response to the client. - * @return Whether the response could be fully sent. This will be false if there is still more data to be written when the call returns. + * Sends a SSL message/response to the client. + * @param channel The channel to use for the sending. + * @param currentOutboundMessage The message to be sending. This may be null if using SSL and sending handshake data (in which case the encryptedWriteBuffer should have data on it). + * @return Whether the response could be fully sent. This will be false if there is still more data to be written for the current message OR in the encrypted SSL write buffer (for SSL handshake messages) when the call returns. */ private boolean writeClientBoundSslMessage(SocketChannel channel, MessageBuffer currentOutboundMessage) { boolean sendMore = true; @@ -408,6 +410,7 @@ private boolean writeClientBoundSslMessage(SocketChannel channel, MessageBuffer }//if// }//if// + //Perform handshaking.// while(sendMore && sslNeedsWrap) { SSLEngineResult handshakeResult; @@ -435,7 +438,7 @@ private boolean writeClientBoundSslMessage(SocketChannel channel, MessageBuffer Debug.log(new RuntimeException("Unexpected ssl engine closed.")); //TODO: Handle this closure without an infinate loop... //Close the socket.// - try {key.channel().close();}catch(Throwable e2) {} + close(); }//else if// else if(handshakeResult.getStatus() == Status.OK) { if(handshakeResult.getHandshakeStatus() == HandshakeStatus.NEED_TASK) { @@ -467,6 +470,7 @@ private boolean writeClientBoundSslMessage(SocketChannel channel, MessageBuffer }//else// }//while// + //If we can send more data and we have an outbound message then initialize and send it! The currentOutboundMessage might be null if performing an SSL handshake.// if(sendMore && currentOutboundMessage != null && !currentOutboundMessage.isClosed()) { //Initialize the outbound message.// if(!currentOutboundMessage.initialize()) { @@ -517,7 +521,7 @@ private boolean writeClientBoundSslMessage(SocketChannel channel, MessageBuffer // Debug.log(new RuntimeException("Unexpected ssl engine closed.")); //TODO: Handle this closure without an infinate loop... //Close the socket.// - try {channel.close();} catch(Throwable e2) {} + close(); }//else if// else if(encryptResult.getStatus() == Status.OK) { //Write the bytes to the stream.// @@ -589,13 +593,16 @@ private boolean writeClientBoundSslMessage(SocketChannel channel, MessageBuffer return sendMore; }//writeClientBoundSslMessage()// /** - * Sends a response to the client. - * @return Whether the response could be fully sent. This will be false if there is still more data to be written when the call returns. + * Sends a plain (non-ssl) message/response to the client. + * @param channel The channel to use for the sending. + * @param currentOutboundMessage The message to be sending. This may be null if using SSL and sending handshake data (in which case the encryptedWriteBuffer should have data on it). + * @return Whether the response could be fully sent. This will be false if there is still more data to be written for the current message OR in the encrypted SSL write buffer (for SSL handshake messages) when the call returns. */ private boolean writeClientBoundPlainMessage(SocketChannel channel, MessageBuffer currentOutboundMessage) { boolean sendMore = true; try { + //If we can send more data and we have an outbound message then initialize and send it! The currentOutboundMessage might be null if performing an SSL handshake.// if(sendMore && currentOutboundMessage != null && !currentOutboundMessage.isClosed()) { //Initialize the outbound message.// if(!currentOutboundMessage.initialize()) { @@ -647,7 +654,7 @@ private boolean writeClientBoundPlainMessage(SocketChannel channel, MessageBuffe return sendMore; }//writeClientBoundPlainMessage()// /* (non-Javadoc) - * @see com.foundation.web.server.WebServer.AbstractSocketContext#processRequest() + * @see com.foundation.web.server.WebServer.AbstractSocketContext#readIncomingMessages() */ protected void readIncomingMessages() throws IOException { boolean requiresRead = true; @@ -889,7 +896,6 @@ protected void readIncomingMessages() throws IOException { IPassThroughDomain passThroughDomain = ((IPassThroughDomain) application); //Setup the pass through socket context (and socket channel). All data will be sent to this context to be sent to the remote process.// - //relatedSocketContext = new PassThroughSocketContext(getWebServer(), getNetworkListener(), this, passThroughDomain.getAddress(), passThroughDomain.getPort()); relatedSocketContext = new PassThroughSocketContext(this, passThroughDomain.getAddress(), passThroughDomain.getPort()); }//if// @@ -1244,8 +1250,7 @@ private boolean processRequestedHost(ByteBuffer fragment) throws IOException { //Keep adding ASCII characters to the message header fragment until the next line is read or we exceed the maximum length of the header.// while(fragment.hasRemaining() && (!(buffer.length() > 1 && (buffer.charAt(buffer.length() - 2) == '\r') && (buffer.charAt(buffer.length() - 1) == '\n')))) { if(totalHeaderSize == 4096) { - //Force the connection to the client to be closed.// - try {key.channel().close();}catch(Throwable e2) {} + close(); //Throw an exception that should not be logged. This happens occationally when an attacker tries to exploit any buffer overrun weaknesses.// throw new IgnoredIOException(null); }//if// @@ -1264,7 +1269,7 @@ private boolean processRequestedHost(ByteBuffer fragment) throws IOException { if(buffer.length() == 2) { //End of the header reached. No host provided. Kill the connection?// //Force the connection to the client to be closed.// - try {key.channel().close();}catch(Throwable e2) {} + close(); //Throw an exception that should not be logged. This happens occationally when an attacker tries to exploit any header reading weaknesses (all major browsers send a host header).// throw new IgnoredIOException(null); }//if// @@ -1510,7 +1515,7 @@ private boolean parseFirstTlsMessage(SocketChannel channel) throws IOException { /** * Processes the client request given the latest fragment of a message. * @param messageFragment The message fragment. - * @result Whether request is in a receive state. Will be false if the request generated a response that could not be completely transmitted. + * @return Whether the request could NOT be built from the given fragment, and thus more data must be read from the socket before the request can be handled. */ private boolean processClientRequest(ByteBuffer fragment) throws IOException { boolean result = true; @@ -1537,7 +1542,7 @@ private boolean processClientRequest(ByteBuffer fragment) throws IOException { while(fragment.hasRemaining() && !isCompleteHeader(messageHeaderFragment)) { if(messageHeaderFragment.length() == 4096) { //Force the connection to the client to be closed.// - try {key.channel().close();}catch(Throwable e2) {} + close(); //Throw an exception that should not be logged. This happens occationally when an attacker tries to exploit any buffer overrun weaknesses.// throw new IgnoredIOException(null); }//if// @@ -1555,11 +1560,11 @@ private boolean processClientRequest(ByteBuffer fragment) throws IOException { //Prepare the request.// try { - request = new Request(++lastRequestNumber, messageHeaderFragment.toString(), ((SocketChannel) key.channel()).socket().getInetAddress().getHostAddress(), this, this, isSsl()); + request = new Request(getNextRequestNumber(), messageHeaderFragment.toString(), ((SocketChannel) getKey().channel()).socket().getInetAddress().getHostAddress(), this, this, isSsl()); - //Log the request header if running in getWebServer().debug() mode.// + //Log the request header if running in debug mode.// if(getWebServer().debug()) { - Debug.log(request.toString()); + Debug.log(getId() + "|" + System.nanoTime() + "|Request created (header only loaded): " + request.getHeaderRequest()); }//if// }//try// catch(IOException e) { @@ -1600,12 +1605,8 @@ private boolean processClientRequest(ByteBuffer fragment) throws IOException { if(application != null) { int contentLength = 0; - //TODO: Remove if(getWebServer().debug()) { - boolean sessionFound = application.getSession(request.getSessionId()) != null; - boolean canRecreate = request.getSessionId() != null; - - Debug.log("SC: " + getId() + "; Req#: " + request.getRequestNumber() + "; ReqURI: " + request.getUri() + "\n\t(SessionId: " + request.getSessionId() + "; SecureSessionId: " + request.getSecureSessionId() + ")\n\tSession Found: " + sessionFound + (!sessionFound ? "; Can Recreate: " + canRecreate : "")); + Debug.log(getId() + "|" + System.nanoTime() + "|Application identified for request: " + request.getHeaderRequest()); }//if// request.setSession(application.getSession(request.getSessionId())); @@ -1627,9 +1628,12 @@ private boolean processClientRequest(ByteBuffer fragment) throws IOException { //This is a security mechanism because the client is SUPPOSED to keep the secure session ID very private and not allow access to it by other sites. It ensures that a recreated session actually comes from a client that had it created in the first place.// if(request.getSecureSessionId() != null && request.getSession() != null && request.getSession().getSecureSessionId() != null) { if(!Comparator.equals(request.getSecureSessionId(), request.getSession().getSecureSessionId())) { - Debug.log("SC: " + getId() + "Forcing connection closure."); + if(getWebServer().debug()) { + Debug.log(getId() + "|" + System.nanoTime() + "|Forcing connection closure (invalid secure session id)."); + }//if// + //Force the connection to the client to be closed.// - try {key.channel().close();}catch(Throwable e2) {} + close(); //Throw an exception that should not be logged. This might happen when an attacker tries to reuse stored session data on a client.// throw new IgnoredIOException(null); }//if// @@ -1678,6 +1682,10 @@ private boolean processClientRequest(ByteBuffer fragment) throws IOException { //If this is a multipart or custom message then we should do as much verification of size validity as possible.// if(request.getContentTypeCode() == IRequest.CONTENT_TYPE_MULTI_PART_FORM_DATA) { + if(getWebServer().debug()) { + Debug.log(getId() + "|" + System.nanoTime() + "|Incoming request is Multipart Form Data: " + request.getHeaderRequest()); + }//if// + //Ensure that the multi-part form data message begins with the \r\n from the end of the header, allowing for the boundary recognition to proceed.// //This could be a problem if the fragment doesn't have the last two characters (it might have only the last 1).// if(fragment.position() == 1) { @@ -1719,7 +1727,7 @@ private boolean processClientRequest(ByteBuffer fragment) throws IOException { request.getRequestBytes().put(bytes); }//else// -// Debug.log("Read " + (remaining - context.request.getRequestBytes().remaining()) + " bytes of the current client request."); +// Debug.log("Read " + (remaining - request.getRequestBytes().remaining()) + " bytes of the current client request."); //Reset the fragment back to its previous position.// fragment.position(position); @@ -1773,7 +1781,7 @@ private boolean processClientRequest(ByteBuffer fragment) throws IOException { //TODO: What do we do if there are no parts in a multi-part message? Here we found the end part while NOT reading a message - shouldn't ever happen. //Force the connection to the client to be closed.// - try {key.channel().close();}catch(Throwable e2) {} + close(); //Throw an exception that should not be logged. This is a malformed message.// throw new IgnoredIOException(null); }//if// @@ -2010,20 +2018,6 @@ private boolean processClientRequest(final Request request, SelectionKey key) th try { boolean applicationNotFound = false; - //Moved this bit of code into the calling method near the top. Access to the application was required that far up in the sequence of events.// -// if(context.webApplicationContainer == null) { -// //Save the domain.// -// context.domain = request.getHost() != null ? request.getHost().toLowerCase() : null; -// -// if(context.domain != null) { -// //Get the web application for the given domain.// -// //Synchronize to prevent another thread from altering the service's web applications while we are accessing it.// -// synchronized(WebServer.this) { -// context.webApplicationContainer = context.serverSocketContext.serviceListener.getWebApplicationContainer(context.domain); -// }//synchronized// -// }//if// -// }//if// - //Verify an application was found for the context.// if(webApplicationContainer != null) { //Increment the activity counter until we finish processing this request.// @@ -2039,9 +2033,8 @@ private boolean processClientRequest(final Request request, SelectionKey key) th boolean hasNewSessionData = false; if(session == null) { - //TODO: Remove if(getWebServer().debug()) { - Debug.log("SC: " + getId() + " Creating Session"); + Debug.log(getId() + "|" + System.nanoTime() + "|Creating Session"); }//if// request.setSession(session = application.createSession()); @@ -2062,13 +2055,12 @@ private boolean processClientRequest(final Request request, SelectionKey key) th allowSecureAccess = true; }//if// else { - Debug.log(new RuntimeException("Error: The client did not send the correct secure session getId() with the request!")); + Debug.log(new RuntimeException("Error: The client did not send the correct secure session id with the request!")); }//else// }//if// else if(session != null && session.getSecureSessionId() == null) { - //TODO: Remove if(getWebServer().debug()) { - Debug.log("SC: " + getId() + " Creating Secure Session"); + Debug.log(getId() + "|" + System.nanoTime() + "|Creating Secure Session"); }//if// application.createSecureSession(session); @@ -2121,7 +2113,7 @@ private boolean processClientRequest(final Request request, SelectionKey key) th if(content == null) { //For now we will simply close the connection to the client - this should never happen anyhow and if it does, it could be an attack.// Debug.log(new RuntimeException("Application not found for the host: " + request.getHost())); - key.channel().close(); + close(); }//if// else { response = new Response(request, null, null); @@ -2138,7 +2130,7 @@ private boolean processClientRequest(final Request request, SelectionKey key) th //response.setResourceNotFound(true); //For now simply close the connection. This simplifies threading.// //Close the socket.// - try {key.channel().close();}catch(Throwable e2) {} + close(); //Close the response (which also closes the request).// try {response.close();} catch(Throwable e2) {} }//catch// @@ -2147,6 +2139,7 @@ private boolean processClientRequest(final Request request, SelectionKey key) th }//processClientRequest()// /** * Processes a client request. + * @param application The web application the request is associated with. * @param request The request. * @param response The response container. * @param session The session for the request. This may be null in the case of non-standard web applications such as a forwarding domain. @@ -2163,6 +2156,10 @@ private boolean internalProcessClientRequest(SelectionKey key, final IWebApplica }//if// //Send the request to the application to be processed if we are not dealing with an error.// else if(application != null) { + if(getWebServer().debug()) { + Debug.log(getId() + "|" + System.nanoTime() + "|Received request: " + request.getHeaderRequest()); + }//if// + String upgrade = request.getHeaderFieldValue("Upgrade"); if(upgrade != null && "websocket".equalsIgnoreCase(upgrade)) { @@ -2175,18 +2172,26 @@ private boolean internalProcessClientRequest(SelectionKey key, final IWebApplica application.handleWebSocketUpgrade(request, response, session, allowSecureAccess, clientHadBadSession, this, connection, secWebSocketKey, secWebSocketProtocol, secWebSocketVersion, origin); }//if// else { + if(getWebServer().debug()) { + Debug.log(getId() + "|" + System.nanoTime() + "|Passing request to application: " + request.getHeaderRequest()); + }//if// + application.processRequest(request, response, session, allowSecureAccess, clientHadBadSession, this); }//else// }//else if// - //Convert the response into a byte stream and send it via the socket.// + if(getWebServer().debug()) { + Debug.log(getId() + "|" + System.nanoTime() + "|Beginning response: " + request.getHeaderRequest()); + }//if// + + //Convert the response into a byte stream and send it via the socket (the actual sending occurs later on another thread so this is non-blocking).// sendHttpResponse(response); result = true; }//try// catch(Throwable e) { Debug.log(e); //Close the socket.// - try {key.channel().close();} catch(Throwable e2) {} + close(); //Close the response (which also closes the request).// try {response.close();} catch(Throwable e2) {} }//catch//