ssl_verify key in remote config does not accept custom CA bundle path and aws config is ignored

[rgvanwesep]

The ssl_verify key in the remote config gets passed through to the S3FileSystem client_kwargs:
https://github.com/iterative/dvc/blob/89b40afee740146af42efeb8563c08053f984a88/dvc/fs/s3.py#L105
https://github.com/iterative/dvc/blob/89b40afee740146af42efeb8563c08053f984a88/dvc/fs/fsspec_wrapper.py#L17
https://github.com/iterative/dvc/blob/89b40afee740146af42efeb8563c08053f984a88/dvc/fs/s3.py#L154

These are in turn passed to the aiobotocore.AioSession :
https://github.com/dask/s3fs/blob/a3d7a946f85b6dbef62ab75c61fe1319a482c8ba/s3fs/core.py#L366

In the AioSession it checks if the verify key is set and if it isn't then it looks in the aws config:
https://github.com/aio-libs/aiobotocore/blob/2a7c7f5a8c7a61daebe484bc5a6f2232607af82c/aiobotocore/session.py#L70-L71
verify can either be a boolean or a string, with the latter being a path to a custom CA bundle:
https://boto3.amazonaws.com/v1/documentation/api/latest/reference/core/session.html (see the verify argument of the client method.)

However, the config schema for DVC only allows boolean for ssl_verify and defaults true:
https://github.com/iterative/dvc/blob/89b40afee740146af42efeb8563c08053f984a88/dvc/config_schema.py#L148

The result is that the aws config is never checked and a custom CA bundle cannot be used. If such a CA bundle is needed when trying to communicate to remote (e.g. using push or pull) the result is

[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate

I ran into this problem because my company uses a self-hosted S3 clone with a bundle of internally signed certificates. Setting the AWS_CA_BUNDLE environment variable did not resolve the issue. But modifying the config schema to accept a string:

Optional("ssl_verify", default=True): Any(Bool, str),

and running

dvc remote modify object-store ssl_verify "$HOME/.aws/cabundle.pem"

resolved the issue for me.

I'm happy to open a pull request to make the change to the config schema if that solution is acceptable, but it would be my first contribution (for any OSS project!), so it'll take extra time for me to setup my environment, etc.

[pmrowla]

Looks like we have 2 (related) action points here

• client["verify"] = config.get("ssl_verify", True) should default to passing None into boto instead of True if ssl_verify is not explicitly set for a DVC remote
• DVC's ssl_verify should also accept a ca_bundle string

(and finally the docs for ssl_verify will need to be updated)

@rgvanwesep if you'd like to work on this just let us know in this ticket. I'd say the first change is higher priority since it will make aiobotocore fall back to reading ca_bundle from the user's AWS config by default, but both can be done together.

pinging @isidentical to double check this

[rgvanwesep]

@pmrowla Good point on the first change. It would be good to be able to let boto fall back to the config.

I would like to work on this. I can make both changes at the same time.

[pmrowla]

related #5972

[dberenbaum]

Also see #5732 for discussion of how to implement this (whether as part of ssl_verify or a separate parameter) and related issues in #5388 and treeverse/dvc.org#1079. cc @skshetry

[rgvanwesep]

@dberenbaum I read through the related issues you posted. I think the discussion you are referring to this thread?
#5732 (comment)
The linked PR I posted implements a custom CA bundle path as part of ssl_verify (similar to how botocore and requests does it.)

Also, it seems that I ran into the same issue as this commenter:
#5972 (comment)
There, the AWS_CA_BUNDLE environment variable is ignored if anything is passed into verify either True or False. The linked PR should fix that issue as well.

[skshetry]

@dberenbaum, I am fine with this, but let me ping @efiop on this. What alternative name do we have?

[dberenbaum]

I thought you had recommended certs and I also think I saw or hear ca_bundle, but I don't have a strong opinion about the name or whether we even need a separate key.

[efiop]

@dberenbaum Thanks for reminding about that PR 🙏 , I totally forgot about it.

The ssl_verify is ok if user knows about boto, but probably not so good from a regular user's perspective. I guess we can keep it as is with ssl_verify for now, looks like there is not much pushback these days. Also, #6018 implementation is significantly better than the attempt we had before, so I personally won't feel bad for merging it.