devel/arcanist-lib: Drop ca_root_nss dependency.

[des]

Summary

This was much harder than it needs to be, because Arcanist is dead set
on forcing a CA bundle instead of letting curl pick one or use the OS
native trust store. Remove the enforced fallback and set CURL_CAINFO
only if a CA bundle was explicitly configured or custom.pem was found
on disk. Furthermore, if the configured value is a directory, set
CURL_CAPATH instead.

[des]

the default needs to be set in two different places

[des]

Setting a sensible default doesn't cover all cases, so just remove the offending code.

[grembo]

devel/arcanist-lib/files/patch-src_future_http_HTTPSFuture.php

35–47

TL;DR Should I maybe just finish this up (cleaning up the patch, add a pkg-message etc.) and commit it?

---

This is essentially a big noop, as $ca_bundle will be $this->cabundle, which will be uninitialized as none of the setCABundle* functions is called anymore. So these lines can be removed as well, to reduce confusion (lines 362 through 403 of HTTPSFuture.php can be patched out, maybe also the getters/setters that aren't necessary anymore).

Just to make sure: This basically means arcanist will use whatever is the default with the PHP installation (set in openssl.cafile or curl.cainfo - there is no php.ini setting for setting CAPATH) or fallback to libcurl's default on the system.

It would also be userfriendly to keep the "custom.pem" part of the removed logic (so people who installed their own custom certbundle will still be able to do so - that's useful if one only wants to set a specific bundle for using arcanist but not alter php.ini system-wide).

[des]

devel/arcanist-lib/files/patch-src_future_http_HTTPSFuture.php

35–47

This is essentially a big noop, as $ca_bundle will be $this->cabundle, which will be uninitialized as none of the setCABundle* functions is called anymore.

No, it can still be set in ~/.arcrc.

(Initially I had left the code intact but set a default value for https.cabundle which referenced the system trust store. This is where the CURL_CAPATH part of the patch comes from, because passing the system trust store to CURL_CAINFO won't work. But I had to set the default in two separate places and I _still_ ran into a scenario where neither default got loaded, so I dropped that part. It works fine if you set it in your ~/.arcrc though. Maybe it also works if you set it in the repository's .arcconfig.)

It would also be userfriendly to keep the "custom.pem" part of the removed logic

Sure, I can restore support for custom.pem and curl.cainfo. The important part is not to ship (and fall back to) default.ssl.

[grembo]

devel/arcanist-lib/files/patch-src_future_http_HTTPSFuture.php

35–47

No, it can still be set in ~/.arcrc.

Are you sure that works? I cannot find anything in the code looking for ssl_key or calling setCABundleFromString outside of comments.

Afaik curl.cainfo is handled in php-curl - so if nothing is set in .arcrc and there is no default.pem, not setting anything explicitly should be ok (re-interpreting a php module's setting is a bit weird anyway).

This is the code from php-curl:

cainfo = INI_STR("openssl.cafile");
if (!(cainfo && cainfo[0] != '\0')) {
        cainfo = INI_STR("curl.cainfo");
}
if (cainfo && cainfo[0] != '\0') {
        curl_easy_setopt(ch->cp, CURLOPT_CAINFO, cainfo);
}

So the order should be (to be implemented):

1. Use certs from .arcrc (if that actually works?!??)
2. Use custom.pem, if it exists
3. Use default (accomplished by not setting CAINFO or CAPATH) [1]

[1] Default (which is outside of the scope of the patch) means:

1. php-curl's config (openssl.cafile or curl.cainfo) and if not set
2. libcurl's default (depending on ftp/curl's config)

[grembo]

devel/arcanist-lib/files/patch-src_future_http_HTTPSFuture.php

35–47

Ah, ok I found the setting in arcanist.php:

// Apply global CA bundle from configs.
$ca_bundle = $configuration_manager->getConfigFromAnySource('https.cabundle');
if ($ca_bundle) {
  $ca_bundle = Filesystem::resolvePath(
    $ca_bundle, $working_copy->getProjectRoot());
  HTTPSFuture::setGlobalCABundleFromPath($ca_bundle);
}

so the order above makes sense.

[des]

restore part of the original logic

[des]

let php-curl deal with curl.cainfo, it's none of our business

[grembo]

I haven't tested the changes myself, but this looks sound to me.

Thanks for bringing it up & fixing it.