[SERVER-28469] Improve SSLManager error handling Created: 24/Mar/17  Updated: 11/Jun/19  Resolved: 11/Jun/19

Status: Closed
Project: Core Server
Component/s: Networking
Affects Version/s: None
Fix Version/s: 4.3.1

Type: Improvement Priority: Major - P3
Reporter: Kevin Pulo Assignee: Alya Berciu
Resolution: Done Votes: 1
Labels: neweng, platforms-interns-2017
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible
Sprint: Dev Tools 2019-06-17
Participants:
Case:

 Description   

These all relate to the code in and around SSLManager::_handleSSLError.

  • Empty string passed for the hostname/IP in the thrown SocketException:

        throw SocketException(SocketException::CONNECT_ERROR, "");
    

    This leads to strange log messages "socket exception [CONNECT_ERROR] for" (with no hostname or IP address after the "for"), which at first blush appear to have been abnormally truncated, eg:

    2017-03-17T15:32:38.301-0400 F STORAGE [initandlisten] Aborting due to exception in WT_ENCRYPTOR::customize: Location9001: socket exception [CONNECT_ERROR] for
    

    In fact _handleSSLError is always called within the context of an SSLConnection object. This should be passed into _handleSSLError so that the server name can be passed to the SocketException, ie. via remoteString() (same as is done in SSLManager::_flushNetworkBIO() and throughout sock.cpp).

    -    throw SocketException(SocketException::CONNECT_ERROR, "");
    +    throw SocketException(SocketException::CONNECT_ERROR, sslConn->socket->remoteString());
    

  • CONNECT_ERROR is always thrown, even though _handleSSLError() is called from contexts other than connection establishment, such as SSL_read() and SSL_write(). In these cases RECV_ERROR and SEND_ERROR would be more appropriate. Thus _handleSSLError() should take an additional parameter which is the type of SocketException that should be thrown.
  • In the case of SSL_ERROR_ZERO_RETURN, a message is logged at LOG(3). This case indicates that the SSL stream has been (unexpectedly) closed — whether explicitly in the SSL protocol by the remote, or for some abnormal reason. It is important that this fact be logged so that users can be aware of it. Thus, the LOG(3) should be changed to error() (like the other cases).

    The code comment in this case of the switch suggests that it may be possible to re-establish a new SSL stream on the underlying transport, and to not throw a SocketException. If this later turns out to be possible and desirable, then the error() here could be updated accordingly (ie. changed back to LOG(3) or similar) at that time.


 Comments   
Comment by Githook User [ 11/Jun/19 ]

Author:

{'name': 'Alya Berciu', 'email': 'alya.berciu@mongodb.com', 'username': 'alyacb'}

Message: SERVER-28469 Improve SSLManager error handling
Branch: master
https://github.com/mongodb/mongo/commit/6fd8c8dc3d029c7e69db80bf0a209905e95c5f72

Generated at Thu Feb 08 04:18:14 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.