From 1acf906612883716e417e25bc098be64c311d448 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Fri, 25 Jun 2021 10:02:09 +0200 Subject: [PATCH] Code review changes --- src/internal.c | 30 ++++++++++++++++----------- src/ssl.c | 46 +++++++++++++---------------------------- tests/api.c | 2 +- wolfssl/ssl.h | 27 ++++++++++++------------ wolfssl/wolfcrypt/asn.h | 4 ++-- 5 files changed, 48 insertions(+), 61 deletions(-) diff --git a/src/internal.c b/src/internal.c index 13b62bdaa..a460a513a 100644 --- a/src/internal.c +++ b/src/internal.c @@ -11407,7 +11407,7 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) if (args->totalCerts >= MAX_CHAIN_DEPTH) { - if (!ssl->peerVerifyRet) /* Return first cert error here */ + if (ssl->peerVerifyRet == 0) /* Return first cert error here */ ssl->peerVerifyRet = X509_V_ERR_CERT_CHAIN_TOO_LONG; ret = MAX_CHAIN_ERROR; WOLFSSL_MSG("Too many certs for MAX_CHAIN_DEPTH"); @@ -11625,7 +11625,7 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, else { WOLFSSL_MSG("Failed to verify CA from chain"); #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) - if (!ssl->peerVerifyRet) /* Return first cert error here */ + if (ssl->peerVerifyRet == 0) /* Return first cert error here */ ssl->peerVerifyRet = X509_V_ERR_INVALID_CA; #endif } @@ -11685,7 +11685,7 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, /* extend the limit "+1" until reaching * an ultimately trusted issuer.*/ args->count > (ssl->verifyDepth + 1)) { - if (!ssl->peerVerifyRet) /* Return first cert error here */ + if (ssl->peerVerifyRet == 0) /* Return first cert error here */ ssl->peerVerifyRet = X509_V_ERR_CERT_CHAIN_TOO_LONG; ret = MAX_CHAIN_ERROR; } @@ -11823,7 +11823,7 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, if (ret == 0) { WOLFSSL_MSG("Verified Peer's cert"); #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) - if (!ssl->peerVerifyRet) /* Return first cert error here */ + if (ssl->peerVerifyRet == 0) /* Return first cert error here */ ssl->peerVerifyRet = X509_V_OK; #endif #if defined(SESSION_CERTS) && defined(WOLFSSL_ALT_CERT_CHAINS) @@ -11863,7 +11863,7 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, DoCertFatalAlert(ssl, ret); #endif #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) - if (!ssl->peerVerifyRet) /* Return first cert error here */ + if (ssl->peerVerifyRet == 0) /* Return first cert error here */ ssl->peerVerifyRet = X509_V_ERR_CERT_REJECTED; #endif args->fatal = 1; @@ -11871,14 +11871,15 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, else { WOLFSSL_MSG("Failed to verify Peer's cert"); #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) - if (!ssl->peerVerifyRet) { /* Return first cert error here */ + if (ssl->peerVerifyRet == 0) { /* Return first cert error here */ if (ret == ASN_BEFORE_DATE_E) ssl->peerVerifyRet = X509_V_ERR_CERT_NOT_YET_VALID; else if (ret == ASN_AFTER_DATE_E) ssl->peerVerifyRet = X509_V_ERR_CERT_HAS_EXPIRED; - else + else { ssl->peerVerifyRet = X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE; + } } #endif if (ssl->verifyCallback) { @@ -12003,11 +12004,13 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, WOLFSSL_MSG("\tOCSP Lookup not ok"); args->fatal = 0; #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) - if (!ssl->peerVerifyRet) /* Return first cert error here */ + if (ssl->peerVerifyRet == 0) { + /* Return first cert error here */ ssl->peerVerifyRet = ret == OCSP_CERT_REVOKED ? X509_V_ERR_CERT_REVOKED : X509_V_ERR_CERT_REJECTED; + } #endif } } @@ -12026,11 +12029,13 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, WOLFSSL_MSG("\tCRL check not ok"); args->fatal = 0; #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) - if (!ssl->peerVerifyRet) /* Return first cert error here */ + if (ssl->peerVerifyRet == 0) { + /* Return first cert error here */ ssl->peerVerifyRet = ret == CRL_CERT_REVOKED ? X509_V_ERR_CERT_REVOKED : X509_V_ERR_CERT_REJECTED;; + } #endif } } @@ -12128,7 +12133,7 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, ssl->error = ret; #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) SendAlert(ssl, alert_fatal, bad_certificate); - if (!ssl->peerVerifyRet) /* Return first cert error here */ + if (ssl->peerVerifyRet == 0) /* Return first cert error here */ ssl->peerVerifyRet = X509_V_ERR_CERT_REJECTED; #endif goto exit_ppc; @@ -12472,7 +12477,7 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, * we hit the limit, with X509_V_ERR_CERT_CHAIN_TOO_LONG */ if (args->untrustedDepth > (ssl->options.verifyDepth + 1)) { - if (!ssl->peerVerifyRet) /* Return first cert error here */ + if (ssl->peerVerifyRet == 0) /* Return first cert error here */ ssl->peerVerifyRet = X509_V_ERR_CERT_CHAIN_TOO_LONG; ret = MAX_CHAIN_ERROR; } @@ -18199,10 +18204,11 @@ int SendCertificateRequest(WOLFSSL* ssl) byte seq[MAX_SEQ_SZ]; WOLFSSL_X509_NAME* name = names->data.name; - if (name != NULL) + if (name != NULL) { /* 16-bit length | SEQ | Len | DER of name */ dnLen += OPAQUE16_LEN + SetSequence(name->rawLen, seq) + name->rawLen; + } names = names->next; } reqSz += dnLen; diff --git a/src/ssl.c b/src/ssl.c index 13b4d9970..94d8ca326 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -438,19 +438,17 @@ WOLFSSL_CTX* wolfSSL_CTX_new_ex(WOLFSSL_METHOD* method, void* heap) #ifdef OPENSSL_COMPATIBLE_DEFAULTS if (ctx) { + wolfSSL_CTX_set_verify(ctx, SSL_VERIFY_NONE, NULL); if (wolfSSL_CTX_set_min_proto_version(ctx, SSL3_VERSION) != WOLFSSL_SUCCESS || #ifdef HAVE_ANON wolfSSL_CTX_allow_anon_cipher(ctx) != WOLFSSL_SUCCESS || #endif - wolfSSL_CTX_set_group_messages(ctx) != WOLFSSL_SUCCESS - ) { + wolfSSL_CTX_set_group_messages(ctx) != WOLFSSL_SUCCESS) { WOLFSSL_MSG("Setting OpenSSL CTX defaults failed"); wolfSSL_CTX_free(ctx); ctx = NULL; } - else - wolfSSL_CTX_set_verify(ctx, SSL_VERIFY_NONE, NULL); } #endif @@ -11125,27 +11123,14 @@ void wolfSSL_set_verify(WOLFSSL* ssl, int mode, VerifyCallback vc) if (ssl == NULL) return; - ssl->options.verifyPeer = 0; - ssl->options.verifyNone = 0; - ssl->options.failNoCert = 0; - ssl->options.failNoCertxPSK = 0; - - if (mode != WOLFSSL_VERIFY_DEFAULT) { - if (mode == WOLFSSL_VERIFY_NONE) { - ssl->options.verifyNone = 1; - } - else { - if (mode & WOLFSSL_VERIFY_PEER) { - ssl->options.verifyPeer = 1; - } - if (mode & WOLFSSL_VERIFY_FAIL_EXCEPT_PSK) { - ssl->options.failNoCertxPSK = 1; - } - if (mode & WOLFSSL_VERIFY_FAIL_IF_NO_PEER_CERT) { - ssl->options.failNoCert = 1; - } - } - } + /* Special case for verifyNone since WOLFSSL_VERIFY_NONE == 0 */ + ssl->options.verifyNone = mode == WOLFSSL_VERIFY_NONE; + ssl->options.verifyPeer = (mode & WOLFSSL_VERIFY_PEER) + == WOLFSSL_VERIFY_PEER; + ssl->options.failNoCert = (mode & WOLFSSL_VERIFY_FAIL_IF_NO_PEER_CERT) + == WOLFSSL_VERIFY_FAIL_IF_NO_PEER_CERT; + ssl->options.failNoCertxPSK = (mode & WOLFSSL_VERIFY_FAIL_EXCEPT_PSK) + == WOLFSSL_VERIFY_FAIL_EXCEPT_PSK; ssl->verifyCallback = vc; } @@ -46410,10 +46395,9 @@ void wolfSSL_THREADID_set_numeric(void* id, unsigned long val) WOLFSSL_X509_LOOKUP_TYPE wolfSSL_X509_OBJECT_get_type( const WOLFSSL_X509_OBJECT* obj) { - if (obj != NULL) - return obj->type; - else + if (obj == NULL) return WOLFSSL_X509_LU_NONE; + return obj->type; } WOLFSSL_X509_OBJECT* wolfSSL_X509_OBJECT_new(void) @@ -46458,16 +46442,14 @@ WOLFSSL_X509 *wolfSSL_X509_OBJECT_get0_X509(const WOLFSSL_X509_OBJECT *obj) { if (obj != NULL && obj->type == WOLFSSL_X509_LU_X509) return obj->data.x509; - else - return NULL; + return NULL; } WOLFSSL_X509_CRL *wolfSSL_X509_OBJECT_get0_X509_CRL(WOLFSSL_X509_OBJECT *obj) { if (obj != NULL && obj->type == WOLFSSL_X509_LU_CRL) return obj->data.crl; - else - return NULL; + return NULL; } #endif /* OPENSSL_ALL || (OPENSSL_EXTRA && (HAVE_STUNNEL || WOLFSSL_NGINX || HAVE_LIGHTY)) */ diff --git a/tests/api.c b/tests/api.c index ab349ae50..a63ef10d5 100644 --- a/tests/api.c +++ b/tests/api.c @@ -39622,7 +39622,7 @@ static void test_wolfSSL_i2d_PrivateKey(void) static void test_wolfSSL_OCSP_id_get0_info(void) { #if (defined(OPENSSL_ALL) || defined(WOLFSSL_HAPROXY)) && defined(HAVE_OCSP) && \ - !defined(NO_FILESYSTEM) || !defined(NO_RSA) + !defined(NO_FILESYSTEM) && !defined(NO_RSA) X509* cert; X509* issuer; OCSP_CERTID* id; diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index 49ff34686..c79d59f84 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -1924,7 +1924,7 @@ enum { SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS = 0x00100000, SSL_OP_NO_QUERY_MTU = 0x00200000, SSL_OP_COOKIE_EXCHANGE = 0x00400000, - WOLFSSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION = 0x00800000, + SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION = 0x00800000, SSL_OP_SINGLE_ECDH_USE = 0x01000000, SSL_OP_CIPHER_SERVER_PREFERENCE = 0x02000000, WOLFSSL_OP_NO_TLSv1_1 = 0x04000000, @@ -1957,7 +1957,6 @@ enum { #if !(!defined(WOLFSSL_TLS13) && defined(WOLFSSL_APACHE_HTTPD)) /* apache uses this to determine if TLS 1.3 is enabled */ #define SSL_OP_NO_TLSv1_3 WOLFSSL_OP_NO_TLSv1_3 #endif -#define SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION WOLFSSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION #define SSL_OP_NO_SSL_MASK (SSL_OP_NO_SSLv3 | SSL_OP_NO_TLSv1 | \ SSL_OP_NO_TLSv1_1 | SSL_OP_NO_TLSv1_2 | SSL_OP_NO_TLSv1_3) @@ -2036,14 +2035,14 @@ enum { X509_V_ERR_ERROR_IN_CERT_NOT_BEFORE_FIELD = 20, X509_V_ERR_ERROR_IN_CERT_NOT_AFTER_FIELD = 22, X509_V_ERR_CERT_REVOKED = 23, - X509_V_ERR_CERT_REJECTED, + X509_V_ERR_CERT_REJECTED = 24, /* Required for Nginx */ - X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT, - X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN, - X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY, - X509_V_ERR_CERT_UNTRUSTED, - X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE, - X509_V_ERR_SUBJECT_ISSUER_MISMATCH, + X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT = 25, + X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN = 26, + X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY = 27, + X509_V_ERR_CERT_UNTRUSTED = 28, + X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE = 29, + X509_V_ERR_SUBJECT_ISSUER_MISMATCH = 30, /* additional X509_V_ERR_* enums not used in wolfSSL */ X509_V_ERR_UNABLE_TO_GET_CRL, X509_V_ERR_UNABLE_TO_DECRYPT_CERT_SIGNATURE, @@ -2180,12 +2179,12 @@ enum { /* ssl Constants */ WOLFSSL_FILETYPE_DEFAULT = 2, /* ASN1 */ WOLFSSL_FILETYPE_RAW = 3, /* NTRU raw key blob */ - WOLFSSL_VERIFY_DEFAULT = -1, WOLFSSL_VERIFY_NONE = 0, - WOLFSSL_VERIFY_PEER = 1, - WOLFSSL_VERIFY_FAIL_IF_NO_PEER_CERT = 2, - WOLFSSL_VERIFY_CLIENT_ONCE = 4, - WOLFSSL_VERIFY_FAIL_EXCEPT_PSK = 8, + WOLFSSL_VERIFY_PEER = 1 << 0, + WOLFSSL_VERIFY_FAIL_IF_NO_PEER_CERT = 1 << 1, + WOLFSSL_VERIFY_CLIENT_ONCE = 1 << 2, + WOLFSSL_VERIFY_FAIL_EXCEPT_PSK = 1 << 3, + WOLFSSL_VERIFY_DEFAULT = 1 << 9, WOLFSSL_SESS_CACHE_OFF = 0x0000, WOLFSSL_SESS_CACHE_CLIENT = 0x0001, diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index 0a52208c6..2246e9826 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -317,9 +317,9 @@ enum Misc_ASN { ASN_GEN_TIME_SZ = 15, /* 7 numbers * 2 + Zulu tag */ #ifndef NO_RSA #ifdef WOLFSSL_HAPROXY - MAX_ENCODED_SIG_SZ = 1024, + MAX_ENCODED_SIG_SZ = 1024, /* Supports 8192 bit keys */ #else - MAX_ENCODED_SIG_SZ = 512, + MAX_ENCODED_SIG_SZ = 512, /* Supports 4096 bit keys */ #endif #elif defined(HAVE_ECC) MAX_ENCODED_SIG_SZ = 140,