clearlinux / cve-check-tool

Original Automated CVE Checking Tool
GNU General Public License v2.0
204 stars 78 forks source link

curl: allow overriding default CA certificate file #45

Closed pohly closed 7 years ago

pohly commented 8 years ago

Similar to curl, --cacert can now be used in cve-check-tool and cve-check-update to override the default CA certificate file. Useful in cases where the system default is unsuitable (for example, out-dated) or broken (as in OE's current native libcurl, which embeds a path string from one build host and then uses it on another although the right path may have become something different - see https://bugzilla.yoctoproject.org/show_bug.cgi?id=9883).

ikeydoherty commented 8 years ago

I'm really not too sure on this. @mdemeter thoughts? In the context of cve-check-tool, our primary data source is accessible via https, so https is sacred.

pohly commented 8 years ago

https of course needs to work. The whole point of the --cacert parameter is to ensure that it works in some cases where the default is not good enough. One can of course argue that in theory such a parameter should never be necessary and I would agree. But in practice, practice and theory do not always agree with each other.

The precedent it the "curl" command line, which has exactly the same parameter and also an environment variable for it.

mdemeter commented 8 years ago

Can I get some context as to what the question is referring to?

Michael

From: Ikey Doherty notifications@github.com Reply-To: ikeydoherty/cve-check-tool reply@reply.github.com Date: Wednesday, July 6, 2016 at 7:06 AM To: ikeydoherty/cve-check-tool cve-check-tool@noreply.github.com Cc: Michael Demeter michael.demeter@intel.com, Mention mention@noreply.github.com Subject: Re: [ikeydoherty/cve-check-tool] curl: allow overriding default CA certificate file (#45)

I'm really not too sure on this. @mdemeterhttps://github.com/mdemeter thoughts? In the context of cve-check-tool, our primary data source is accessible via https, so https is sacred.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/ikeydoherty/cve-check-tool/pull/45#issuecomment-230781560, or mute the threadhttps://github.com/notifications/unsubscribe/AA7TyrL79HMlEy2pT2L6bW0eDuJ4tPxmks5qS7ZVgaJpZM4JF4Py.

ikeydoherty commented 8 years ago

@mdemeter adding a --cacert option to cve-check-tool to override where we find the system CA certificate for SSL verification. This is apparently to facilitate integration into Yocto builds. It gave me the heeby jeebies.

mdemeter commented 8 years ago

Well you know how much trouble Leibowitz has had in getting certs to work properly in Clear.. It may not be a bad Idea to have Leibowitz examine the patches and if he doesn’t get the heeby jeebies well you know…

So can you forward the patch set info to leibowitz

Michael

From: Ikey Doherty notifications@github.com Reply-To: ikeydoherty/cve-check-tool reply@reply.github.com Date: Wednesday, July 6, 2016 at 9:00 AM To: ikeydoherty/cve-check-tool cve-check-tool@noreply.github.com Cc: Michael Demeter michael.demeter@intel.com, Mention mention@noreply.github.com Subject: Re: [ikeydoherty/cve-check-tool] curl: allow overriding default CA certificate file (#45)

@mdemeterhttps://github.com/mdemeter adding a --cacert option to cve-check-tool to override where we find the system CA certificate for SSL verification. This is apparently to facilitate integration into Yocto builds. It gave me the heeby jeebies.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/ikeydoherty/cve-check-tool/pull/45#issuecomment-230818695, or mute the threadhttps://github.com/notifications/unsubscribe/AA7TynoB4UnCLMrt7zAtkahgZcHMt0yMks5qS9E4gaJpZM4JF4Py.

mikeleib commented 8 years ago

This is fine. This just allows one to set where a CA bundle file is.

ikeydoherty commented 8 years ago

OK heeby jeebies dealt with, thanks guys.

@pohly we can look at merging this functionality into the 2 branch when that's ready, which is something like 201 commits ahead of master at this point. For now it's best to carry the patch until a new release is done (I won't do another from master until 2 is finished)

pohly commented 8 years ago

@ikeydoherty thanks, that's fine. Mostly I wanted to get it reviewed and the feature taken into account for the next release, which is now accomplished.

ikeydoherty commented 8 years ago

Yeah we'll keep this PR open then if you don't mind so I don't forget about the CLI option. Main concern was getting a thumbs up from the security dudes

ikeydoherty commented 7 years ago

Had to demangle and rebase due to conflicts - but finally got it in. Took master branch back and removed dev code for now to make it easier for others to track

https://github.com/ikeydoherty/cve-check-tool/commit/0cd5810516f3d48b10a36e4c59488d5e6215b15c

Thanks!