catalyst / moodle-auth_saml2

SAML done 100% in Moodle, fast, simple, secure
https://moodle.org/plugins/auth_saml2
73 stars 136 forks source link

improve metadata fetch security (SSL_VERIFYHOST) #172

Open dustinbrisebois opened 6 years ago

dustinbrisebois commented 6 years ago

in moodle-auth_saml2/classes/metadata_fetcher.php on lines: 63 - 71

    $options = [
        'CURLOPT_SSL_VERIFYPEER' => true,
        'CURLOPT_SSL_VERIFYHOST' => true,
        'CURLOPT_CONNECTTIMEOUT' => 20,
        'CURLOPT_FOLLOWLOCATION' => 1,
        'CURLOPT_MAXREDIRS'      => 5,
        'CURLOPT_TIMEOUT'        => 300,
        'CURLOPT_RETURNTRANSFER' => true,
        'CURLOPT_NOBODY'         => false,
    ];

should be:

    $options = [
        'CURLOPT_SSL_VERIFYPEER' => 1,
        'CURLOPT_SSL_VERIFYHOST' => 2,
        'CURLOPT_HTTP_VERSION' => CURL_HTTP_VERSION_2TLS,
        'CURLOPT_CONNECTTIMEOUT' => 20,
        'CURLOPT_FOLLOWLOCATION' => 1,
        'CURLOPT_MAXREDIRS'      => 5,
        'CURLOPT_TIMEOUT'        => 300,
        'CURLOPT_RETURNTRANSFER' => true,
        'CURLOPT_NOBODY'         => false,
    ];

Ideally an option to also enable CURLOPT_SSL_VERIFYSTATUS => 1 for host-servers that support OCSP stapling.

see: https://curl.haxx.se/libcurl/c/CURLOPT_SSL_VERIFYHOST.html for documentation.

brendanheywood commented 6 years ago

@dustinbrisebois can you please convert this into a pull request for both stable branches?

roperto commented 6 years ago

Hi @dustinbrisebois , what would happen if someone is using http instead of https for metadata? I think this will prevent unsecure http (which could be good in some way).

roperto commented 6 years ago

I cannot merge the code as it is only PHP 7 compatible.

I'd suggest making it a configuration available only for environments with PHP 7.

See: https://travis-ci.org/catalyst/moodle-auth_saml2/builds/352596481

Pull requests are welcome.