Skip to content

Commit

Permalink
Make reverse proxy check credentials when accessing internal URLs
Browse files Browse the repository at this point in the history
  • Loading branch information
mbr committed Jan 5, 2024
1 parent 28b9d9a commit bc280d9
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 25 deletions.
1 change: 0 additions & 1 deletion src/container_orchestrator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ macro_rules! try_quiet {
};
}

#[derive(Debug)]
pub(crate) struct ContainerOrchestrator {
podman: Podman,
reverse_proxy: Arc<ReverseProxy>,
Expand Down
16 changes: 6 additions & 10 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ async fn main() -> anyhow::Result<()> {

debug!(?cfg, "loaded configuration");

let rockslide_pw = cfg.rockslide.master_key.as_secret_string();
let auth_provider = Arc::new(cfg.rockslide.master_key);

let local_ip: IpAddr = if podman_is_remote() {
info!("podman is remote, trying to guess IP address");
let local_hostname = gethostname();
Expand All @@ -78,12 +81,9 @@ async fn main() -> anyhow::Result<()> {
let local_addr = SocketAddr::from(([127, 0, 0, 1], cfg.reverse_proxy.http_bind.port()));
info!(%local_addr, "guessing local registry address");

let reverse_proxy = ReverseProxy::new();
let reverse_proxy = ReverseProxy::new(auth_provider.clone());

let credentials = (
"rockslide-podman".to_owned(),
cfg.rockslide.master_key.as_secret_string(),
);
let credentials = ("rockslide-podman".to_owned(), rockslide_pw);
let orchestrator = Arc::new(ContainerOrchestrator::new(
&cfg.containers.podman_path,
reverse_proxy.clone(),
Expand All @@ -97,11 +97,7 @@ async fn main() -> anyhow::Result<()> {
orchestrator.synchronize_all().await?;
orchestrator.updated_published_set().await;

let registry = ContainerRegistry::new(
&cfg.registry.storage_path,
orchestrator,
cfg.rockslide.master_key,
)?;
let registry = ContainerRegistry::new(&cfg.registry.storage_path, orchestrator, auth_provider)?;

let app = Router::new()
.merge(registry.make_router())
Expand Down
12 changes: 4 additions & 8 deletions src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,24 +92,20 @@ impl IntoResponse for AppError {

pub(crate) struct ContainerRegistry {
realm: String,
auth_provider: Box<dyn AuthProvider>,
auth_provider: Arc<dyn AuthProvider>,
storage: Box<dyn RegistryStorage>,
hooks: Box<dyn RegistryHooks>,
}

impl ContainerRegistry {
pub(crate) fn new<
P: AsRef<std::path::Path>,
T: RegistryHooks + 'static,
A: AuthProvider + 'static,
>(
pub(crate) fn new<P: AsRef<std::path::Path>, T: RegistryHooks + 'static>(
storage_path: P,
orchestrator: T,
auth_provider: A,
auth_provider: Arc<dyn AuthProvider>,
) -> Result<Arc<Self>, FilesystemStorageError> {
Ok(Arc::new(ContainerRegistry {
realm: "ContainerRegistry".to_string(),
auth_provider: Box::new(auth_provider),
auth_provider: auth_provider,
storage: Box::new(FilesystemStorage::new(storage_path)?),
hooks: Box::new(orchestrator),
}))
Expand Down
30 changes: 24 additions & 6 deletions src/reverse_proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,21 @@ use axum::{
Method, StatusCode, Uri,
},
response::{IntoResponse, Response},
Router,
RequestExt, Router,
};
use itertools::Itertools;
use tokio::sync::RwLock;
use tracing::{trace, warn};

use crate::{
container_orchestrator::{ContainerOrchestrator, PublishedContainer},
registry::{storage::ImageLocation, ManifestReference, Reference},
registry::{
storage::ImageLocation, AuthProvider, ManifestReference, Reference, UnverifiedCredentials,
},
};

#[derive(Debug)]
pub(crate) struct ReverseProxy {
auth_provider: Arc<dyn AuthProvider>,
client: reqwest::Client,
routing_table: RwLock<RoutingTable>,
orchestrator: OnceLock<Arc<ContainerOrchestrator>>,
Expand Down Expand Up @@ -173,6 +175,7 @@ enum AppError {
InternalUrlInvalid,
AssertionFailed(&'static str),
NonUtf8Header,
AuthFailure(StatusCode),
Internal(anyhow::Error),
}

Expand All @@ -184,6 +187,7 @@ impl Display for AppError {
AppError::InternalUrlInvalid => f.write_str("internal url invalid"),
AppError::AssertionFailed(msg) => f.write_str(msg),
AppError::NonUtf8Header => f.write_str("a header contained non-utf8 data"),
AppError::AuthFailure(_) => f.write_str("authentication missing or not present"),
AppError::Internal(err) => Display::fmt(err, f),
}
}
Expand All @@ -207,6 +211,7 @@ impl IntoResponse for AppError {
AppError::InternalUrlInvalid => StatusCode::NOT_FOUND.into_response(),
AppError::AssertionFailed(msg) => (StatusCode::NOT_FOUND, msg).into_response(),
AppError::NonUtf8Header => StatusCode::BAD_REQUEST.into_response(),
AppError::AuthFailure(status) => status.into_response(),
AppError::Internal(err) => {
(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()).into_response()
}
Expand All @@ -215,8 +220,9 @@ impl IntoResponse for AppError {
}

impl ReverseProxy {
pub(crate) fn new() -> Arc<Self> {
pub(crate) fn new(auth_provider: Arc<dyn AuthProvider>) -> Arc<Self> {
Arc::new(ReverseProxy {
auth_provider,
client: reqwest::Client::new(),
routing_table: RwLock::new(Default::default()),
orchestrator: OnceLock::new(),
Expand All @@ -240,6 +246,7 @@ impl ReverseProxy {
pub(crate) fn set_orchestrator(&self, orchestrator: Arc<ContainerOrchestrator>) -> &Self {
self.orchestrator
.set(orchestrator)
.map_err(|_| ())
.expect("set already set orchestrator");
self
}
Expand Down Expand Up @@ -271,7 +278,18 @@ async fn route_request(
let dest = match dest_uri {
Destination::ReverseProxied(dest) => dest,
Destination::Internal(uri) => {
todo!("check access (master password)");
let method = request.method().clone();
// Note: The auth functionality has been lifted from `registry`. It may need to be
// refactored out because of that.
let creds = request
.extract::<UnverifiedCredentials, _>()
.await
.map_err(AppError::AuthFailure)?;

// Any internal URL is subject to requiring auth through the master key.
if !rp.auth_provider.check_credentials(&creds).await {
todo!("return 403");
}

let remainder = uri
.path()
Expand All @@ -297,7 +315,7 @@ async fn route_request(
.get()
.ok_or_else(|| AppError::AssertionFailed("no orchestrator configured"))?;

return match *request.method() {
return match method {
Method::GET => {
let config = orchestrator
.load_config(&manifest_reference)
Expand Down

0 comments on commit bc280d9

Please sign in to comment.