Skip to content

Commit

Permalink
Add: client authorization on endpoints (#1521)
Browse files Browse the repository at this point in the history
Add: client authorization on endpoints

To prevent that a registered client can see results
or scan of another client a differentiation factor
is introduces.

The scans and results will now be stored as an u64->information and the
key is either calculated by the used client certificate or, when openvasd
is started without client certificates, by the used API key.

When client certificates and the API key is configured than the client
certificates are getting used and the API key is discarded.

When neither is configured the scans endpoints are unreachable.
  • Loading branch information
nichtsfrei authored Nov 6, 2023
1 parent 3f1ea08 commit 6af3aab
Show file tree
Hide file tree
Showing 13 changed files with 763 additions and 166 deletions.
2 changes: 2 additions & 0 deletions rust/.gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
target/
libsrc/
*.rsa
*.cert
2 changes: 2 additions & 0 deletions rust/examples/openvasd/config.example.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
[feed]
# path to the openvas feed. This is required for the /vts endpoint.
path = "/var/lib/openvas/plugins"
# disables or enables the signnature check
signature_check = true

[feed.check_interval]
# how often the feed should be checked for updates
Expand Down
12 changes: 12 additions & 0 deletions rust/infisto/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,18 @@ impl IndexedFileStorer {
.map_err(Error::Remove)?;
Ok(())
}

/// Removes base dir and all its content.
///
/// # Safety
/// Does remove the whole base dir and its content.
/// Do not use carelessly.
pub unsafe fn remove_base(self) -> Result<(), Error> {
fs::remove_dir_all(self.base)
.map_err(|e| e.kind())
.map_err(Error::Remove)
.map(|_| ())
}
}

