Skip to content

Commit

Permalink
Merge branch 'support_ssl_verify'
Browse files Browse the repository at this point in the history
  • Loading branch information
Byron committed Dec 1, 2023
2 parents 698caaa + ead00e9 commit 5ce9784
Show file tree
Hide file tree
Showing 11 changed files with 115 additions and 7 deletions.
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 @@ -180,6 +180,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 @@ -200,6 +208,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

0 comments on commit 5ce9784

Please sign in to comment.