diff --git a/src/SSLProxy.cc b/src/SSLProxy.cc index a0cf73b8fb..38ce3ba085 100644 --- a/src/SSLProxy.cc +++ b/src/SSLProxy.cc @@ -174,7 +174,6 @@ bool SSL_RecordBuilder::addSegment(const u_char* data, int length) if ( ! computeExpectedSize(data, length) ) return false; - // Insert weird here replacing assert. if ( neededSize > expectedSize ) { sslEndpoint->Weird("SSL_RecordBuilder::addSegment neededSize > expectedSize"); @@ -278,7 +277,6 @@ bool SSL_RecordBuilder::addSegment(const u_char* data, int length) { // another (middle) segment if ( length <= MIN_FRAGMENT_SIZE ) sslEndpoint->Parent()->Weird( "SSLProxy: Excessive small TCP Segment!" ); - addData(data, length); break; } diff --git a/src/SSLv3.cc b/src/SSLv3.cc index 92d18c6f26..9343b5076f 100644 --- a/src/SSLv3.cc +++ b/src/SSLv3.cc @@ -383,84 +383,71 @@ void SSLv3_Interpreter::DeliverSSLv3_Record(SSLv3_HandshakeRecord* rec) case SSL3_1_CERTIFICATE: { - if ( rec->length >= 3 ) + const u_char* pData = rec->data; + uint32 certListLength = + uint32((pData[4] << 16) | + pData[5] << 8) | pData[6]; + + // Sum of all cert sizes has to match + // certListLength. + uint tempLength = 0; + uint certCount = 0; + while ( tempLength < certListLength ) { - const u_char* pData = rec->data; - uint32 certListLength = - uint32((pData[4] << 16) | - pData[5] << 8) | pData[6]; - - // Size consistency checks. - if ( certListLength + 3 != uint32(rec->length) ) + if ( tempLength + 3 <= certListLength ) { - if ( rec->endp->IsOrig() ) - Weird("SSLv3x: Corrupt length field in client certificate list!"); - else - Weird("SSLv3x: Corrupt length field in server certificate list!"); - return; - } - - // Sum of all cert sizes has to match - // certListLength. - uint tempLength = 0; - uint certCount = 0; - while ( tempLength < certListLength ) - { - if ( tempLength + 3 <= certListLength ) - { - ++certCount; - uint32 certLength = - uint32((pData[tempLength + 7] << 16) | pData[tempLength + 8] << 8) | pData[tempLength + 9]; - tempLength += certLength + 3; - } - else - { - Weird("SSLv3x: Corrupt length field in certificate list!"); - return; - } - } - - if ( tempLength > certListLength ) - { - Weird("SSLv3x: sum of size of certificates doesn't match size of certificate chain"); - return; - } - - SSL_InterpreterEndpoint* pEp = - (SSL_InterpreterEndpoint*) rec->endp; - - if ( certCount == 0 ) - { // we don't have a certificate... - if ( rec->endp->IsOrig() ) - { - Weird("SSLv3x: Client certificate is missing!"); - break; - } - else - { - Weird("SSLv3x: Server certificate is missing!"); - break; - } - } - - if ( certCount > 1 ) - { // we have a chain - analyzeCertificate(pEp, - rec->data + 7, - certListLength, 1, true); + ++certCount; + uint32 certLength = + uint32((pData[tempLength + 7] << 16) | pData[tempLength + 8] << 8) | pData[tempLength + 9]; + tempLength += certLength + 3; } else { - // We have a single certificate. - // FIXME. - analyzeCertificate(pEp, - rec->data + 10, - certListLength-3, 1, false); + Weird("SSLv3x: Corrupt length field in certificate list!"); + return; } + } + if ( tempLength > certListLength ) + { + Weird("SSLv3x: sum of size of certificates doesn't match size of certificate chain"); + return; + } + + SSL_InterpreterEndpoint* pEp = + (SSL_InterpreterEndpoint*) rec->endp; + + if ( certCount == 0 ) + { + // we don't have a certificate, but this is valid + // according to RFC2246 + if ( rec->endp->IsOrig() ) + { + Weird("SSLv3x: Client certificate is missing!"); + break; + } + else + { + Weird("SSLv3x: Server certificate is missing!"); + break; + } + } + + if ( certCount > 1 ) + { // we have a chain + analyzeCertificate(pEp, + rec->data + 7, + certListLength, 1, true); } else - Weird("SSLv3x: Certificate record too small!" ); + { + // We have a single certificate. + // FIXME. + analyzeCertificate(pEp, + rec->data + 10, + certListLength-3, 1, false); + } + break; } @@ -938,13 +925,15 @@ TableVal* SSLv3_Interpreter::analyzeCiphers(const SSLv3_Endpoint* s, int length, { int is_orig = (SSL_InterpreterEndpoint*) s == orig; - if ( length > ssl_max_cipherspec_size ) - { - if ( is_orig ) - Weird("SSLv3: Client has CipherSpecs > ssl_max_cipherspec_size"); - else - Weird("SSLv3: Server has CipherSpecs > ssl_max_cipherspec_size"); - } + // This probably shouldn't be a weird. This data should be passed to + // script layer and dealt with there as appropriate. + //if ( length > ssl_max_cipherspec_size ) + // { + // if ( is_orig ) + // Weird("SSLv3: Client has CipherSpecs > ssl_max_cipherspec_size"); + // else + // Weird("SSLv3: Server has CipherSpecs > ssl_max_cipherspec_size"); + // } const u_char* pCipher = data; SSL_CipherSpec* pCipherSuiteTemp = 0; @@ -1236,16 +1225,6 @@ SSLv3_HandshakeRecord::SSLv3_HandshakeRecord(const u_char* data, int len, uint16 version, SSLv3_Endpoint const* e) : SSLv3_Record(data, len, version, e) { - // Weird-check for minimum handshake length header. - if ( len < 4 ) - { - e->Interpreter()->Weird("SSLv3x: Handshake-header-length too small!"); - type = 255; - length = 0; - next = 0; - return; - } - // Don't analyze encrypted client handshake messages. if ( e->IsOrig() && ((SSLv3_Interpreter*) e->Interpreter())->change_cipher_client_seen && @@ -1270,7 +1249,10 @@ SSLv3_HandshakeRecord::SSLv3_HandshakeRecord(const u_char* data, int len, type = uint8(*(this->data)); length = ExtractInt24(data, len, 1); - if ( length + 4 < len ) + + if ( length == 0 ) // this is a special case to deal with 0 length certs + next = 0; + else if ( length + 4 < len ) next = new SSLv3_HandshakeRecord(data + length + 4, len - (length + 4), version, e); else if ( length + 4 > len ) @@ -1340,7 +1322,6 @@ int SSLv3_HandshakeRecord::checkClientHello() uint16 cipherSuiteLength = uint16(data[offset] << 8) | data[offset+1]; offset += (2 + cipherSuiteLength); - if ( cipherSuiteLength < 2 ) endp->Interpreter()->Weird("SSLv3x: CipherSuite length too small!"); @@ -1352,16 +1333,14 @@ int SSLv3_HandshakeRecord::checkClientHello() uint8 compressionMethodLength = uint8(data[offset]); offset += (1 + compressionMethodLength); - if ( compressionMethodLength < 1 ) endp->Interpreter()->Weird("SSLv3x: CompressionMethod length too small!"); - if ( offset != length ) + if ( offset < length ) { uint16 sslExtensionsLength = - uint16(data[offset] << 8 ) | data[offset+1]; + uint16(data[offset] << 8) | data[offset+1]; offset += 2; - if ( sslExtensionsLength < 4 ) endp->Interpreter()->Weird("SSLv3x: Extensions length too small!"); @@ -1391,16 +1370,33 @@ int SSLv3_HandshakeRecord::checkServerHello() version != SSLProxy_Analyzer::SSLv31 ) endp->Interpreter()->Weird("SSLv3x: Corrupt version information in Server hello!"); - uint8 sessionIDLength = uint8(data[38]); + uint16 offset = 38; + uint8 sessionIDLength = uint8(data[offset]); if ( sessionIDLength > 32 ) { endp->Interpreter()->Weird("SSLv3x: SessionID too long in Server hello!"); return 0; } - - if ( (sessionIDLength + 45) != length ) + offset += (1 + sessionIDLength); + + offset += 3; // account for cipher and compression method + if ( offset < length ) { - endp->Interpreter()->Weird("SSLv3x: Corrupt length fields in Server hello!"); + uint16 sslExtensionsLength = + uint16(data[offset] << 8) | data[offset+1]; + offset += 2; + if ( sslExtensionsLength < 4 ) + endp->Interpreter()->Weird("SSLv3x: Extensions length too small!"); + + // TODO: extract SSL extensions here + offset += sslExtensionsLength; + + if ( offset != length+4 ) + { + endp->Interpreter()->Weird("SSLv3x: Corrupt length fields in Server hello!"); + return 0; + } + return 0; }