Issue #13681 has been updated by rinzler (Colton Jenkins).


rhenium (Kazuki Yamaguchi) wrote:
> > ```diff
> > diff -Nurp old/ext/digest/digest.h new/ext/digest/digest.h
> > --- old/ext/digest/digest.h	2017-06-21 12:56:09.007011362 -0400
> > +++ new/ext/digest/digest.h	2017-06-21 12:56:32.458975554 -0400
> > @@ -31,6 +31,26 @@ typedef struct {
> >      rb_digest_hash_finish_func_t finish_func;
> >  } rb_digest_metadata_t;
> >  
> > +#define DEFINE_INIT_FUNC_FOR_EVP(upper_name, lower_name) \
> > +int \
> > +rb_digest_##upper_name##_evp_init(upper_name##_CTX* ctx) \
> > +{ \
> > +  SSL_load_error_strings(); \
> > +  EVP_MD_CTX *md_ctx = EVP_MD_CTX_create(); \
> > + \
> > +  if(!EVP_DigestInit_ex(md_ctx, EVP_##lower_name(), NULL)) { \
> > +    const char *error_message; \
> > +    EVP_MD_CTX_destroy(md_ctx); \
> > + \
> > +    error_message = ERR_reason_error_string(ERR_peek_last_error()); \
> 
> The OpenSSL error queue must be cleared by `ERR_clear_error()`.

Ah, good catch.

> 
> > ```diff
> > +    rb_raise(rb_eRuntimeError, error_message); \
> > +  } \
> > +  *ctx = *(upper_name##_CTX*)md_ctx->md_data; \
> 
> This won't compile with OpenSSL 1.1.x since `EVP_MD_CTX` was made opaque.

Hasn't it always been opaque? They just moved the struct def from evp.h -> evp_locl.h, but the typedef is still in ossl_typ.h
> 
> Also I suspect this approach breaks if an external OpenSSL engine
> replaces the default implementation for the algorithm. I think we have
> to completely rewrite to use the EVP API only.

Not sure I follow. Passing NULL for engine skips it? But using EVP api would be ideal.
> 
> > ```diff
> > +  EVP_MD_CTX_destroy(md_ctx); \
> > + \
> > +  return 1; \
> > +}
> > +
> >  #define DEFINE_UPDATE_FUNC_FOR_UINT(name) \
> >  void \
> >  rb_digest_##name##_update(void *ctx, unsigned char *ptr, size_t size) \
> 
> 
> Another idea: I wonder if it would be possible or desirable to rip out
> the OpenSSL backend entirely instead. There would be some performance
> degradation, though, people could switch to OpenSSL::Digest of the
> 'openssl' extension if they really care about. It uses the EVP API and
> works as a drop-in replacement (in fact it inherits from `Digest::Base`).

That's an option. OpenSSL 1.1.x 'currently' doesn't support FIPS so it would 
be up to the app devs (me) to ensure all code doesn't use non fips compliant algos (ouch).
From what I've seen the majority of gems use digest over openssl::digest 
given it isn't guaranteed the system it deploys too will have it.
What is nice about it's current implementation is if the system does have OpenSSL 
with fips enabled then digest will obtain that functionality with this. 
I doubt many rubyist have to deal with FIPS, but this does make life a bit easier.
https://github.com/bundler/bundler/issues/4989

----------------------------------------
Feature #13681: Ruby digest init fails in FIPS mode when built against OpenSSL ~> 1.0.1
https://bugs.ruby-lang.org/issues/13681#change-65480

* Author: rinzler (Colton Jenkins)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
When FIPS (https://en.wikipedia.org/wiki/FIPS_140-2) is enabled attempting to initialize any digest will kill the process due to https://github.com/openssl/openssl/commit/65300dcfb04bae643ea7b8f42ff8c8f1b1210a9e

Example,

~~~
> require 'digest'
> Digest::MD5.new
md5_dgst.c(75): OpenSSL internal error, assertion failed: Low level API call to digest MD5 forbidden in FIPS mode!

> require 'digest'
> Digest::SHA1.new
sha_locl.h(128): OpenSSL internal error, assertion failed: Low level API call to digest SHA1 forbidden in FIPS mode!
~~~

This patch will redefine alg##_Init to use the EVP interface. This allows the digest initialization to never die, but will fail when using a non FIPS algorithm (MD5).

Example,

~~~
irb(main):002:0> Digest::MD5.new
RuntimeError: disabled for fips
	from (irb):2:in `new'
	from (irb):2
	from /usr/local/bin/irb:11:in `<main>'
irb(main):003:0> Digest::SHA1.new
=> #<Digest::SHA1: da39a3ee5e6b4b0d3255bfef95601890afd80709>
~~~

---Files--------------------------------
add_evp_init_to_digests.patch (3.77 KB)


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

Unsubscribe: <mailto:ruby-core-request / ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>