Skip to content

Commit

Permalink
Add: client authorization on endpoints
Browse files Browse the repository at this point in the history
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 certifactes, by the used API key.

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

When neither is configured the scans endpoints are unreachable.
  • Loading branch information
nichtsfrei committed Nov 2, 2023
1 parent bef154c commit 483d745
Show file tree
Hide file tree
Showing 14 changed files with 821 additions and 220 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
116 changes: 62 additions & 54 deletions rust/examples/tls/Self-Signed mTLS Method/server_certificates.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,57 +3,65 @@
#
# SPDX-License-Identifier: GPL-2.0-or-later

set -xe

openssl req -nodes \
-x509 \
-days 3650 \
-newkey rsa:4096 \
-keyout ca.key \
-out ca.cert \
-sha256 \
-batch \
-subj "/CN=ponytown RSA CA"

openssl req -nodes \
-newkey rsa:3072 \
-keyout inter.key \
-out inter.req \
-sha256 \
-batch \
-subj "/CN=ponytown RSA level 2 intermediate"

openssl req -nodes \
-newkey rsa:2048 \
-keyout end.key \
-out end.req \
-sha256 \
-batch \
-subj "/CN=testserver.com"

openssl rsa \
-in end.key \
-out server.rsa

openssl x509 -req \
-in inter.req \
-out inter.cert \
-CA ca.cert \
-CAkey ca.key \
-sha256 \
-days 3650 \
-set_serial 123 \
-extensions v3_inter -extfile ../openssl.cnf

openssl x509 -req \
-in end.req \
-out end.cert \
-CA inter.cert \
-CAkey inter.key \
-sha256 \
-days 2000 \
-set_serial 456 \
-extensions v3_end -extfile ../openssl.cnf

cat end.cert inter.cert ca.cert > server.pem
rm *.key *.cert *.req
set -e
generate_certificates()
{
out="$1"
name="$2"
printf "generating $out for $name\t"
openssl req -nodes \
-x509 \
-days 3650 \
-newkey rsa:4096 \
-keyout ca.key \
-out ca.cert \
-sha256 \
-batch \
-subj "/CN=$name RSA CA"

openssl req -nodes \
-newkey rsa:3072 \
-keyout inter.key \
-out inter.req \
-sha256 \
-batch \
-subj "/CN=$name RSA level 2 intermediate"

openssl req -nodes \
-newkey rsa:2048 \
-keyout end.key \
-out end.req \
-sha256 \
-batch \
-subj "/CN=testserver.com"

openssl rsa \
-in end.key \
-out $out.rsa

openssl x509 -req \
-in inter.req \
-out inter.cert \
-CA ca.cert \
-CAkey ca.key \
-sha256 \
-days 3650 \
-set_serial 123 \
-extensions v3_inter -extfile openssl.cnf

openssl x509 -req \
-in end.req \
-out end.cert \
-CA inter.cert \
-CAkey inter.key \
-sha256 \
-days 2000 \
-set_serial 456 \
-extensions v3_end -extfile openssl.cnf

cat end.cert inter.cert ca.cert > $out.pem
rm *.key *.cert *.req
printf "done\n"
}

generate_certificates "server" "ponytown"
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 = ["secret_extraction", "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 483d745

Please sign in to comment.