facebook/sapling
1
1
Fork 0
mirror of https://github.com/facebook/sapling.git synced 2024-10-09 00:14:35 +03:00
Code Issues Projects Releases Wiki Activity

mononoke: lfs_server: Enforce ACL checks on a per repo basis

Browse Source 
Summary:
Update the LFS server to use the `enforce_lfs_acl_check` to enforce
ACL checks for specific repos and also reject clients with missing idents.

In the next diff, I will use the existing LFS server config's
`enforce_acl_check` flag as a killswitch.

Reviewed By: krallin

Differential Revision: D22762451

fbshipit-source-id: 61d26944127711f3503e04154e8c079ae75dc815
This commit is contained in:
Harvey Hunt 2020-07-27 11:02:26 -07:00 committed by Facebook GitHub Bot
parent 7944ca9105
commit cce86abf14
8 changed files with 64 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
eden/mononoke/lfs_server/Cargo.toml
View File

@ -16,6 +16,7 @@ filestore = { path = "../filestore" }
gotham_ext = { path = "../gotham_ext" }
lfs_protocol = { path = "../lfs_protocol" }
metaconfig_parser = { path = "../metaconfig/parser" }
metaconfig_types = { path = "../metaconfig/types" }
mononoke_types = { path = "../mononoke_types" }
permission_checker = { path = "../permission_checker" }
redactedblobstore = { path = "../blobstore/redactedblobstore" }

2
eden/mononoke/lfs_server/src/config.rs
View File

