Patch from bug 87902 to fix SSL/TLS logic.

- make TLS intolerant server detection over proxies work
  (this bug 87902)
- on connection failure, only retry without TLS when it is really
  likely to help (bug 149910)
- remove obsolete workarounds in SSL i/o layer
  (see removed comments in patch)
- avoid to confuse programmers reading code,
  by renaming TLSStepUp (which means something else)
  to the correct term STARTTLS (what the code is actually doing).
  (As suggested by nelsonb)
- If an invalid or expired etc. server certificate is presented,
  a warning is shown. If the user decides to cancel,
  network activity should stop immediately.
  (we currently warn multiple times) (bug 87209)

r=javi/darin/ducarroz/dmose sr=alecf
This commit is contained in:
kaie%netscape.com 2002-08-14 23:43:28 +00:00
parent 404f750082
commit 9190a3d74e
11 changed files with 136 additions and 92 deletions

View File

@ -177,7 +177,7 @@ nsLDAPSSLConnect(const char *hostlist, int defport, int timeout,
// Add the NSPR layer for SSL provided by PSM to this socket.
//
tlsSocketProvider = do_GetService(NS_TLSSTEPUPSOCKETPROVIDER_CONTRACTID,
tlsSocketProvider = do_GetService(NS_STARTTLSSOCKETPROVIDER_CONTRACTID,
&rv);
if (NS_FAILED(rv)) {
NS_ERROR("nsLDAPSSLConnect(): unable to get socket provider service");
@ -203,16 +203,18 @@ nsLDAPSSLConnect(const char *hostlist, int defport, int timeout,
// If possible we want to avoid using SSLv2, as this can confuse
// some directory servers (notably the netscape 4.1 ds). The only
// way that PSM provides for us to do this is to invoke TLSStepUp.
// way that PSM provides for us to do this is to use a socket that can
// be used for the STARTTLS protocol, because the STARTTLS protocol disallows
// the use of SSLv2.
// (Thanks to Brian Ryner for helping figure this out).
//
sslSocketControl = do_QueryInterface(securityInfo, &rv);
if (NS_FAILED(rv)) {
NS_WARNING("nsLDAPSSLConnect(): unable to QI to nsISSLSocketControl");
} else {
rv = sslSocketControl->TLSStepUp();
rv = sslSocketControl->StartTLS();
if (NS_FAILED(rv)) {
NS_WARNING("nsLDAPSSLConnect(): TLSStepUp failed");
NS_WARNING("nsLDAPSSLConnect(): StartTLS failed");
}
}

View File

@ -319,7 +319,7 @@ void nsSmtpProtocol::Initialize(nsIURI * aURL)
smtpUrl->GetNotificationCallbacks(getter_AddRefs(callbacks));
if (m_prefTrySSL != PREF_SSL_NEVER) {
rv = OpenNetworkSocket(aURL, "tlsstepup", callbacks);
rv = OpenNetworkSocket(aURL, "starttls", callbacks);
if (NS_FAILED(rv) && m_prefTrySSL == PREF_SSL_TRY) {
m_prefTrySSL = PREF_SSL_NEVER;
rv = OpenNetworkSocket(aURL, nsnull, callbacks);
@ -710,7 +710,7 @@ PRInt32 nsSmtpProtocol::SendTLSResponse()
nsCOMPtr<nsISSLSocketControl> sslControl = do_QueryInterface(secInfo, &rv);
if (NS_SUCCEEDED(rv) && sslControl) {
rv = sslControl->TLSStepUp();
rv = sslControl->StartTLS();
}
}

View File

@ -908,9 +908,9 @@ nsresult nsSocketTransport::doConnection(PRInt16 aSelectFlags)
if (NS_FAILED(rv) || !mSocketFD) break;
// if the service was ssl or tlsstepup, we want to hold onto the socket info
// if the service was ssl or starttls, we want to hold onto the socket info
if (nsCRT::strcmp(mSocketTypes[type], "ssl") == 0 ||
nsCRT::strcmp(mSocketTypes[type], "tlsstepup") == 0) {
nsCRT::strcmp(mSocketTypes[type], "starttls") == 0) {
mSecurityInfo = socketInfo;
nsCOMPtr<nsISSLSocketControl> secCtrl(do_QueryInterface(mSecurityInfo));
if (secCtrl)
@ -1040,10 +1040,10 @@ nsresult nsSocketTransport::doConnection(PRInt16 aSelectFlags)
// if the connection phase is finished, and the ssl layer
// has been pushed, and we were proxying (transparently; ie. nothing
// has to happen in the protocol layer above us), it's time
// for the ssl to "step up" and start doing it's thing.
// for the ssl to start doing it's thing.
nsCOMPtr<nsISSLSocketControl> sslControl = do_QueryInterface(mSecurityInfo, &rv);
if (NS_SUCCEEDED(rv) && sslControl)
sslControl->ProxyStepUp();
sslControl->ProxyStartSSL();
}
return rv;
}

View File

@ -129,9 +129,9 @@ nsHttpConnection::SetTransaction(nsAHttpTransaction *transaction,
// called from the socket thread
nsresult
nsHttpConnection::ProxyStepUp()
nsHttpConnection::ProxyStartSSL()
{
LOG(("nsHttpConnection::ProxyStepUp [this=%x]\n", this));
LOG(("nsHttpConnection::ProxyStartSSL [this=%x]\n", this));
#ifdef DEBUG
NS_PRECONDITION(PR_GetCurrentThread() == NS_SOCKET_THREAD, "wrong thread");
#endif
@ -143,7 +143,7 @@ nsHttpConnection::ProxyStepUp()
nsCOMPtr<nsISSLSocketControl> ssl = do_QueryInterface(securityInfo, &rv);
if (NS_FAILED(rv)) return rv;
return ssl->ProxyStepUp();
return ssl->ProxyStartSSL();
}
PRBool
@ -283,7 +283,7 @@ nsHttpConnection::OnHeadersAvailable(nsAHttpTransaction *trans,
if (responseHead->Status() == 200) {
LOG(("SSL proxy CONNECT succeeded!\n"));
*reset = PR_TRUE;
ProxyStepUp();
ProxyStartSSL();
mWriteRequest->Resume();
}
else {

View File

@ -75,7 +75,7 @@ public:
nsresult SetTransaction(nsAHttpTransaction *, PRUint8 capabilities);
// called to cause the underlying socket to start speaking SSL
nsresult ProxyStepUp();
nsresult ProxyStartSSL();
PRBool SupportsPipelining() { return mSupportsPipelining; }
PRBool IsKeepAlive() { return mKeepAliveMask && mKeepAlive; }

View File

@ -31,7 +31,7 @@ interface nsISSLSocketControl : nsISupports {
attribute nsIInterfaceRequestor notificationCallbacks;
attribute boolean forceHandshake; /* obsolete, unused */
void proxyStepUp();
void TLSStepUp();
void proxyStartSSL();
void StartTLS();
};

View File

@ -32,10 +32,10 @@ interface nsISSLSocketProvider : nsISocketProvider {
#define NS_ISSLSOCKETPROVIDER_CONTRACTID NS_NETWORK_SOCKET_CONTRACTID_PREFIX "ssl"
#define NS_ISSLSOCKETPROVIDER_CLASSNAME "Mozilla SSL Socket Provider Component"
/* This code produces a normal socket which can be stepped up to TLS by
* calling its nsISSLSocketControl->TLSStepUp()
/* This code produces a normal socket which can be used to initiate the STARTTLS protocol
* by calling its nsISSLSocketControl->StartTLS()
*/
#define NS_TLSSTEPUPSOCKETPROVIDER_CONTRACTID NS_NETWORK_SOCKET_CONTRACTID_PREFIX "tlsstepup"
#define NS_TLSSTEPUPSOCKETPROVIDER_CLASSNAME "Mozilla TLS Step-up Socket Provider Component"
#define NS_STARTTLSSOCKETPROVIDER_CONTRACTID NS_NETWORK_SOCKET_CONTRACTID_PREFIX "starttls"
#define NS_STARTTLSSOCKETPROVIDER_CLASSNAME "Mozilla STARTTLS Capable Socket Provider Component"
%}

View File

@ -143,7 +143,7 @@ nsSSLIOLayerImportFD(PRFileDesc *fd,
nsNSSSocketInfo *infoObject,
const char *host);
static nsresult
nsSSLIOLayerSetOptions(PRFileDesc *fd, PRBool forTLSStepUp,
nsSSLIOLayerSetOptions(PRFileDesc *fd, PRBool forSTARTTLS,
const char *proxyHost, const char *host, PRInt32 port,
nsNSSSocketInfo *infoObject);
@ -151,9 +151,10 @@ nsSSLIOLayerSetOptions(PRFileDesc *fd, PRBool forTLSStepUp,
nsNSSSocketInfo::nsNSSSocketInfo()
: mFd(nsnull),
mSecurityState(nsIWebProgressListener::STATE_IS_INSECURE),
mForTLSStepUp(PR_FALSE),
mForSTARTTLS(PR_FALSE),
mFirstWrite(PR_TRUE),
mTLSIntolerant(PR_FALSE),
mCanceled(PR_FALSE),
mHasCleartextPhase(PR_FALSE),
mPort(0),
mCAChain(nsnull)
{
@ -215,18 +216,24 @@ nsNSSSocketInfo::GetPort(PRInt32 *aPort)
return NS_OK;
}
nsresult
nsNSSSocketInfo::GetTLSIntolerant(PRBool *aTLSIntolerant)
void nsNSSSocketInfo::SetCanceled(PRBool aCanceled)
{
*aTLSIntolerant = mTLSIntolerant;
return NS_OK;
mCanceled = aCanceled;
}
nsresult
nsNSSSocketInfo::SetTLSIntolerant(PRBool aTLSIntolerant)
PRBool nsNSSSocketInfo::GetCanceled()
{
mTLSIntolerant = aTLSIntolerant;
return NS_OK;
return mCanceled;
}
void nsNSSSocketInfo::SetHasCleartextPhase(PRBool aHasCleartextPhase)
{
mHasCleartextPhase = aHasCleartextPhase;
}
PRBool nsNSSSocketInfo::GetHasCleartextPhase()
{
return mHasCleartextPhase;
}
NS_IMETHODIMP
@ -318,27 +325,32 @@ nsNSSSocketInfo::SetForceHandshake(PRBool forceHandshake)
}
nsresult
nsNSSSocketInfo::GetForTLSStepUp(PRBool* aResult)
nsNSSSocketInfo::GetForSTARTTLS(PRBool* aForSTARTTLS)
{
*aResult = mForTLSStepUp;
*aForSTARTTLS = mForSTARTTLS;
return NS_OK;
}
nsresult
nsNSSSocketInfo::SetForTLSStepUp(PRBool forTLSStepUp)
nsNSSSocketInfo::SetForSTARTTLS(PRBool aForSTARTTLS)
{
mForTLSStepUp = forTLSStepUp;
mForSTARTTLS = aForSTARTTLS;
return NS_OK;
}
NS_IMETHODIMP
nsNSSSocketInfo::ProxyStepUp()
nsNSSSocketInfo::ProxyStartSSL()
{
return TLSStepUp();
return ActivateSSL();
}
NS_IMETHODIMP
nsNSSSocketInfo::TLSStepUp()
nsNSSSocketInfo::StartTLS()
{
return ActivateSSL();
}
nsresult nsNSSSocketInfo::ActivateSSL()
{
if (SECSuccess != SSL_OptionSet(mFd, SSL_SECURITY, PR_TRUE))
return NS_ERROR_FAILURE;
@ -346,11 +358,6 @@ nsNSSSocketInfo::TLSStepUp()
if (SECSuccess != SSL_ResetHandshake(mFd, PR_FALSE))
return NS_ERROR_FAILURE;
// This is a work around for NSS bug 56924 which is scheduled
// for a fix in NSS 3.3, but we're currently on version 3.2.1,
// so we need this work around.
PR_Write(mFd, nsnull, 0);
mFirstWrite = PR_TRUE;
return NS_OK;
@ -442,6 +449,13 @@ displayAlert(nsXPIDLString formattedString, nsNSSSocketInfo *infoObject)
static nsresult
nsHandleSSLError(nsNSSSocketInfo *socketInfo, PRInt32 err)
{
if (socketInfo->GetCanceled()) {
// If the socket has been flagged as canceled,
// the code who did was responsible for showing
// an error message (if desired).
return NS_OK;
}
nsresult rv;
NS_DEFINE_CID(nssComponentCID, NS_NSSCOMPONENT_CID);
nsCOMPtr<nsINSSComponent> nssComponent(do_GetService(nssComponentCID, &rv));
@ -957,12 +971,8 @@ nsSSLIOLayerRead(PRFileDesc* fd, void* buf, PRInt32 amount)
nsNSSSocketInfo *socketInfo = nsnull;
socketInfo = (nsNSSSocketInfo*)fd->secret;
NS_ASSERTION(socketInfo,"nsNSSSocketInfo was null for an fd");
PRBool tlsIntolerant;
socketInfo->GetTLSIntolerant(&tlsIntolerant);
if (tlsIntolerant) {
// By returning 0 here, necko will retry the connection
// again.
return 0;
if (socketInfo->GetCanceled()) {
return PR_FAILURE;
}
PRInt32 bytesRead = fd->lower->methods->read(fd->lower, buf, amount);
@ -986,7 +996,7 @@ nsSSLIOLayerWrite(PRFileDesc* fd, const void* buf, PRInt32 amount)
{
if (!fd || !fd->lower)
return PR_FAILURE;
#ifdef DEBUG_SSL_VERBOSE
DEBUG_DUMP_BUFFER((unsigned char*)buf, amount);
#endif
@ -994,6 +1004,11 @@ nsSSLIOLayerWrite(PRFileDesc* fd, const void* buf, PRInt32 amount)
PRBool firstWrite;
socketInfo = (nsNSSSocketInfo*)fd->secret;
NS_ASSERTION(socketInfo,"nsNSSSocketInfo was null for an fd");
if (socketInfo->GetCanceled()) {
return PR_FAILURE;
}
socketInfo->GetFirstWrite(&firstWrite);
PRBool oldBlockVal = PR_FALSE;
PRBool oldBlockReset = PR_FALSE;
@ -1029,19 +1044,27 @@ nsSSLIOLayerWrite(PRFileDesc* fd, const void* buf, PRInt32 amount)
// there are enough broken servers out there that such a gross work-around
// is necessary. :(
if (bytesWritten == -1) {
if (bytesWritten == -1 && firstWrite) {
// Let's see if there was an error set by the SSL libraries that we
// should tell the user about.
PRInt32 err = PR_GetError();
if (firstWrite) {
PRBool wantRetry = PR_FALSE;
PRBool withInitialCleartext = socketInfo->GetHasCleartextPhase();
// When not using a proxy we'll see a connection reset error.
// When using a proxy, we'll see an end of file error.
if ((!withInitialCleartext && PR_CONNECT_RESET_ERROR == err)
||
(withInitialCleartext && PR_END_OF_FILE_ERROR == err)) {
PRBool tlsOn;
SSL_OptionGet(fd->lower, SSL_ENABLE_TLS, &tlsOn);
if (tlsOn) {
// Make necko re-try this connection by sending back an EOF
// on the first read. (ie premature EOF)
bytesWritten = 0;
socketInfo->SetTLSIntolerant(PR_TRUE);
// We don't want to communicate over this socket any longer.
// Mark it as canceled, and make both our read and write
// functions return failure.
socketInfo->SetCanceled(PR_TRUE);
// Now let's add this site to the list of TLS intolerant
// sites.
char buf[1024];
@ -1054,19 +1077,25 @@ nsSSLIOLayerWrite(PRFileDesc* fd, const void* buf, PRInt32 amount)
// We don't really wanna associate a value. If it's
// in the table, that means it's TLS intolerant and
// we don't really need to know anything else.
gTLSIntolerantSites->Put(&key, nsnull);
} else if (IS_SSL_ERROR(err) || IS_SEC_ERROR(err)) {
// This is the case where the first write failed with
// TLS turned off.
nsHandleSSLError(socketInfo, err);
}
} else if (IS_SSL_ERROR(err) || IS_SEC_ERROR(err)) {
// This is the case where a subsequent write has failed,
// ie not the first write.
nsHandleSSLError(socketInfo, err);
gTLSIntolerantSites->Put(&key, nsnull);
wantRetry = PR_TRUE;
// We want to cause the network layer to retry the connection.
// It won't retry on an end of file error.
// If we were using a proxy, change the error code
// to the connection reset error.
if (withInitialCleartext && PR_END_OF_FILE_ERROR == err) {
PR_SetError(PR_CONNECT_RESET_ERROR, 0);
}
}
}
if (!wantRetry && (IS_SSL_ERROR(err) || IS_SEC_ERROR(err))) {
nsHandleSSLError(socketInfo, err);
}
}
// TLS intolerant servers only cause the first write to fail, so let's
// set the fristWrite attribute to false so that we don't try the logic
// above again in a subsequent write.
@ -1108,7 +1137,7 @@ nsSSLIOLayerNewSocket(const char *host,
PRInt32 proxyPort,
PRFileDesc **fd,
nsISupports** info,
PRBool forTLSStepUp)
PRBool forSTARTTLS)
{
// XXX - this code is duplicated in nsSSLIOLayerAddToSocket
if (firstTime) {
@ -1123,7 +1152,7 @@ nsSSLIOLayerNewSocket(const char *host,
if (!sock) return NS_ERROR_OUT_OF_MEMORY;
nsresult rv = nsSSLIOLayerAddToSocket(host, port, proxyHost, proxyPort,
sock, info, forTLSStepUp);
sock, info, forSTARTTLS);
if (NS_FAILED(rv)) {
PR_Close(sock);
return rv;
@ -2086,6 +2115,10 @@ nsNSSBadCertHandler(void *arg, PRFileDesc *sslSocket)
}
NS_RELEASE(nssCert);
CERT_DestroyCertificate(peerCert);
if (rv != SECSuccess) {
// if the cert is bad, we don't want to connect
infoObject->SetCanceled(PR_TRUE);
}
return rv;
}
@ -2120,15 +2153,18 @@ loser:
}
static nsresult
nsSSLIOLayerSetOptions(PRFileDesc *fd, PRBool forTLSStepUp,
nsSSLIOLayerSetOptions(PRFileDesc *fd, PRBool forSTARTTLS,
const char *proxyHost, const char *host, PRInt32 port,
nsNSSSocketInfo *infoObject)
{
if ((forTLSStepUp || proxyHost) &&
SECSuccess != SSL_OptionSet(fd, SSL_SECURITY, PR_FALSE))
return NS_ERROR_FAILURE;
if (forSTARTTLS || proxyHost) {
if (SECSuccess != SSL_OptionSet(fd, SSL_SECURITY, PR_FALSE)) {
return NS_ERROR_FAILURE;
}
infoObject->SetHasCleartextPhase(PR_TRUE);
}
if (forTLSStepUp) {
if (forSTARTTLS) {
if (SECSuccess != SSL_OptionSet(fd, SSL_ENABLE_SSL2, PR_FALSE)) {
return NS_ERROR_FAILURE;
}
@ -2173,7 +2209,7 @@ nsSSLIOLayerAddToSocket(const char* host,
PRInt32 proxyPort,
PRFileDesc* fd,
nsISupports** info,
PRBool forTLSStepUp)
PRBool forSTARTTLS)
{
PRFileDesc* layer = nsnull;
nsresult rv;
@ -2191,7 +2227,7 @@ nsSSLIOLayerAddToSocket(const char* host,
if (!infoObject) return NS_ERROR_FAILURE;
NS_ADDREF(infoObject);
infoObject->SetForTLSStepUp(forTLSStepUp);
infoObject->SetForSTARTTLS(forSTARTTLS);
infoObject->SetHostName(host);
infoObject->SetPort(port);
@ -2203,7 +2239,7 @@ nsSSLIOLayerAddToSocket(const char* host,
infoObject->SetFileDescPtr(sslSock);
rv = nsSSLIOLayerSetOptions(sslSock, forTLSStepUp, proxyHost, host, port,
rv = nsSSLIOLayerSetOptions(sslSock, forSTARTTLS, proxyHost, host, port,
infoObject);
if (NS_FAILED(rv))
@ -2226,7 +2262,7 @@ nsSSLIOLayerAddToSocket(const char* host,
infoObject->QueryInterface(NS_GET_IID(nsISupports), (void**) (info));
// We are going use a clear connection first //
if (forTLSStepUp || proxyHost) {
if (forSTARTTLS || proxyHost) {
infoObject->SetFirstWrite(PR_FALSE);
}

View File

@ -56,8 +56,8 @@ public:
nsresult SetSecurityState(PRUint32 aState);
nsresult SetShortSecurityDescription(const PRUnichar *aText);
nsresult SetForTLSStepUp(PRBool useTLS);
nsresult GetForTLSStepUp(PRBool *useTLS);
nsresult SetForSTARTTLS(PRBool aForSTARTTLS);
nsresult GetForSTARTTLS(PRBool *aForSTARTTLS);
nsresult GetFileDescPtr(PRFileDesc** aFilePtr);
nsresult SetFileDescPtr(PRFileDesc* aFilePtr);
@ -71,8 +71,11 @@ public:
nsresult GetPort(PRInt32 *aPort);
nsresult SetPort(PRInt32 aPort);
nsresult GetTLSIntolerant(PRBool *aTLSIntolerant);
nsresult SetTLSIntolerant(PRBool aTLSIntolerant);
void SetCanceled(PRBool aCanceled);
PRBool GetCanceled();
void SetHasCleartextPhase(PRBool aHasCleartextPhase);
PRBool GetHasCleartextPhase();
nsresult RememberCAChain(CERTCertList *aCertList);
@ -84,15 +87,18 @@ protected:
PRFileDesc* mFd;
PRUint32 mSecurityState;
nsString mShortDesc;
PRBool mForTLSStepUp;
PRBool mFirstWrite;
PRBool mTLSIntolerant;
PRPackedBool mForSTARTTLS;
PRPackedBool mFirstWrite;
PRPackedBool mCanceled;
PRPackedBool mHasCleartextPhase;
PRInt32 mPort;
nsXPIDLCString mHostName;
CERTCertList *mCAChain;
/* SSL Status */
nsCOMPtr<nsISSLStatus> mSSLStatus;
nsresult ActivateSSL();
};
nsresult nsSSLIOLayerNewSocket(const char *host,
@ -101,7 +107,7 @@ nsresult nsSSLIOLayerNewSocket(const char *host,
PRInt32 proxyPort,
PRFileDesc **fd,
nsISupports **securityInfo,
PRBool forTLSStepUp);
PRBool forSTARTTLS);
nsresult nsSSLIOLayerAddToSocket(const char *host,
PRInt32 port,
@ -109,7 +115,7 @@ nsresult nsSSLIOLayerAddToSocket(const char *host,
PRInt32 proxyPort,
PRFileDesc *fd,
nsISupports **securityInfo,
PRBool forTLSStepUp);
PRBool forSTARTTLS);
nsresult nsSSLIOLayerFreeTLSIntolerantSites();
nsresult displayAlert(nsXPIDLString formattedString, nsNSSSocketInfo *infoObject);

View File

@ -224,9 +224,9 @@ static const nsModuleComponentInfo components[] =
},
{
NS_TLSSTEPUPSOCKETPROVIDER_CLASSNAME,
NS_TLSSTEPUPSOCKETPROVIDER_CID,
NS_TLSSTEPUPSOCKETPROVIDER_CONTRACTID,
NS_STARTTLSSOCKETPROVIDER_CLASSNAME,
NS_STARTTLSSOCKETPROVIDER_CID,
NS_STARTTLSSOCKETPROVIDER_CONTRACTID,
nsTLSSocketProviderConstructor
},

View File

@ -27,7 +27,7 @@
#include "nsISSLSocketProvider.h"
#define NS_TLSSTEPUPSOCKETPROVIDER_CID \
#define NS_STARTTLSSOCKETPROVIDER_CID \
{ /* b9507aec-1dd1-11b2-8cd5-c48ee0c50307 */ \
0xb9507aec, \
0x1dd1, \