Skip to content

support ssl verify (built on atop of 1135) #1142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions gix-transport/src/client/blocking_io/http/curl/remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ pub fn new() -> (
verbose,
ssl_ca_info,
ssl_version,
ssl_verify,
http_version,
backend,
},
Expand Down Expand Up @@ -194,6 +195,9 @@ pub fn new() -> (
}
}

handle.ssl_verify_peer(ssl_verify)?;
handle.ssl_verify_host(ssl_verify)?;

if let Some(http_version) = http_version {
let version = match http_version {
HttpVersion::V1_1 => curl::easy::HttpVersion::V11,
Expand Down
29 changes: 28 additions & 1 deletion gix-transport/src/client/blocking_io/http/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ pub mod options {

/// Options to configure http requests.
// TODO: testing most of these fields requires a lot of effort, unless special flags to introspect ongoing requests are added.
#[derive(Default, Clone)]
#[derive(Clone)]
pub struct Options {
/// Headers to be added to every request.
/// They are applied unconditionally and are expected to be valid as they occur in an HTTP request, like `header: value`, without newlines.
Expand Down Expand Up @@ -179,12 +179,39 @@ pub struct Options {
pub ssl_ca_info: Option<PathBuf>,
/// The SSL version or version range to use, or `None` to let the TLS backend determine which versions are acceptable.
pub ssl_version: Option<SslVersionRangeInclusive>,
/// Controls whether to perform SSL identity verification or not. Turning this off is not recommended and can lead to
/// various security risks. An example where this may be needed is when an internal git server uses a self-signed
/// certificate and the user accepts the associated security risks.
pub ssl_verify: bool,
/// The HTTP version to enforce. If unset, it is implementation defined.
pub http_version: Option<HttpVersion>,
/// Backend specific options, if available.
pub backend: Option<Arc<Mutex<dyn Any + Send + Sync + 'static>>>,
}

impl Default for Options {
fn default() -> Self {
Options {
extra_headers: vec![],
follow_redirects: Default::default(),
low_speed_limit_bytes_per_second: 0,
low_speed_time_seconds: 0,
proxy: None,
no_proxy: None,
proxy_auth_method: Default::default(),
proxy_authenticate: None,
user_agent: None,
connect_timeout: None,
verbose: false,
ssl_ca_info: None,
ssl_version: None,
ssl_verify: true,
http_version: None,
backend: None,
}
}
}

/// The actual http client implementation, using curl
#[cfg(feature = "http-client-curl")]
pub type Impl = curl::Curl;
Expand Down
9 changes: 9 additions & 0 deletions gix/src/config/cache/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,15 @@ fn apply_environment_overrides(
},
],
),
(
"gitoxide",
Some(Cow::Borrowed("http".into())),
git_prefix,
&[{
let key = &gitoxide::Http::SSL_NO_VERIFY;
(env(key), key.name)
}],
),
(
"gitoxide",
Some(Cow::Borrowed("credentials".into())),
Expand Down
9 changes: 9 additions & 0 deletions gix/src/config/tree/sections/gitoxide.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,14 @@ mod subsections {
http::SslVersion::new_ssl_version("sslVersionMax", &Gitoxide::HTTP).with_note(
"entirely new to set the upper bound for the allowed ssl version range. Overwrites the max bound of `http.sslVersion` if set. Min and Max must be set to become effective.",
);
/// The `gitoxide.http.sslNoVerify` key.
///
/// If set, disable SSL verification. Using this is discouraged as it can lead to
/// various security risks. An example where this may be needed is when an internal
/// git server uses a self-signed certificate and the user accepts the associated security risks.
pub const SSL_NO_VERIFY: keys::Boolean = keys::Boolean::new_boolean("sslNoVerify", &Gitoxide::HTTP)
.with_environment_override("GIT_SSL_NO_VERIFY")
.with_note("used to disable SSL verification. When this is enabled it takes priority over http.sslVerify");
/// The `gitoxide.http.proxyAuthMethod` key.
pub const PROXY_AUTH_METHOD: http::ProxyAuthMethod =
http::ProxyAuthMethod::new_proxy_auth_method("proxyAuthMethod", &Gitoxide::HTTP)
Expand All @@ -199,6 +207,7 @@ mod subsections {
&Self::CONNECT_TIMEOUT,
&Self::SSL_VERSION_MIN,
&Self::SSL_VERSION_MAX,
&Self::SSL_NO_VERIFY,
&Self::PROXY_AUTH_METHOD,
]
}
Expand Down
4 changes: 4 additions & 0 deletions gix/src/config/tree/sections/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ impl Http {
.with_deviation(
"accepts the new 'default' value which means to use the curl default just like the empty string does",
);
/// The `http.sslVerify` key.
pub const SSL_VERIFY: keys::Boolean = keys::Boolean::new_boolean("sslVerify", &config::Tree::HTTP)
.with_note("also see the `gitoxide.http.sslNoVerify` key");
/// The `http.proxy` key.
pub const PROXY: keys::String =
keys::String::new_string("proxy", &config::Tree::HTTP).with_deviation("fails on strings with illformed UTF-8");
Expand Down Expand Up @@ -58,6 +61,7 @@ impl Section for Http {
fn keys(&self) -> &[&dyn Key] {
&[
&Self::SSL_VERSION,
&Self::SSL_VERIFY,
&Self::PROXY,
&Self::PROXY_AUTH_METHOD,
&Self::VERSION,
Expand Down
24 changes: 24 additions & 0 deletions gix/src/repository/config/transport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,30 @@ impl crate::Repository {
}
}

{
let key = "gitoxide.http.sslNoVerify";
let ssl_no_verify = config
.boolean_filter_by_key(key, &mut trusted_only)
.map(|value| config::tree::gitoxide::Http::SSL_NO_VERIFY.enrich_error(value))
.transpose()
.with_leniency(lenient)
.map_err(config::transport::http::Error::from)?
.unwrap_or_default();

if ssl_no_verify {
opts.ssl_verify = false;
} else {
let key = "http.sslVerify";
opts.ssl_verify = config
.boolean_filter_by_key(key, &mut trusted_only)
.map(|value| config::tree::Http::SSL_VERIFY.enrich_error(value))
.transpose()
.with_leniency(lenient)
.map_err(config::transport::http::Error::from)?
.unwrap_or(true);
}
}

#[cfg(feature = "blocking-http-transport-curl")]
{
let key = "http.schannelCheckRevoke";
Expand Down
Git LFS file not shown
11 changes: 11 additions & 0 deletions gix/tests/fixtures/make_config_repos.sh
Original file line number Diff line number Diff line change
Expand Up @@ -164,3 +164,14 @@ mkdir not-a-repo-with-files;
(cd not-a-repo-with-files
touch this that
)

git init ssl-verify-disabled
(cd ssl-verify-disabled
git config http.sslVerify false
)

git init ssl-no-verify-enabled
(cd ssl-no-verify-enabled
git config http.sslVerify true
git config gitoxide.http.sslNoVerify true
)
2 changes: 2 additions & 0 deletions gix/tests/gix-init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ mod with_overrides {
.set("GIT_HTTP_LOW_SPEED_LIMIT", "1")
.set("GIT_HTTP_LOW_SPEED_TIME", "1")
.set("GIT_HTTP_PROXY_AUTHMETHOD", "proxy-auth-method-env")
.set("GIT_SSL_NO_VERIFY", "true")
.set("GIT_CURL_VERBOSE", "true")
.set("https_proxy", "https-lower-override")
.set("HTTPS_PROXY", "https-upper")
Expand Down Expand Up @@ -231,6 +232,7 @@ mod with_overrides {
]
);
for (key, expected) in [
("gitoxide.http.sslNoVerify", "true"),
("gitoxide.http.verbose", "true"),
("gitoxide.allow.protocolFromUser", "file-allowed"),
("core.useReplaceRefs", "no-replace"),
Expand Down
22 changes: 22 additions & 0 deletions gix/tests/repository/config/transport_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ mod http {
verbose,
ssl_ca_info,
ssl_version,
ssl_verify,
http_version,
backend,
} = http_options(&repo, None, "https://example.com/does/not/matter");
Expand Down Expand Up @@ -106,6 +107,8 @@ mod http {
max: version
})
);

assert!(ssl_verify, "SSL verification is enabled by default if not configured");
assert_eq!(http_version, Some(HttpVersion::V1_1));
}

Expand Down Expand Up @@ -314,4 +317,23 @@ mod http {
assert_eq!(opts.proxy.as_deref(), Some("http://localhost:9090"));
assert_eq!(opts.follow_redirects, FollowRedirects::Initial);
}

#[test]
fn ssl_verify_disabled() {
let repo = repo("ssl-verify-disabled");

let opts = http_options(&repo, None, "https://example.com/does/not/matter");
assert!(!opts.ssl_verify);
}

#[test]
fn ssl_no_verify_takes_precedence() {
let repo = repo("ssl-no-verify-enabled");

let opts = http_options(&repo, None, "https://example.com/does/not/matter");
assert!(
!opts.ssl_verify,
"even with `http.sslVerify` enabled, `gitoxide.http.sslNoVerify` takes precedence`"
);
}
}
4 changes: 0 additions & 4 deletions src/plumbing/progress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,10 +408,6 @@ static GIT_CONFIG: &[Record] = &[
config: "http.sslCipherList",
usage: NotPlanned { reason: "on demand" }
},
Record {
config: "http.sslVerify",
usage: NotPlanned { reason: "on demand" }
},
Record {
config: "http.sslCert",
usage: NotPlanned { reason: "on demand" }
Expand Down