@ -141,6 +141,8 @@ impl ServerConfig {
pub fn acl_check(&self) -> bool {
self.raw_server_config.acl_check
}
#[allow(dead_code)]
// TODO(harveyhunt) Use this method as a killswitch for ACL checking.
pub fn enforce_acl_check(&self) -> bool {
self.raw_server_config.enforce_acl_check
}

49
eden/mononoke/lfs_server/src/lfs_server_context.rs
View File

@ -5,6 +5,7 @@
* GNU General Public License version 2.
*/
use std::borrow::Cow;
use std::collections::HashMap;
use std::fmt::{Arguments, Write};
use std::sync::{
@ -36,6 +37,7 @@ use context::CoreContext;
use hyper::{client::HttpConnector, Client};
use hyper_openssl::HttpsConnector;
use lfs_protocol::{RequestBatch, RequestObject, ResponseBatch};
use metaconfig_types::RepoConfig;
use mononoke_types::hash::Sha256;
use mononoke_types::ContentId;
@ -49,7 +51,7 @@ pub type HttpsHyperClient = Client<HttpsConnector<HttpConnector>>;
const ACL_CHECK_ACTION: &str = "read";
struct LfsServerContextInner {
repositories: HashMap<String, (BlobRepo, ArcPermissionChecker)>,
repositories: HashMap<String, (BlobRepo, ArcPermissionChecker, RepoConfig)>,
client: Arc<HttpsHyperClient>,
server: Arc<ServerUris>,
always_wait_for_upstream: bool,
@ -65,7 +67,7 @@ pub struct LfsServerContext {
impl LfsServerContext {
pub fn new(
repositories: HashMap<String, (BlobRepo, ArcPermissionChecker)>,
repositories: HashMap<String, (BlobRepo, ArcPermissionChecker, RepoConfig)>,
server: ServerUris,
always_wait_for_upstream: bool,
max_upload_size: Option<u64>,
@ -98,11 +100,20 @@ impl LfsServerContext {
repository: String,
identities: Option<&MononokeIdentitySet>,
) -> Result<RepositoryRequestContext, LfsServerContextErrorKind> {
let (repo, aclchecker, client, server, always_wait_for_upstream, max_upload_size, config) = {
let (
repo,
aclchecker,
client,
server,
always_wait_for_upstream,
max_upload_size,
config,
enforce_acl_check,
) = {
let inner = self.inner.lock().expect("poisoned lock");
match inner.repositories.get(&repository) {
Some((repo, aclchecker)) => (
Some((repo, aclchecker, repo_config)) => (
repo.clone(),
aclchecker.clone(),
inner.client.clone(),
@ -110,6 +121,7 @@ impl LfsServerContext {
inner.always_wait_for_upstream,
inner.max_upload_size,
inner.config_handle.get(),
repo_config.enforce_lfs_acl_check,
),
None => {
return Err(LfsServerContextErrorKind::RepositoryDoesNotExist(
@ -120,7 +132,8 @@ impl LfsServerContext {
};
if config.acl_check() {
acl_check(aclchecker, identities, config.enforce_acl_check()).await?;
// TODO(harveyhunt): Use the live config's enforce_acl_check() method as a killswitch.
acl_check(aclchecker, identities, enforce_acl_check).await?;
}
Ok(RepositoryRequestContext {
@ -157,21 +170,19 @@ async fn acl_check(
identities: Option<&MononokeIdentitySet>,
enforce_acl_check: bool,
) -> Result<(), LfsServerContextErrorKind> {
if let Some(identities) = identities {
// Always make the request for permission even if enforce_acl_check is
// false, so that we get the even logged in our system
let acl_check = aclchecker
.check_set(identities, &[ACL_CHECK_ACTION])
.await
.map_err(LfsServerContextErrorKind::PermissionCheckFailed)?;
if !acl_check && enforce_acl_check {
return Err(LfsServerContextErrorKind::Forbidden.into());
} else {
return Ok(());
}
let identities: Cow<MononokeIdentitySet> = match identities {
Some(idents) => Cow::Borrowed(idents),
None => Cow::Owned(MononokeIdentitySet::new()),
};
let acl_check = aclchecker
.check_set(identities.as_ref(), &[ACL_CHECK_ACTION])
.await
.map_err(LfsServerContextErrorKind::PermissionCheckFailed)?;
if !acl_check && enforce_acl_check {
return Err(LfsServerContextErrorKind::Forbidden.into());
} else {
// For now, allow clients to connect that don't provide an identity. Once we know
// all clients have idents, we can return Forbidden.
return Ok(());
}
}

7
eden/mononoke/lfs_server/src/main.rs
View File

@ -38,6 +38,7 @@ use cmdlib::{
monitoring::{start_fb303_server, AliveService},
};
use metaconfig_parser::RepoConfigs;
use metaconfig_types::RepoConfig;
use crate::lfs_server_context::{LfsServerContext, ServerUris};
use crate::middleware::{
@ -274,7 +275,7 @@ fn main(fb: FacebookInit) -> Result<(), Error> {
&logger,
);
let hipster_acl = config.hipster_acl;
let hipster_acl = config.hipster_acl.as_ref();
let aclchecker = async {
if let Some(test_checker) = test_acl_checker {
Ok(test_checker.clone())
@ -298,9 +299,9 @@ fn main(fb: FacebookInit) -> Result<(), Error> {
let (repo, aclchecker) = try_join!(builder.build(), aclchecker)?;
Result::<(String, (BlobRepo, ArcPermissionChecker)), Error>::Ok((
Result::<(String, (BlobRepo, ArcPermissionChecker, RepoConfig)), Error>::Ok((
name,
(repo, aclchecker),
(repo, aclchecker, config),
))
}
});

6
eden/mononoke/metaconfig/parser/src/config.rs
View File

@ -209,6 +209,7 @@ fn parse_repo_config(
scuba_local_path_hooks,
hgsql_name,
hgsql_globalrevs_name,
enforce_lfs_acl_check,
..
} = repo_config;
@ -321,6 +322,8 @@ fn parse_repo_config(
}
};
let enforce_lfs_acl_check = enforce_lfs_acl_check.unwrap_or(false);
Ok(RepoConfig {
enabled,
storage_config,
@ -356,6 +359,7 @@ fn parse_repo_config(
derived_data_config,
hgsql_name,
hgsql_globalrevs_name,
enforce_lfs_acl_check,
})
}
@ -1152,6 +1156,7 @@ mod test {
},
hgsql_name: HgsqlName("fbsource".to_string()),
hgsql_globalrevs_name: HgsqlGlobalrevsName("fbsource".to_string()),
enforce_lfs_acl_check: false,
},
);
@ -1199,6 +1204,7 @@ mod test {
derived_data_config: DerivedDataConfig::default(),
hgsql_name: HgsqlName("www-foobar".to_string()),
hgsql_globalrevs_name: HgsqlGlobalrevsName("www-barfoo".to_string()),
enforce_lfs_acl_check: false,
},
);
assert_eq!(

2
eden/mononoke/metaconfig/types/src/lib.rs
View File

@ -178,6 +178,8 @@ pub struct RepoConfig {
/// Name of this repository in hgsql ... for globalrevs. This could, in some cases, not be the
/// same as HgsqlName.
pub hgsql_globalrevs_name: HgsqlGlobalrevsName,
/// Whether to enforce strict LFS ACL checks for this repo.
pub enforce_lfs_acl_check: bool,
}
/// Config for derived data

6
eden/mononoke/tests/integration/library.sh
View File

@ -647,6 +647,12 @@ scuba_local_path="$SCUBA_LOGGING_PATH"
CONFIG
fi
if [[ -n "${ENFORCE_LFS_ACL_CHECK:-}" ]]; then
cat >> "repos/$reponame/server.toml" <<CONFIG
enforce_lfs_acl_check=true
CONFIG
fi
# Normally point to common storageconfig, but if none passed, create per-repo
if [[ -z "$storageconfig" ]]; then
storageconfig="blobstore_$reponame"

15
eden/mononoke/tests/integration/test-lfs-server-acl-check.t
View File

@ -9,6 +9,8 @@
# Create a repository
$ setup_mononoke_config
$ REPOID=1 FILESTORE=1 FILESTORE_CHUNK_SIZE=10 setup_mononoke_repo_config repo1
$ ENFORCE_LFS_ACL_CHECK=1 REPOID=2 FILESTORE=1 FILESTORE_CHUNK_SIZE=10 setup_mononoke_repo_config repo2
$ LIVE_CONFIG="${TESTTMP}/live.json"
$ cat > "$LIVE_CONFIG" << EOF
> {
@ -25,6 +27,7 @@
$ LFS_LOG="$TESTTMP/lfs.log"
$ LFS_ROOT="$(lfs_server --log "$LFS_LOG" --tls --live-config "file:${LIVE_CONFIG}" --allowed-test-identity USER:test --trusted-proxy-identity USER:myusername0)"
$ LFS_URI="$LFS_ROOT/repo1"
$ LFS_URI_REPO_ENFORCE_ACL="$LFS_ROOT/repo2"
# Setup constants. These headers are normally provided by proxygen, they store
# an encoded form of the original client identity. In this case, we have
@ -32,6 +35,7 @@
$ ALLOWED_IDENT="x-fb-validated-client-encoded-identity: %7B%22ai%22%3A%20%22%22%2C%20%22ch%22%3A%20%22%22%2C%20%22it%22%3A%20%22user%22%2C%20%22id%22%3A%20%22test%22%7D"
$ DISALLOWED_IDENT="x-fb-validated-client-encoded-identity: %7B%22ai%22%3A%20%22%22%2C%20%22ch%22%3A%20%22%22%2C%20%22it%22%3A%20%22user%22%2C%20%22id%22%3A%20%22invalid%22%7D"
$ DOWNLOAD_URL="$LFS_URI/download/d28548bc21aabf04d143886d717d72375e3deecd0dafb3d110676b70a192cb5d"
$ DOWNLOAD_URL_REPO_ENFORCE_ACL="$LFS_URI_REPO_ENFORCE_ACL/download/d28548bc21aabf04d143886d717d72375e3deecd0dafb3d110676b70a192cb5d"
# Upload a blob
$ yes A 2>/dev/null | head -c 2KiB | ssldebuglfssend "$LFS_URI"
@ -46,15 +50,22 @@
$ sslcurl -s -o /dev/null -w "%{http_code}\n" "$DOWNLOAD_URL" --header "$ALLOWED_IDENT"
200
# Make a request with an invalid encoded client identity header
# Make a request with an invalid encoded client identity header. As
# enforce_lfs_acl_check is not set, this is allowed.
$ sslcurl -s -o /dev/null -w "%{http_code}\n" "$DOWNLOAD_URL" --header "$DISALLOWED_IDENT"
403
200
# Make a request without specifying an identity in the header
# NOTE: We allow this whilst we wait for all clients to get certs
$ sslcurl -s -o /dev/null -w "%{http_code}\n" "$DOWNLOAD_URL"
200
# Make a request without specifying an identity in the header, as the repo is
# configured with enforce_lfs_acl_check this request is forbidden as there is
# no ident provided.
$ sslcurl -s -o /dev/null -w "%{http_code}\n" "$DOWNLOAD_URL_REPO_ENFORCE_ACL"
403
# Disable ACL checking
$ sed -i 's/"enforce_acl_check": true/"enforce_acl_check": false/g' "$LIVE_CONFIG"
$ sleep 2