[ruby-core:43967] [ruby-trunk - Bug #6233] Definition of EVP_MD_CTX_cleanup incomplete.

Issue #6233 has been updated by MartinBosslet (Martin Bosslet).


mame (Yusuke Endoh) wrote:
> Hello, emboss
> 
> What do you think?
> 

Hi, I think it's a valid point - here is how OpenSSL 1.0.1 implements it:

/* This call frees resources associated with the context */
int EVP_MD_CTX_cleanup(EVP_MD_CTX *ctx)
	{
#ifndef OPENSSL_FIPS
	/* Don't assume ctx->md_data was cleaned in EVP_Digest_Final,
	 * because sometimes only copies of the context are ever finalised.
	 */
	if (ctx->digest && ctx->digest->cleanup
	    && !EVP_MD_CTX_test_flags(ctx,EVP_MD_CTX_FLAG_CLEANED))
		ctx->digest->cleanup(ctx);
	if (ctx->digest && ctx->digest->ctx_size && ctx->md_data
	    && !EVP_MD_CTX_test_flags(ctx, EVP_MD_CTX_FLAG_REUSE))
		{
		OPENSSL_cleanse(ctx->md_data,ctx->digest->ctx_size);
		OPENSSL_free(ctx->md_data);
		}
#endif
	if (ctx->pctx)
		EVP_PKEY_CTX_free(ctx->pctx);
#ifndef OPENSSL_NO_ENGINE
	if(ctx->engine)
		/* The EVP_MD we used belongs to an ENGINE, release the
		 * functional reference we held for this reason. */
		ENGINE_finish(ctx->engine);
#endif
#ifdef OPENSSL_FIPS
	FIPS_md_ctx_cleanup(ctx);
#endif
	memset(ctx,'\0',sizeof *ctx);

	return 1;
	}

Quite some additional cleansing besides the memset. We could simply copy this, but I'm afraid it could
cause compatibility problems with older versions and we would additionally have to keep this in sync with
what OpenSSL does there - both unpleasant situations. EVP_MD_CTX_cleanup was introduced in 0.9.7, I just 
checked. We claim compatibility from 0.9.6 on. I think the question must be allowed: do we really need
compatibility with 0.9.6? It was released in 2000, 0.9.7 came in 2002. 10 years of backward compatibility
aren't that bad either :) No but honestly, when I think of all the security fixes that came since then,
nobody should really be running on 0.9.6 anymore anyway. I'm not against fixing this with the above and
keeping the code in sync, but I'd be more happy with not having to poke around in implementation details
and dropping 0.9.6 support instead.

But I'm not sure how this plays with the general principles of 2.0.0 or if anyone would be really against
dropping 0.9.6 support. Opinions?   

----------------------------------------
Bug #6233: Definition of EVP_MD_CTX_cleanup incomplete. 
https://bugs.ruby-lang.org/issues/6233#change-25487

Author: rubysubmit (Ruby Submit)
Status: Assigned
Priority: Low
Assignee: MartinBosslet (Martin Bosslet)
Category: 
Target version: 
ruby -v: ruby 1.9.2p290


File: ext\openssl\openssl_missing.c
Line: 67

#if !defined(HAVE_EVP_MD_CTX_CLEANUP)
int
EVP_MD_CTX_cleanup(EVP_MD_CTX *ctx)
{
    /* FIXME!!! */
    memset(ctx, 0, sizeof(EVP_MD_CTX));

    return 1;
}
#endif



-- 
http://bugs.ruby-lang.org/