From d8e40dea3f03c112ec60ff2f9c612ead490324ae Mon Sep 17 00:00:00 2001 From: David Garske Date: Mon, 11 Nov 2019 15:39:23 -0800 Subject: [PATCH] Fixes from peer review: * Reduced codesize when building with `OPENSSL_EXTRA_X509_SMALL`. * Additional argument checks in `wolfSSL_ASN1_BIT_STRING_set_bit`, `wolfSSL_ASN1_STRING_to_UTF8`, `wolfSSL_RSA_meth_new`, `wolfSSL_RSA_meth_set`. * Fix for compiler warnings in asn.c using strncmp to duplicate string. "specified bound depends on the length of the source argument" --- src/ssl.c | 47 ++++++++++++++++++++++++++++++--------------- wolfcrypt/src/asn.c | 37 +++++++++++++++++------------------ 2 files changed, 50 insertions(+), 34 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 6341c4be7..40c6f7ea1 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -3562,6 +3562,9 @@ static const struct cipher{ { 0, NULL, 0} }; +#ifdef OPENSSL_EXTRA + +/* returns cipher using provided ctx type */ const WOLFSSL_EVP_CIPHER *wolfSSL_EVP_CIPHER_CTX_cipher( const WOLFSSL_EVP_CIPHER_CTX *ctx) { @@ -3597,6 +3600,8 @@ int wolfSSL_EVP_CIPHER_nid(const WOLFSSL_EVP_CIPHER *cipher) return 0; } +#endif /* OPENSSL_EXTRA */ + const WOLFSSL_EVP_CIPHER *wolfSSL_EVP_get_cipherbyname(const char *name) { @@ -7752,17 +7757,19 @@ void wolfSSL_sk_X509_EXTENSION_free(WOLFSSL_STACK* sk) int wolfSSL_ASN1_BIT_STRING_set_bit(WOLFSSL_ASN1_BIT_STRING* str, int pos, int val) { - int bytes_cnt = pos/8; - int bit = 1<<(7-(pos%8)); + int bytes_cnt, bit; char* temp; - if (!str || (val != 0 && val != 1)) { + if (!str || (val != 0 && val != 1) || pos < 0) { return WOLFSSL_FAILURE; } + bytes_cnt = pos/8; + bit = 1<<(7-(pos%8)); + if (bytes_cnt+1 > str->length) { if (!(temp = (char*)XREALLOC(str->data, bytes_cnt+1, NULL, - DYNAMIC_TYPE_OPENSSL))) { + DYNAMIC_TYPE_OPENSSL))) { return WOLFSSL_FAILURE; } XMEMSET(temp+str->length, 0, bytes_cnt+1 - str->length); @@ -7772,6 +7779,7 @@ int wolfSSL_ASN1_BIT_STRING_set_bit(WOLFSSL_ASN1_BIT_STRING* str, int pos, str->data[bytes_cnt] &= ~bit; str->data[bytes_cnt] |= val ? bit : 0; + return WOLFSSL_SUCCESS; } @@ -19309,19 +19317,23 @@ int wolfSSL_ASN1_STRING_to_UTF8(unsigned char **out, WOLFSSL_ASN1_STRING *in) The buffer *out should be free using OPENSSL_free(). */ unsigned char* buf; + unsigned char* inPtr; int inLen; if (!out || !in) { return -1; } + inPtr = wolfSSL_ASN1_STRING_data(in); inLen = wolfSSL_ASN1_STRING_length(in); - buf = (unsigned char*)XMALLOC(inLen + 1, NULL, - DYNAMIC_TYPE_OPENSSL); + if (!inPtr || inLen < 0) { + return -1; + } + buf = (unsigned char*)XMALLOC(inLen + 1, NULL, DYNAMIC_TYPE_OPENSSL); if (!buf) { return -1; } - XMEMCPY(buf, wolfSSL_ASN1_STRING_data(in), inLen + 1); + XMEMCPY(buf, inPtr, inLen + 1); *out = buf; return inLen; } @@ -33279,11 +33291,16 @@ int wolfSSL_RSA_LoadDer_ex(WOLFSSL_RSA* rsa, const unsigned char* derBuf, return WOLFSSL_SUCCESS; } +#if defined(OPENSSL_EXTRA) WOLFSSL_RSA_METHOD *wolfSSL_RSA_meth_new(const char *name, int flags) { int name_len; WOLFSSL_RSA_METHOD* meth; + if (name == NULL) { + return NULL; + } + meth = (WOLFSSL_RSA_METHOD*)XMALLOC(sizeof(WOLFSSL_RSA_METHOD), NULL, DYNAMIC_TYPE_OPENSSL); name_len = (int)XSTRLEN(name); @@ -33297,7 +33314,7 @@ WOLFSSL_RSA_METHOD *wolfSSL_RSA_meth_new(const char *name, int flags) return NULL; } XMEMCPY(meth->name, name, name_len+1); - WOLFSSL_MSG("RSA_METHOD is not implemented."); + return meth; } @@ -33309,18 +33326,20 @@ void wolfSSL_RSA_meth_free(WOLFSSL_RSA_METHOD *meth) } } +#ifndef NO_WOLFSSL_STUB int wolfSSL_RSA_meth_set(WOLFSSL_RSA_METHOD *rsa, void* p) { (void)rsa; (void)p; - WOLFSSL_MSG("RSA_METHOD is not implemented."); + WOLFSSL_STUB("RSA_METHOD is not implemented."); return 1; } +#endif -#if defined(OPENSSL_EXTRA) int wolfSSL_RSA_set_method(WOLFSSL_RSA *rsa, WOLFSSL_RSA_METHOD *meth) { - rsa->meth = meth; + if (rsa) + rsa->meth = meth; return 1; } @@ -33347,7 +33366,6 @@ void wolfSSL_RSA_set_flags(WOLFSSL_RSA *r, int flags) r->meth->flags = flags; } } -#endif /* OPENSSL_EXTRA */ int wolfSSL_RSA_set0_key(WOLFSSL_RSA *r, WOLFSSL_BIGNUM *n, WOLFSSL_BIGNUM *e, WOLFSSL_BIGNUM *d) @@ -33374,6 +33392,7 @@ int wolfSSL_RSA_set0_key(WOLFSSL_RSA *r, WOLFSSL_BIGNUM *n, WOLFSSL_BIGNUM *e, return 1; } +#endif /* OPENSSL_EXTRA */ #endif /* NO_RSA */ #ifdef OPENSSL_EXTRA @@ -36422,8 +36441,7 @@ err: } #endif /* OPENSSL_EXTRA */ -#if (defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL)) && \ - !defined(NO_ASN) +#if defined(OPENSSL_EXTRA) && !defined(NO_ASN) /* DN_Tags to strings */ static const struct DN_Tag_Strings { enum DN_Tags tag; @@ -44718,4 +44736,3 @@ int wolfSSL_X509_REQ_set_pubkey(WOLFSSL_X509 *req, WOLFSSL_EVP_PKEY *pkey) return wolfSSL_X509_set_pubkey(req, pkey); } #endif /* OPENSSL_EXTRA && !NO_CERTS && WOLFSSL_CERT_GEN && WOLFSSL_CERT_REQ */ - diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 01afe7a94..fe031c619 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -147,8 +147,8 @@ extern int wc_InitRsaHw(RsaKey* key); #endif #endif #ifdef WOLFSSL_RENESAS_TSIP_TLS -WOLFSSL_LOCAL void tsip_inform_key_position(const word32 key_n_start, - const word32 key_n_len, const word32 key_e_start, +WOLFSSL_LOCAL void tsip_inform_key_position(const word32 key_n_start, + const word32 key_n_len, const word32 key_e_start, const word32 key_e_len); WOLFSSL_LOCAL int tsip_tls_CertVerify(const byte *cert, word32 certSz, const byte *signature, word32 sigSz, @@ -4010,9 +4010,9 @@ exit_dc: /* This function is to retrieve key position information in a cert.* * The information will be used to call TSIP TLS-linked API for * * certificate verification. */ -static int RsaPublicKeyDecodeRawIndex(const byte* input, word32* inOutIdx, - word32 inSz, word32* key_n, - word32* key_n_len, word32* key_e, +static int RsaPublicKeyDecodeRawIndex(const byte* input, word32* inOutIdx, + word32 inSz, word32* key_n, + word32* key_n_len, word32* key_e, word32* key_e_len) { @@ -4534,7 +4534,7 @@ void FreeDecodedCert(DecodedCert* cert) #ifdef WOLFSSL_RENESAS_TSIP_TLS if (cert->tsip_encRsaKeyIdx != NULL) XFREE(cert->tsip_encRsaKeyIdx, cert->heap, DYNAMIC_TYPE_RSA); -#endif +#endif #ifndef NO_CERTS FreeSignatureCtx(&cert->sigCtx); #endif @@ -6541,14 +6541,14 @@ static int ConfirmSignature(SignatureCtx* sigCtx, #ifdef WOLFSSL_RENESAS_TSIP_TLS if (rsaKeyIdx != NULL) { - ret = tsip_tls_CertVerify(buf, bufSz, sigCtx->plain, + ret = tsip_tls_CertVerify(buf, bufSz, sigCtx->plain, sigSz, sigCtx->pubkey_n_start - sigCtx->certBegin, sigCtx->pubkey_n_len - 1, sigCtx->pubkey_e_start - sigCtx->certBegin, sigCtx->pubkey_e_len - 1, rsaKeyIdx); - + if (ret == 0){ sigCtx->verifyByTSIP = 1; ret = 0; @@ -7180,7 +7180,7 @@ static int DecodeBasicCaConstraint(const byte* input, int sz, DecodedCert* cert) if (input[idx] == ASN_INTEGER) return 0; #endif - + ret = GetBoolean(input, &idx, sz); if (ret < 0) { WOLFSSL_MSG("\tfail: constraint not valid BOOLEAN"); @@ -8719,10 +8719,10 @@ int ParseCertRelative(DecodedCert* cert, int type, int verify, void* cm) /* TSIP can only handle 2048 bits(256 byte) key. */ if (cert->ca && tsip_checkCA(cert->ca->cm_idx) != 0 && cert->sigCtx.pubkey_n_len == 256) { - + /* assign memory to encrypted tsip Rsa key index */ if (!cert->tsip_encRsaKeyIdx) - cert->tsip_encRsaKeyIdx = + cert->tsip_encRsaKeyIdx = (byte*)XMALLOC(TSIP_TLS_ENCPUBKEY_SZ_BY_CERTVRFY, cert->heap, DYNAMIC_TYPE_RSA); if (cert->tsip_encRsaKeyIdx == NULL) @@ -11451,13 +11451,12 @@ int EncodePolicyOID(byte *out, word32 *outSz, const char *in, void* heap) if (out == NULL || outSz == NULL || *outSz < 2 || in == NULL) return BAD_FUNC_ARG; + /* duplicate string (including terminator) */ len = (word32)XSTRLEN(in); - str = (char *)XMALLOC(len+1, heap, DYNAMIC_TYPE_TMP_BUFFER); if (str == NULL) return MEMORY_E; - - XSTRNCPY(str, in, len+1); + XMEMCPY(str, in, len+1); nb_val = 0; @@ -13278,12 +13277,12 @@ int wc_SetKeyUsage(Cert *cert, const char *value) cert->keyUsage = 0; + /* duplicate string (including terminator) */ len = (word32)XSTRLEN(value); str = (char*)XMALLOC(len+1, cert->heap, DYNAMIC_TYPE_TMP_BUFFER); if (str == NULL) return MEMORY_E; - - XSTRNCPY(str, value, len+1); + XMEMCPY(str, value, len+1); /* parse value, and set corresponding Key Usage value */ if ((token = XSTRTOK(str, ",", &ptr)) == NULL) { @@ -13337,12 +13336,12 @@ int wc_SetExtKeyUsage(Cert *cert, const char *value) cert->extKeyUsage = 0; + /* duplicate string (including terminator) */ len = (word32)XSTRLEN(value); str = (char*)XMALLOC(len+1, cert->heap, DYNAMIC_TYPE_TMP_BUFFER); if (str == NULL) return MEMORY_E; - - XSTRNCPY(str, value, len+1); + XMEMCPY(str, value, len+1); /* parse value, and set corresponding Key Usage value */ if ((token = XSTRTOK(str, ",", &ptr)) == NULL) { @@ -15196,7 +15195,7 @@ static int DecodeBasicOcspResponse(byte* source, word32* ioIndex, resp->response, resp->responseSz, cert.publicKey, cert.pubKeySize, cert.keyOID, resp->sig, resp->sigSz, resp->sigOID, NULL); - + FreeDecodedCert(&cert); if (ret != 0) {