Skip to content

Commit 8902afd

Browse files
SparkiDevSparkiDev
committed
TLS: more sanity checks on message order
Add more checks on message ordering for TLS 1.2 and below. Reformat code.
1 parent 6264c11 commit 8902afd

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
@@ -17609,7 +17609,9 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1760917609
if (ssl->msgsReceived.got_certificate_status ||
1761017610
ssl->msgsReceived.got_server_key_exchange ||
1761117611
ssl->msgsReceived.got_certificate_request ||
17612-
ssl->msgsReceived.got_server_hello_done) {
17612+
ssl->msgsReceived.got_server_hello_done ||
17613+
ssl->msgsReceived.got_change_cipher ||
17614+
ssl->msgsReceived.got_finished) {
1761317615
WOLFSSL_MSG("Cert received in wrong order");
1761417616
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1761517617
return OUT_OF_ORDER_E;
@@ -17650,19 +17652,21 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1765017652
return DUPLICATE_MSG_E;
1765117653
}
1765217654

17653-
if (ssl->msgsReceived.got_certificate == 0) {
17655+
if (!ssl->msgsReceived.got_certificate) {
1765417656
WOLFSSL_MSG("No Certificate before CertificateStatus");
1765517657
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1765617658
return OUT_OF_ORDER_E;
1765717659
}
17658-
if (ssl->msgsReceived.got_server_key_exchange != 0) {
17660+
if (ssl->msgsReceived.got_server_key_exchange) {
1765917661
WOLFSSL_MSG("CertificateStatus after ServerKeyExchange");
1766017662
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1766117663
return OUT_OF_ORDER_E;
1766217664
}
1766317665
if (ssl->msgsReceived.got_server_key_exchange ||
1766417666
ssl->msgsReceived.got_certificate_request ||
17665-
ssl->msgsReceived.got_server_hello_done) {
17667+
ssl->msgsReceived.got_server_hello_done ||
17668+
ssl->msgsReceived.got_change_cipher ||
17669+
ssl->msgsReceived.got_finished) {
1766617670
WOLFSSL_MSG("CertificateStatus received in wrong order");
1766717671
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1766817672
return OUT_OF_ORDER_E;
@@ -17686,13 +17690,25 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1768617690
WOLFSSL_ERROR_VERBOSE(DUPLICATE_MSG_E);
1768717691
return DUPLICATE_MSG_E;
1768817692
}
17689-
if (ssl->msgsReceived.got_server_hello == 0) {
17693+
if (!ssl->msgsReceived.got_server_hello) {
1769017694
WOLFSSL_MSG("No ServerHello before ServerKeyExchange");
1769117695
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1769217696
return OUT_OF_ORDER_E;
1769317697
}
17698+
if (!ssl->msgsReceived.got_certificate) {
17699+
if (ssl->specs.kea != psk_kea &&
17700+
ssl->specs.kea != dhe_psk_kea &&
17701+
ssl->specs.kea != ecdhe_psk_kea &&
17702+
!ssl->options.usingAnon_cipher) {
17703+
WOLFSSL_MSG("No Certificate before ServerKeyExchange");
17704+
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17705+
return OUT_OF_ORDER_E;
17706+
}
17707+
}
1769417708
if (ssl->msgsReceived.got_certificate_request ||
17695-
ssl->msgsReceived.got_server_hello_done) {
17709+
ssl->msgsReceived.got_server_hello_done ||
17710+
ssl->msgsReceived.got_change_cipher ||
17711+
ssl->msgsReceived.got_finished) {
1769617712
WOLFSSL_MSG("ServerKeyExchange received in wrong order");
1769717713
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1769817714
return OUT_OF_ORDER_E;
@@ -17716,11 +17732,16 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1771617732
WOLFSSL_ERROR_VERBOSE(DUPLICATE_MSG_E);
1771717733
return DUPLICATE_MSG_E;
1771817734
}
17719-
if (ssl->msgsReceived.got_server_hello == 0) {
17735+
if (!ssl->msgsReceived.got_server_hello) {
1772017736
WOLFSSL_MSG("No ServerHello before CertificateRequest");
1772117737
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1772217738
return OUT_OF_ORDER_E;
1772317739
}
17740+
if (!ssl->msgsReceived.got_certificate) {
17741+
WOLFSSL_MSG("No Certificate before CertificateRequest");
17742+
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17743+
return OUT_OF_ORDER_E;
17744+
}
1772417745
if (!ssl->options.resuming && ssl->specs.kea != rsa_kea &&
1772517746
(ssl->specs.kea != ecc_diffie_hellman_kea ||
1772617747
!ssl->specs.static_ecdh) &&
@@ -17730,12 +17751,9 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1773017751
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1773117752
return OUT_OF_ORDER_E;
1773217753
}
17733-
if (!ssl->msgsReceived.got_certificate) {
17734-
WOLFSSL_MSG("No Certificate before CertificateRequest");
17735-
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17736-
return OUT_OF_ORDER_E;
17737-
}
17738-
if (ssl->msgsReceived.got_server_hello_done) {
17754+
if (ssl->msgsReceived.got_server_hello_done ||
17755+
ssl->msgsReceived.got_change_cipher ||
17756+
ssl->msgsReceived.got_finished) {
1773917757
WOLFSSL_MSG("CertificateRequest received in wrong order");
1774017758
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1774117759
return OUT_OF_ORDER_E;
@@ -17761,7 +17779,7 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1776117779
}
1776217780
ssl->msgsReceived.got_server_hello_done = 1;
1776317781

17764-
if (ssl->msgsReceived.got_certificate == 0) {
17782+
if (!ssl->msgsReceived.got_certificate) {
1776517783
if (ssl->specs.kea == psk_kea ||
1776617784
ssl->specs.kea == dhe_psk_kea ||
1776717785
ssl->specs.kea == ecdhe_psk_kea ||
@@ -17774,7 +17792,7 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1777417792
return OUT_OF_ORDER_E;
1777517793
}
1777617794
}
17777-
if (ssl->msgsReceived.got_server_key_exchange == 0) {
17795+
if (!ssl->msgsReceived.got_server_key_exchange) {
1777817796
int pskNoServerHint = 0; /* not required in this case */
1777917797

1778017798
#ifndef NO_PSK
@@ -17796,7 +17814,7 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1779617814
}
1779717815
#if defined(HAVE_CERTIFICATE_STATUS_REQUEST) || \
1779817816
defined(HAVE_CERTIFICATE_STATUS_REQUEST_V2)
17799-
if (ssl->msgsReceived.got_certificate_status == 0) {
17817+
if (!ssl->msgsReceived.got_certificate_status) {
1780017818
int csrRet = 0;
1780117819
#ifdef HAVE_CERTIFICATE_STATUS_REQUEST
1780217820
if (csrRet == 0 && ssl->status_request) {
@@ -17842,6 +17860,12 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1784217860
}
1784317861
}
1784417862
#endif
17863+
if (ssl->msgsReceived.got_change_cipher ||
17864+
ssl->msgsReceived.got_finished) {
17865+
WOLFSSL_MSG("ServerHelloDone received in wrong order");
17866+
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17867+
return OUT_OF_ORDER_E;
17868+
}
1784517869
break;
1784617870
#endif
1784717871

@@ -17859,7 +17883,12 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1785917883
WOLFSSL_ERROR_VERBOSE(DUPLICATE_MSG_E);
1786017884
return DUPLICATE_MSG_E;
1786117885
}
17862-
if ( ssl->msgsReceived.got_certificate == 0) {
17886+
if (!ssl->msgsReceived.got_client_key_exchange) {
17887+
WOLFSSL_MSG("No ClientKeyExchange before CertVerify");
17888+
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17889+
return OUT_OF_ORDER_E;
17890+
}
17891+
if (!ssl->msgsReceived.got_certificate) {
1786317892
WOLFSSL_MSG("No Cert before CertVerify");
1786417893
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1786517894
return OUT_OF_ORDER_E;
@@ -17888,7 +17917,7 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1788817917
WOLFSSL_ERROR_VERBOSE(DUPLICATE_MSG_E);
1788917918
return DUPLICATE_MSG_E;
1789017919
}
17891-
if (ssl->msgsReceived.got_client_hello == 0) {
17920+
if (!ssl->msgsReceived.got_client_hello) {
1789217921
WOLFSSL_MSG("No ClientHello before ClientKeyExchange");
1789317922
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1789417923
return OUT_OF_ORDER_E;
@@ -17919,7 +17948,7 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1791917948
}
1792017949
}
1792117950
#endif
17922-
if (ssl->msgsReceived.got_change_cipher == 0) {
17951+
if (!ssl->msgsReceived.got_change_cipher) {
1792317952
WOLFSSL_MSG("Finished received before ChangeCipher");
1792417953
WOLFSSL_ERROR_VERBOSE(NO_CHANGE_CIPHER_E);
1792517954
return NO_CHANGE_CIPHER_E;
@@ -17940,62 +17969,63 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type)
1794017969

1794117970
#ifndef NO_WOLFSSL_CLIENT
1794217971
if (ssl->options.side == WOLFSSL_CLIENT_END) {
17972+
if (!ssl->msgsReceived.got_server_hello) {
17973+
WOLFSSL_MSG("ChangeCipherSpec received in wrong order");
17974+
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17975+
return OUT_OF_ORDER_E;
17976+
}
1794317977
if (!ssl->options.resuming) {
17944-
if (ssl->msgsReceived.got_server_hello_done == 0) {
17978+
if (!ssl->msgsReceived.got_server_hello_done) {
1794517979
WOLFSSL_MSG("No ServerHelloDone before ChangeCipher");
1794617980
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1794717981
return OUT_OF_ORDER_E;
1794817982
}
1794917983
}
17950-
else {
17951-
if (ssl->msgsReceived.got_server_hello == 0) {
17952-
WOLFSSL_MSG("No ServerHello before ChangeCipher on "
17953-
"Resume");
17954-
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17955-
return OUT_OF_ORDER_E;
17956-
}
17984+
#ifdef HAVE_SESSION_TICKET
17985+
if (ssl->expect_session_ticket) {
17986+
WOLFSSL_MSG("Expected session ticket missing");
17987+
#ifdef WOLFSSL_DTLS
17988+
if (ssl->options.dtls) {
17989+
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17990+
return OUT_OF_ORDER_E;
17991+
}
17992+
#endif
17993+
WOLFSSL_ERROR_VERBOSE(SESSION_TICKET_EXPECT_E);
17994+
return SESSION_TICKET_EXPECT_E;
1795717995
}
17958-
#ifdef HAVE_SESSION_TICKET
17959-
if (ssl->expect_session_ticket) {
17960-
WOLFSSL_MSG("Expected session ticket missing");
17961-
#ifdef WOLFSSL_DTLS
17962-
if (ssl->options.dtls) {
17963-
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17964-
return OUT_OF_ORDER_E;
17965-
}
17966-
#endif
17967-
WOLFSSL_ERROR_VERBOSE(SESSION_TICKET_EXPECT_E);
17968-
return SESSION_TICKET_EXPECT_E;
17969-
}
17970-
#endif
17996+
#endif
1797117997
}
1797217998
#endif
1797317999
#ifndef NO_WOLFSSL_SERVER
1797418000
if (ssl->options.side == WOLFSSL_SERVER_END) {
18001+
if (!ssl->msgsReceived.got_client_hello) {
18002+
WOLFSSL_MSG("ChangeCipherSpec received in wrong order");
18003+
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
18004+
return OUT_OF_ORDER_E;
18005+
}
1797518006
if (!ssl->options.resuming &&
17976-
ssl->msgsReceived.got_client_key_exchange == 0) {
18007+
!ssl->msgsReceived.got_client_key_exchange) {
1797718008
WOLFSSL_MSG("No ClientKeyExchange before ChangeCipher");
1797818009
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
1797918010
return OUT_OF_ORDER_E;
1798018011
}
17981-
#ifndef NO_CERTS
17982-
if (ssl->options.verifyPeer &&
18012+
#ifndef NO_CERTS
18013+
if (ssl->options.verifyPeer &&
1798318014
ssl->options.havePeerCert) {
17984-
17985-
if (!ssl->options.havePeerVerify ||
17986-
!ssl->msgsReceived.got_certificate_verify) {
17987-
WOLFSSL_MSG("client didn't send cert verify");
17988-
#ifdef WOLFSSL_DTLS
17989-
if (ssl->options.dtls) {
17990-
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
17991-
return OUT_OF_ORDER_E;
17992-
}
17993-
#endif
17994-
WOLFSSL_ERROR_VERBOSE(NO_PEER_VERIFY);
17995-
return NO_PEER_VERIFY;
17996-
}
18015+
if (!ssl->options.havePeerVerify ||
18016+
!ssl->msgsReceived.got_certificate_verify) {
18017+
WOLFSSL_MSG("client didn't send cert verify");
18018+
#ifdef WOLFSSL_DTLS
18019+
if (ssl->options.dtls) {
18020+
WOLFSSL_ERROR_VERBOSE(OUT_OF_ORDER_E);
18021+
return OUT_OF_ORDER_E;
18022+
}
18023+
#endif
18024+
WOLFSSL_ERROR_VERBOSE(NO_PEER_VERIFY);
18025+
return NO_PEER_VERIFY;
1799718026
}
17998-
#endif
18027+
}
18028+
#endif
1799918029
}
1800018030
#endif /* !NO_WOLFSSL_SERVER */
1800118031
if (ssl->options.dtls)

0 commit comments

Comments
 (0)