On 12/02/18 16:07, Matt Caswell wrote: > > > On 12/02/18 16:05, Short, Todd wrote: >> My 2cents (since I can’t reply to the list), is that other functions >> (e.g. most SSL and SSL_CTX functions) require a non-NULL object. I’m not >> sure this is any different. > > Good point. Although that then changes the question slightly to be can > we assume that this function will never return an error (even though it > has in the past)?
But then thinking about it more - I don't think we can remove this check. Since we are currently checking this, removing it could cause a binary compat issue if some application is relying on that check. Matt > > > (BTW you can reply to the list - its just moderated so you have to wait > for someone to approve it.) > > Matt > >> >> -- >> -Todd Short >> // tsh...@akamai.com <mailto:tsh...@akamai.com> >> // "One if by land, two if by sea, three if by the Internet." >> >>> On Feb 12, 2018, at 11:02 AM, Matt Caswell <m...@openssl.org >>> <mailto:m...@openssl.org>> wrote: >>> >>> I've been looking at our use of EVP_MD_size() (prompted by Coverity). >>> >>> That function can return a -1 on error: >>> >>> int EVP_MD_size(const EVP_MD *md) >>> { >>> if (!md) { >>> EVPerr(EVP_F_EVP_MD_SIZE, EVP_R_MESSAGE_DIGEST_IS_NULL); >>> return -1; >>> } >>> return md->md_size; >>> } >>> >>> >>> The only (current) possible error is that the passed digest is NULL. >>> Otherwise it returns the size of the digest as you would expect. >>> >>> In some places we do things like this: >>> >>> const EVP_MD *md = ssl_md(s->session->cipher->algorithm2); >>> >>> if (md != NULL) { >>> /* >>> * Add the fixed PSK overhead, the identity length and the >>> binder >>> * length. >>> */ >>> hlen += PSK_PRE_BINDER_OVERHEAD + s->session->ext.ticklen >>> + EVP_MD_size(md); >>> } >>> >>> So we have an explicit NULL check of the md before we call the function. >>> Therefore there is no possible way that EVP_MD_size() can return >>> anything except a success response. >>> >>> Are we entitled to assume that? Or should we always check the return >>> value regardless? My instinct says we should in case we ever wanted to >>> change the function in the future to return an error in some other >>> circumstances. >>> >>> Just to make it more interesting our documentation does not mention the >>> possibility of an error return at all. >>> >>> Matt >>> _______________________________________________ >>> openssl-project mailing list >>> openssl-project@openssl.org <mailto:openssl-project@openssl.org> >>> https://mta.openssl.org/mailman/listinfo/openssl-project >> > _______________________________________________ > openssl-project mailing list > openssl-project@openssl.org > https://mta.openssl.org/mailman/listinfo/openssl-project > _______________________________________________ openssl-project mailing list openssl-project@openssl.org https://mta.openssl.org/mailman/listinfo/openssl-project