BUGGED:

flags = !opt_benchmark
       && ( strncmp( rpc_url, "https:", 6 )
           || strncasecmp(rpc_url, "stratum+tcps://", 15 ) )
        ? ( CURL_GLOBAL_ALL & ~CURL_GLOBAL_SSL )
        : CURL_GLOBAL_ALL;

FIXED:

if (opt_benchmark)
    flags = CURL_GLOBAL_NOTHING;
else if ( strncmp(rpc_url, "https:", 6) && strncasecmp(rpc_url, "stratum+tcps://", 15) )
    flags = CURL_GLOBAL_ALL & ~CURL_GLOBAL_SSL;
else
    flags = CURL_GLOBAL_ALL;

if ( curl_global_init( flags ) )
{
    applog(LOG_ERR, "CURL initialization failed");
    return 1;
}

JayDDee commented 4 years ago

Good catch, an always true trap,

Would it not be better to protect the curl_global_init call from opt_benchmark?

if ( !opt_benchmark ) { if ( strncmp(rpc_url, "https:", 6) && strncasecmp(rpc_url, "stratum+tcps://", 15) ) flags = CURL_GLOBAL_ALL & ~CURL_GLOBAL_SSL; else flags = CURL_GLOBAL_ALL; if ( curl_global_init( flags ) ) { applog(LOG_ERR, "CURL initialization failed"); return 1; } }

I will probably change strncasecmp to strncmp also.

JayDDee commented 4 years ago

cpuminer-opt-3.12.6 is release with a fix for this issue. It corrects the logic and uses strncasecmp for both terms. Benchmark was not changed, as you suggested, to use CURL_GLOBAL_NOTHING.

The fix is missing from the release notes.

Please test and post results.

Also, if you cold, please explain the impact of the bug. stratum+tcps was recently tested and reported to be working. Did it connect without ssl? Were there any errors or warnings? How did you discover the bug?

If the miner silently fell back to an isecure connection without warning when the user explicitly requested a secure connection it would be critical security issue in most cases. For crypto mining is isn't such a big deal. Regardless of the type of application it should be fixed. I will look for a way to either, error out if a secure connection can't be made, or, display a warning when falling back.

JayDDee commented 4 years ago

There is a problem with v3.12.6, it broke benchmark.

Another fix will be in the next release.

biospb commented 4 years ago

I was testing one algo on Rplant pool which supports SSL connection and another cpuminers including their fork worked fine, while your versions gave this error:

Stratum connection failed: SSL: couldn't create a context: error:140A90A1:lib(20):func(169):reason(161)

It was related to libcurl and CURL_GLOBAL_SSL. This may depend on libcurl version and target system. Mine was Ubuntu 16 LTS.

JayDDee commented 4 years ago

Thanks for testing. It looks like the fix made things worse (#254).

Try this code change, I just tested it with benchmark and SSL.

cpu-miner.c:3574

old

flags = !opt_benchmark || ( strncasecmp( rpc_url, "https:", 6 ) && strncasecmp( rpc_url, "stratum+tcps://", 15 ) ) ? ( CURL_GLOBAL_ALL & ~CURL_GLOBAL_SSL ) : CURL_GLOBAL_ALL;

new

flags = CURL_GLOBAL_ALL; if ( !opt_benchmark ) if ( strncasecmp( rpc_url, "https:", 6 ) && strncasecmp( rpc_url, "stratum+tcps://", 15 ) ) flags &= ~CURL_GLOBAL_SSL;

rplant8 commented 4 years ago

maybe !strncasecmp( rpc_url, "https:", 6 ) || !strncasecmp( rpc_url, "stratum+tcps://", 15 ) if one zero then ssl

JayDDee commented 4 years ago

Thanks @rplant8.

I was trying to be too clever with the boolean algebra, was distracted by other issues and didn't test properly.

The above change has been tested with standard stratum, SSL and benchmark.

JayDDee commented 4 years ago

cpuminer-opt-3.12.6.1 is released with the fix. It should work now, please let me know otherwise.

JayDDee commented 4 years ago

No problems reported, closing.

JayDDee / cpuminer-opt

SSL connection not working (with fix) #252

BUGGED:

FIXED: