Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GitHub update issues on some accounts (... Download failed. SSL certificate problem...) #368

Closed
pglewis opened this issue Sep 13, 2012 · 7 comments
Assignees
Milestone

Comments

@pglewis
Copy link
Contributor

pglewis commented Sep 13, 2012

Not a direct Pods issue from what I can tell, but still an issue on some setups.

The "force plugin refresh" functionality uses the WP bulk updater. This eventually calls download_url() in wp-admin/includes/file.php, which calls wp_remote_get() directly with a hard-coded argument list that doesn't include 'sslverify'. If 'sslverify' is not set to FALSE on some configurations, the update will fail with the message:

An error occurred while updating Pods Framework: Download failed. SSL certificate problem, verify that the CA cert is OK. Details: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed.
@ghost ghost assigned pglewis Sep 13, 2012
@pglewis
Copy link
Contributor Author

pglewis commented Sep 13, 2012

One possibility for a patch, I'm currently using this in a theme functions.php with success:

    define ('PODS_SSL_VERIFY', FALSE);

    add_action('http_request_args', 'pods_ssl_verify', 10, 2);
    function pods_ssl_verify($args, $url) {

        if ($url === 'https://github.com/pods-framework/pods/zipball/2.0') {
            $args['sslverify'] = PODS_SSL_VERIFY;
        }

        return $args;
    }
  • Could make the value a plugin setting, or just leave it as a contant/define
  • Probably want to break out the GitHub URL parts into separate constants if we're going to reference them in multiple places... in any event, hardcoding the URL here is just a local quick-and-dirty solution.

@pglewis
Copy link
Contributor Author

pglewis commented Sep 13, 2012

As a quick note to clarify just in case there is any confusion: the WPGitHubUpdater class doesn't appear to have been the culprit on this issue at all. The error never occurs on update CHECKS (which is where WPGitHubUpdater is involved). The error occurs when grabbing the actual update from GitHub, which, as explained, gets funneled through the WP bulk updater (which eventually calls wp_remote_get() directly with hard-coded parameters).

The proposed patch deals with this by hooking the http_request_args action and adding sslverify = false if/when downloading the latest Pods zip.

@sc0ttkclark
Copy link
Member

I'm only OK with this code if we can get it to be restricted only to that plugin request which does that. If it effects all wp_remote_get() then that's a deal breaker :(

@pglewis
Copy link
Contributor Author

pglewis commented Sep 13, 2012

The action hook will call pods_ssl_verify() on any wp_remote_get() call... but it's only changing sslverify if the $url parameter matches the Pods zipball:

if ($url === 'https://github.com/pods-framework/pods/zipball/2.0')

I definitely wouldn't want to turn off SSL verify willy-nilly for any https request.

@sc0ttkclark
Copy link
Member

Sounds good, but use this:

if ( defined( 'PODS_GITHUB_ZIP' ) && PODS_GITHUB_ZIP == $url )

@pglewis
Copy link
Contributor Author

pglewis commented Sep 13, 2012

Okay, simplified and seems to work:

add_action( 'http_request_args', 'pods_ssl_verify', 10, 2 );
function pods_ssl_verify( $args, $url ) {

    if ( defined( 'PODS_GITHUB_ZIP' ) && PODS_GITHUB_ZIP == $url ) {
        $args[ 'sslverify' ] = FALSE;
    }

    return $args;
}

@sc0ttkclark
Copy link
Member

Thanks @pglewis, let's put this back into the GitHub updater instead of something custom in our plugin. Check out the pull request over at radishconcepts/WordPress-GitHub-Plugin-Updater#20 for my proposal. Thanks for figuring this out!

sc0ttkclark added a commit that referenced this issue Sep 15, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants