Skip to content

Commit e4e79dd

Browse files
dgarskedgarske
authored
Merge pull request #9694 from SparkiDev/tls_msg_sanity_fix
TLS: more sanity checks on message order
2 parents 11ddec3 + 8902afd commit e4e79dd

File tree

1 file changed

+87
-57
lines changed

1 file changed

+87
-57
lines changed

‎src/internal.c‎

Lines changed: 87 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -17604,7 +17604,9 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1760417604
if (ssl->msgsReceived.got_certificate_status ||
1760517605
ssl->msgsReceived.got_server_key_exchange ||
1760617606
ssl->msgsReceived.got_certificate_request ||
17607-
ssl->msgsReceived.got_server_hello_done) {
17607+
ssl->msgsReceived.got_server_hello_done ||
17608+
ssl->msgsReceived.got_change_cipher ||
17609+
ssl->msgsReceived.got_finished) {
1760817610
WOLFSSL_MSG("Cert received in wrong order");
1760917611
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1761017612
return OUT_OF_ORDER_E;
@@ -17645,19 +17647,21 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1764517647
return DUPLICATE_MSG_E;
1764617648
}
1764717649

17648-
if (ssl->msgsReceived.got_certificate == 0) {
17650+
if (!ssl->msgsReceived.got_certificate) {
1764917651
WOLFSSL_MSG("No Certificate before CertificateStatus");
1765017652
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1765117653
return OUT_OF_ORDER_E;
1765217654
}
17653-
if (ssl->msgsReceived.got_server_key_exchange != 0) {
17655+
if (ssl->msgsReceived.got_server_key_exchange) {
1765417656
WOLFSSL_MSG("CertificateStatus after ServerKeyExchange");
1765517657
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1765617658
return OUT_OF_ORDER_E;
1765717659
}
1765817660
if (ssl->msgsReceived.got_server_key_exchange ||
1765917661
ssl->msgsReceived.got_certificate_request ||
17660-
ssl->msgsReceived.got_server_hello_done) {
17662+
ssl->msgsReceived.got_server_hello_done ||
17663+
ssl->msgsReceived.got_change_cipher ||
17664+
ssl->msgsReceived.got_finished) {
1766117665
WOLFSSL_MSG("CertificateStatus received in wrong order");
1766217666
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1766317667
return OUT_OF_ORDER_E;
@@ -17681,13 +17685,25 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1768117685
WOLFSSL_ERROR_VERBOSE(DUPLICATE_MSG_E);
1768217686
return DUPLICATE_MSG_E;
1768317687
}
17684-
if (ssl->msgsReceived.got_server_hello == 0) {
17688+
if (!ssl->msgsReceived.got_server_hello) {
1768517689
WOLFSSL_MSG("No ServerHello before ServerKeyExchange");
1768617690
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1768717691
return OUT_OF_ORDER_E;
1768817692
}
17693+
if (!ssl->msgsReceived.got_certificate) {
17694+
if (ssl->specs.kea != psk_kea &&
17695+
ssl->specs.kea != dhe_psk_kea &&
17696+
ssl->specs.kea != ecdhe_psk_kea &&
17697+
!ssl->options.usingAnon_cipher) {
17698+
WOLFSSL_MSG("No Certificate before ServerKeyExchange");
17699+
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17700+
return OUT_OF_ORDER_E;
17701+
}
17702+
}
1768917703
if (ssl->msgsReceived.got_certificate_request ||
17690-
ssl->msgsReceived.got_server_hello_done) {
17704+
ssl->msgsReceived.got_server_hello_done ||
17705+
ssl->msgsReceived.got_change_cipher ||
17706+
ssl->msgsReceived.got_finished) {
1769117707
WOLFSSL_MSG("ServerKeyExchange received in wrong order");
1769217708
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1769317709
return OUT_OF_ORDER_E;
@@ -17711,11 +17727,16 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1771117727
WOLFSSL_ERROR_VERBOSE(DUPLICATE_MSG_E);
1771217728
return DUPLICATE_MSG_E;
1771317729
}
17714-
if (ssl->msgsReceived.got_server_hello == 0) {
17730+
if (!ssl->msgsReceived.got_server_hello) {
1771517731
WOLFSSL_MSG("No ServerHello before CertificateRequest");
1771617732
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1771717733
return OUT_OF_ORDER_E;
1771817734
}
17735+
if (!ssl->msgsReceived.got_certificate) {
17736+
WOLFSSL_MSG("No Certificate before CertificateRequest");
17737+
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17738+
return OUT_OF_ORDER_E;
17739+
}
1771917740
if (!ssl->options.resuming && ssl->specs.kea != rsa_kea &&
1772017741
(ssl->specs.kea != ecc_diffie_hellman_kea ||
1772117742
!ssl->specs.static_ecdh) &&
@@ -17725,12 +17746,9 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1772517746
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1772617747
return OUT_OF_ORDER_E;
1772717748
}
17728-
if (!ssl->msgsReceived.got_certificate) {
17729-
WOLFSSL_MSG("No Certificate before CertificateRequest");
17730-
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17731-
return OUT_OF_ORDER_E;
17732-
}
17733-
if (ssl->msgsReceived.got_server_hello_done) {
17749+
if (ssl->msgsReceived.got_server_hello_done ||
17750+
ssl->msgsReceived.got_change_cipher ||
17751+
ssl->msgsReceived.got_finished) {
1773417752
WOLFSSL_MSG("CertificateRequest received in wrong order");
1773517753
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1773617754
return OUT_OF_ORDER_E;
@@ -17756,7 +17774,7 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1775617774
}
1775717775
ssl->msgsReceived.got_server_hello_done = 1;
1775817776

17759-
if (ssl->msgsReceived.got_certificate == 0) {
17777+
if (!ssl->msgsReceived.got_certificate) {
1776017778
if (ssl->specs.kea == psk_kea ||
1776117779
ssl->specs.kea == dhe_psk_kea ||
1776217780
ssl->specs.kea == ecdhe_psk_kea ||
@@ -17769,7 +17787,7 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1776917787
return OUT_OF_ORDER_E;
1777017788
}
1777117789
}
17772-
if (ssl->msgsReceived.got_server_key_exchange == 0) {
17790+
if (!ssl->msgsReceived.got_server_key_exchange) {
1777317791
int pskNoServerHint = 0; /* not required in this case */
1777417792

1777517793
#ifndef NO_PSK
@@ -17791,7 +17809,7 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1779117809
}
1779217810
#if defined(HAVE_CERTIFICATE_STATUS_REQUEST) || \
1779317811
defined(HAVE_CERTIFICATE_STATUS_REQUEST_V2)
17794-
if (ssl->msgsReceived.got_certificate_status == 0) {
17812+
if (!ssl->msgsReceived.got_certificate_status) {
1779517813
int csrRet = 0;
1779617814
#ifdef HAVE_CERTIFICATE_STATUS_REQUEST
1779717815
if (csrRet == 0 && ssl->status_request) {
@@ -17837,6 +17855,12 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1783717855
}
1783817856
}
1783917857
#endif
17858+
if (ssl->msgsReceived.got_change_cipher ||
17859+
ssl->msgsReceived.got_finished) {
17860+
WOLFSSL_MSG("ServerHelloDone received in wrong order");
17861+
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17862+
return OUT_OF_ORDER_E;
17863+
}
1784017864
break;
1784117865
#endif
1784217866

@@ -17854,7 +17878,12 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1785417878
WOLFSSL_ERROR_VERBOSE(DUPLICATE_MSG_E);
1785517879
return DUPLICATE_MSG_E;
1785617880
}
17857-
if ( ssl->msgsReceived.got_certificate == 0) {
17881+
if (!ssl->msgsReceived.got_client_key_exchange) {
17882+
WOLFSSL_MSG("No ClientKeyExchange before CertVerify");
17883+
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17884+
return OUT_OF_ORDER_E;
17885+
}
17886+
if (!ssl->msgsReceived.got_certificate) {
1785817887
WOLFSSL_MSG("No Cert before CertVerify");
1785917888
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1786017889
return OUT_OF_ORDER_E;
@@ -17883,7 +17912,7 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1788317912
WOLFSSL_ERROR_VERBOSE(DUPLICATE_MSG_E);
1788417913
return DUPLICATE_MSG_E;
1788517914
}
17886-
if (ssl->msgsReceived.got_client_hello == 0) {
17915+
if (!ssl->msgsReceived.got_client_hello) {
1788717916
WOLFSSL_MSG("No ClientHello before ClientKeyExchange");
1788817917
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1788917918
return OUT_OF_ORDER_E;
@@ -17914,7 +17943,7 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1791417943
}
1791517944
}
1791617945
#endif
17917-
if (ssl->msgsReceived.got_change_cipher == 0) {
17946+
if (!ssl->msgsReceived.got_change_cipher) {
1791817947
WOLFSSL_MSG("Finished received before ChangeCipher");
1791917948
WOLFSSL_ERROR_VERBOSE(NO_CHANGE_CIPHER_E);
1792017949
return NO_CHANGE_CIPHER_E;
@@ -17935,62 +17964,63 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1793517964

1793617965
#ifndef NO_WOLFSSL_CLIENT
1793717966
if (ssl->options.side == WOLFSSL_CLIENT_END) {
17967+
if (!ssl->msgsReceived.got_server_hello) {
17968+
WOLFSSL_MSG("ChangeCipherSpec received in wrong order");
17969+
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17970+
return OUT_OF_ORDER_E;
17971+
}
1793817972
if (!ssl->options.resuming) {
17939-
if (ssl->msgsReceived.got_server_hello_done == 0) {
17973+
if (!ssl->msgsReceived.got_server_hello_done) {
1794017974
WOLFSSL_MSG("No ServerHelloDone before ChangeCipher");
1794117975
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1794217976
return OUT_OF_ORDER_E;
1794317977
}
1794417978
}
17945-
else {
17946-
if (ssl->msgsReceived.got_server_hello == 0) {
17947-
WOLFSSL_MSG("No ServerHello before ChangeCipher on "
17948-
"Resume");
17949-
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17950-
return OUT_OF_ORDER_E;
17951-
}
17979+
#ifdef HAVE_SESSION_TICKET
17980+
if (ssl->expect_session_ticket) {
17981+
WOLFSSL_MSG("Expected session ticket missing");
17982+
#ifdef WOLFSSL_DTLS
17983+
if (ssl->options.dtls) {
17984+
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17985+
return OUT_OF_ORDER_E;
17986+
}
17987+
#endif
17988+
WOLFSSL_ERROR_VERBOSE(SESSION_TICKET_EXPECT_E);
17989+
return SESSION_TICKET_EXPECT_E;
1795217990
}
17953-
#ifdef HAVE_SESSION_TICKET
17954-
if (ssl->expect_session_ticket) {
17955-
WOLFSSL_MSG("Expected session ticket missing");
17956-
#ifdef WOLFSSL_DTLS
17957-
if (ssl->options.dtls) {
17958-
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17959-
return OUT_OF_ORDER_E;
17960-
}
17961-
#endif
17962-
WOLFSSL_ERROR_VERBOSE(SESSION_TICKET_EXPECT_E);
17963-
return SESSION_TICKET_EXPECT_E;
17964-
}
17965-
#endif
17991+
#endif
1796617992
}
1796717993
#endif
1796817994
#ifndef NO_WOLFSSL_SERVER
1796917995
if (ssl->options.side == WOLFSSL_SERVER_END) {
17996+
if (!ssl->msgsReceived.got_client_hello) {
17997+
WOLFSSL_MSG("ChangeCipherSpec received in wrong order");
17998+
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17999+
return OUT_OF_ORDER_E;
18000+
}
1797018001
if (!ssl->options.resuming &&
17971-
ssl->msgsReceived.got_client_key_exchange == 0) {
18002+
!ssl->msgsReceived.got_client_key_exchange) {
1797218003
WOLFSSL_MSG("No ClientKeyExchange before ChangeCipher");
1797318004
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1797418005
return OUT_OF_ORDER_E;
1797518006
}
17976-
#ifndef NO_CERTS
17977-
if (ssl->options.verifyPeer &&
18007+
#ifndef NO_CERTS
18008+
if (ssl->options.verifyPeer &&
1797818009
ssl->options.havePeerCert) {
17979-
17980-
if (!ssl->options.havePeerVerify ||
17981-
!ssl->msgsReceived.got_certificate_verify) {
17982-
WOLFSSL_MSG("client didn't send cert verify");
17983-
#ifdef WOLFSSL_DTLS
17984-
if (ssl->options.dtls) {
17985-
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17986-
return OUT_OF_ORDER_E;
17987-
}
17988-
#endif
17989-
WOLFSSL_ERROR_VERBOSE(NO_PEER_VERIFY);
17990-
return NO_PEER_VERIFY;
17991-
}
18010+
if (!ssl->options.havePeerVerify ||
18011+
!ssl->msgsReceived.got_certificate_verify) {
18012+
WOLFSSL_MSG("client didn't send cert verify");
18013+
#ifdef WOLFSSL_DTLS
18014+
if (ssl->options.dtls) {
18015+
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
18016+
return OUT_OF_ORDER_E;
18017+
}
18018+
#endif
18019+
WOLFSSL_ERROR_VERBOSE(NO_PEER_VERIFY);
18020+
return NO_PEER_VERIFY;
1799218021
}
17993-
#endif
18022+
}
18023+
#endif
1799418024
}
1799518025
#endif /* !NO_WOLFSSL_SERVER */
1799618026
if (ssl->options.dtls)

0 commit comments

Comments
 (0)