JuliaLang / MbedTLS.jl

Wrapper around mbedtls
Other
41 stars 50 forks source link

MBEDTLSJL_CERT_PEM_* env vars to a more flexible scheme to cert.pem #261

Closed csantosb closed 1 year ago

csantosb commented 1 year ago

Not all distributions package cert.pem file along with julia, as it is the case of Archlinux and Guix. The certificates file is to be located somewhere else.

Using two env variables (optional), one may declare where this package may find the cert.pem, whether in a directory, whether in a given absolute location.

codecov[bot] commented 1 year ago

Codecov Report

Merging #261 (3468cd2) into master (e224150) will decrease coverage by 0.47%. The diff coverage is 37.50%.

@@            Coverage Diff             @@
##           master     #261      +/-   ##
==========================================
- Coverage   73.56%   73.09%   -0.48%     
==========================================
  Files          12       12              
  Lines         715      721       +6     
==========================================
+ Hits          526      527       +1     
- Misses        189      194       +5     
Impacted Files Coverage Δ
src/ssl.jl 71.23% <37.50%> (-1.15%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

quinnj commented 1 year ago

I'm not sure I quite understand; in the PR, you seem to be checking if the new MBEDTLSJL_ envs are set, but then not using them?

csantosb commented 1 year ago

Sorry for the awful PR, I completely missed it ! I fixed it for good, taking into account all your remarks.

DilumAluthge commented 1 year ago

Not all distributions package cert.pem file along with julia, as it is the case of Archlinux and Guix. The certificates file is to be located somewhere else.

This is only the case if you are not using the official Julia binaries. The official binaries will always include the cert.pem file with Julia itself.

Personally, I don't think that we should be adding features like this to Julia packages just to work around people using broken distro Julias. I'd rather we revert this change, and then tell users to use the official Julia binaries.

quinnj commented 1 year ago

I think there's still a valid use-case for this; as we recently saw in https://github.com/JuliaWeb/HTTP.jl/issues/947, in private networks w/ self-signed certificates, it's convenient to specify env variables in order to customize which certs get used in the network stack.

DilumAluthge commented 1 year ago

Sure, but that's a different use case than was brought up in the OP.

Anyway, it would be good to at least standardize the names of these environment variables to match the names used by NetworkOptions (https://github.com/JuliaLang/NetworkOptions.jl).

khinsen commented 1 year ago

@DilumAluthge Your comment is rather disrespectful towards packagers. System-level distributions exist for a good reason. Different distributions exist because people's needs differ, in many ways. Packagers often have to modify software to make them fit their distribution's policies, which exist for good reasons. The official Julia binaries don't work with all these distributions, and then there are people who use specific distributions because they don't want to download and run binaries, for reasons ranging from security to reproducibility concerns.

The Julia community can of course decide that it doesn't want to accommodate specific distributions' needs because they would cause additional complexity in Julia. But please don't call distributions or their packages "broken" just because they cater for needs that the official Julia binaries do not satisfy.