impl IndexedByteStorage for IndexedFileStorer {
Expand Down
2 changes: 1 addition & 1 deletion rust/openvasd/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ serde_json = "1.0.96"
serde = { version = "1.0.163", features = ["derive"] }
uuid = {version = "1", features = ["v4", "fast-rng", "serde"]}
hyper-rustls = "0.24.0"
rustls = "0.21.1"
rustls = {version = "0.21.1", features = ["dangerous_configuration"]}
tokio-rustls = "0.24.0"
futures-util = "0.3.28"
rustls-pemfile = "1.0.2"
Expand Down
4 changes: 2 additions & 2 deletions rust/openvasd/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,8 @@ impl Config {
where
P: AsRef<std::path::Path> + std::fmt::Display,
{
let config = std::fs::read_to_string(path).unwrap_or_default();
toml::from_str(&config).unwrap_or_default()
let config = std::fs::read_to_string(path).unwrap();
toml::from_str(&config).unwrap()
}

pub fn load() -> Self {
Expand Down
3 changes: 2 additions & 1 deletion rust/openvasd/src/controller/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ impl<S, DB, T> ContextBuilder<S, DB, T> {
if let Some(fp) = self.feed_config.as_ref() {
let loader = nasl_interpreter::FSPluginLoader::new(fp.path.clone());
let dispatcher: DefaultDispatcher<String> = DefaultDispatcher::default();
let version = feed::version(&loader, &dispatcher).unwrap();
let version =
feed::version(&loader, &dispatcher).unwrap_or_else(|_| String::from("UNDEFINED"));
self.response.set_feed_version(&version);
}
self
Expand Down
102 changes: 72 additions & 30 deletions rust/openvasd/src/controller/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,18 @@
//!
//! All known paths must be handled in the entrypoint function.

use std::{fmt::Display, sync::Arc};
use std::{
fmt::Display,
sync::{Arc, RwLock},
};

use super::context::Context;
use super::{context::Context, ClientIdentifier};
use hyper::{Body, Method, Request, Response};

use crate::scan::{self, Error, ScanDeleter, ScanStarter, ScanStopper};
use crate::{
controller::ClientHash,
scan::{self, Error, ScanDeleter, ScanStarter, ScanStopper},
};

enum HealthOpts {
/// Ready
Expand All @@ -38,6 +44,10 @@ enum KnownPaths {
}

impl KnownPaths {
pub fn requires_id(&self) -> bool {
!matches!(self, Self::Health(_) | Self::Vts)
}

#[tracing::instrument]
/// Parses a path and returns the corresponding `KnownPaths` variant.
fn from_path(path: &str) -> Self {
Expand All @@ -60,13 +70,20 @@ impl KnownPaths {
Some("alive") => KnownPaths::Health(HealthOpts::Alive),
Some("started") => KnownPaths::Health(HealthOpts::Started),
_ => KnownPaths::Unknown,
}
},
_ => {
tracing::trace!("Unknown path: {path}");
KnownPaths::Unknown
}
}
}

fn scan_id(&self) -> Option<&str> {
match self {
Self::Scans(Some(id)) | Self::ScanResults(id, _) | Self::ScanStatus(id) => Some(id),
_ => None,
}
}
}

impl Display for KnownPaths {
Expand All @@ -89,53 +106,76 @@ impl Display for KnownPaths {
}

/// Is used to handle all incomng requests.
///
/// First it will be checked if a known path is requested and if the method is supported.
/// Than corresponding functions will be called to handle the request.
pub async fn entrypoint<'a, S, DB>(
req: Request<Body>,
ctx: Arc<Context<S, DB>>,
cid: Arc<RwLock<ClientIdentifier>>,
) -> Result<Response<Body>, Error>
where
S: ScanStarter
+ ScanStopper
+ ScanDeleter
+ scan::ScanResultFetcher
+ std::marker::Send
+ 'static
+ std::marker::Sync,
+ std::marker::Sync
+ 'static,
DB: crate::storage::Storage + std::marker::Send + 'static + std::marker::Sync,
{
use KnownPaths::*;
// on head requests we just return an empty response without checking the api key
tracing::trace!(
"{} {}:{:?}",
req.method(),
req.uri().path(),
req.uri().query()
);
if req.method() == Method::HEAD {
return Ok(ctx.response.empty(hyper::StatusCode::OK));
}
let kp = KnownPaths::from_path(req.uri().path());
if let Some(key) = ctx.api_key.as_ref() {
match req.headers().get("x-api-key") {
Some(v) if v == key => {}
Some(v) => {
tracing::debug!("{} {} invalid key: {:?}", req.method(), kp, v);
return Ok(ctx.response.unauthorized());
}
_ => {
tracing::debug!("{} {} unauthorized", req.method(), kp);
return Ok(ctx.response.unauthorized());
let cid: Option<ClientHash> = {
let cid = cid.read().unwrap();
match &*cid {
ClientIdentifier::Unknown => {
if let Some(key) = ctx.api_key.as_ref() {
match req.headers().get("x-api-key") {
Some(v) if v == key => ctx.api_key.as_ref().map(|x| x.into()),
Some(v) => {
tracing::debug!("{} {} invalid key: {:?}", req.method(), kp, v);
None
}
_ => None,
}
} else {
None
}
}
ClientIdentifier::Known(cid) => Some(cid.clone()),
}
};

if kp.requires_id() && cid.is_none() {
tracing::debug!("{} {} unauthorized", req.method(), kp);
return Ok(ctx.response.unauthorized());
}
let cid = cid.unwrap_or_default();

if let Some(scan_id) = kp.scan_id() {
if !ctx.db.is_client_allowed(scan_id.to_owned(), &cid).await? {
tracing::debug!(
"client {:x?} is not allowed to operate on scan {} ",
&cid.0,
scan_id
);
// we return 404 instead of 401 to not leak any ids
return Ok(ctx.response.not_found("scans", scan_id));
}
}

tracing::debug!(
"{} {}:{:?}",
req.method(),
req.uri().path(),
req.uri().query(),
);
match (req.method(), kp) {
(&Method::GET, Health(HealthOpts::Alive)) |
(&Method::GET, Health(HealthOpts::Started)) =>
Ok(ctx.response.empty(hyper::StatusCode::OK)),
(&Method::GET, Health(HealthOpts::Alive)) | (&Method::GET, Health(HealthOpts::Started)) => {
Ok(ctx.response.empty(hyper::StatusCode::OK))
}
(&Method::GET, Health(HealthOpts::Ready)) => {
let oids = ctx.db.oids().await?;
if oids.count() == 0 {
Expand All @@ -156,6 +196,7 @@ where
let resp = ctx.response.created(&id);
scan.scan_id = Some(id.clone());
ctx.db.insert_scan(scan).await?;
ctx.db.add_scan_client_id(id.clone(), cid).await?;
tracing::debug!("Scan with ID {} created", &id);
Ok(resp)
}
Expand Down Expand Up @@ -202,7 +243,7 @@ where
}
(&Method::GET, Scans(None)) => {
if ctx.enable_get_scans {
match ctx.db.get_scan_ids().await {
match ctx.db.get_scans_of_client_id(&cid).await {
Ok(scans) => Ok(ctx.response.ok(&scans)),
Err(e) => Ok(ctx.response.internal_server_error(&e)),
}
Expand All @@ -226,7 +267,8 @@ where
ctx.scanner.stop_scan(id.clone()).await?;
}
ctx.db.remove_scan(&id).await?;
ctx.scanner.delete_scan(id).await?;
ctx.scanner.delete_scan(id.clone()).await?;
ctx.db.remove_scan_id(id).await?;
Ok(ctx.response.no_content())
}
Err(crate::storage::Error::NotFound) => Ok(ctx.response.not_found("scans", &id)),
Expand Down
Loading

0 comments on commit 6af3aab

Please sign in to comment.