Use async reqwest for openid connect checks (#645)

PR #646

* cargo update
* Use an enum instead of a trait (prepare to allow async)
* Use async reqwest client for openid connect checks.
* Move extract bearer token function to util::httpclient.

Co-authored-by: Ximon Eighteen <3304436+ximon18@users.noreply.github.com>
This commit is contained in:
Tim Bruijnzeels
2021-09-06 13:38:17 +02:00
committed by GitHub
co-authored by Ximon Eighteen
parent 1dfffed3ff
commit 27fd25dcb2
11 changed files with 702 additions and 1456 deletions
Generated
+405 -1214
View File
File diff suppressed because it is too large Load Diff
+1 -2
View File
@@ -42,7 +42,6 @@ openssl = { version = "^0.10", features = ["v110"] }
oso = { version = "^0.12", optional = true, default_features = false }
regex = { version = "^1.4", optional = true, default_features = false, features = ["std"] }
reqwest = { version = "0.11", features = ["json"] }
reqwestblocking = { version = "0.9.24", optional = true, package = "reqwest" }
rpassword = { version = "^5.0", optional = true }
# rpki = { version = "0.11.1-dev", features = [ "repository", "rrdp", "serde" ], git = "https://github.com/NLnetLabs/rpki-rs.git" }
rpki = { version = "0.12.2", features = [ "repository", "rrdp", "serde" ] }
@@ -67,7 +66,7 @@ rustc_version = "0.2.3"
[features]
default = [ "multi-user" ]
rta = []
multi-user = [ "basic-cookies", "jmespatch/sync", "regex", "oso", "openidconnect", "reqwestblocking", "rpassword", "scrypt", "unicode-normalization", "urlparse" ]
multi-user = [ "basic-cookies", "jmespatch/sync", "regex", "oso", "openidconnect", "rpassword", "scrypt", "unicode-normalization", "urlparse" ]
functional-tests = []
ui-tests = []
extra-debug = [ "rpki/extra-debug" ]
+36 -6
View File
@@ -58,6 +58,16 @@ fn report_delete(uri: &str, content_type: Option<&str>, token: Option<&Token>) {
}
}
/// Gets the Bearer token from the request header, if present.
pub fn get_bearer_token(request: &hyper::Request<hyper::Body>) -> Option<Token> {
request
.headers()
.get(hyper::header::AUTHORIZATION)
.and_then(|value| value.to_str().ok())
.map(|header_string| header_string.strip_prefix("Bearer ").map(|s| Token::from(s.trim())))
.flatten()
}
/// Performs a GET request that expects a json response that can be
/// deserialized into the an owned value of the expected type. Returns an error
/// if nothing is returned.
@@ -216,8 +226,18 @@ fn load_root_cert(path: &str) -> Result<reqwest::Certificate, Error> {
reqwest::Certificate::from_pem(file.as_ref()).map_err(Error::https_root_cert_error)
}
/// Default client for Krill use cases.
pub fn client(uri: &str) -> Result<reqwest::Client, Error> {
let mut builder = reqwest::ClientBuilder::new().timeout(Duration::from_secs(HTTP_CLIENT_TIMEOUT_SECS));
client_with_tweaks(uri, Duration::from_secs(HTTP_CLIENT_TIMEOUT_SECS), true)
}
/// Client with tweaks - in particular needed by the openid connect client
pub fn client_with_tweaks(uri: &str, timeout: Duration, allow_redirects: bool) -> Result<reqwest::Client, Error> {
let mut builder = reqwest::ClientBuilder::new().timeout(timeout);
if !allow_redirects {
builder = builder.redirect(reqwest::redirect::Policy::none());
}
if let Ok(cert_list) = env::var(KRILL_HTTPS_ROOT_CERTS_ENV) {
for path in cert_list.split(':') {
@@ -241,7 +261,10 @@ fn headers(content_type: Option<&str>, token: Option<&Token>) -> Result<HeaderMa
headers.insert(CONTENT_TYPE, HeaderValue::from_str(content_type)?);
}
if let Some(token) = token {
headers.insert("Authorization", HeaderValue::from_str(&format!("Bearer {}", token))?);
headers.insert(
hyper::header::AUTHORIZATION,
HeaderValue::from_str(&format!("Bearer {}", token))?,
);
}
Ok(headers)
}
@@ -292,7 +315,10 @@ pub enum Error {
ErrorWithBody(StatusCode, String),
ErrorWithJson(StatusCode, ErrorResponse),
JsonError(serde_json::Error),
InvalidHeader(InvalidHeaderValue),
InvalidHeaderName,
InvalidHeaderValue,
InvalidMethod(String),
InvalidStatusCode(u16),
EmptyResponse,
UnexpectedResponse(String),
HttpsRootCertError(String),
@@ -307,7 +333,10 @@ impl fmt::Display for Error {
Error::ErrorWithBody(code, e) => write!(f, "Status: {}, Error: {}", code, e),
Error::ErrorWithJson(code, res) => write!(f, "Status: {}, ErrorResponse: {}", code, res),
Error::JsonError(e) => e.fmt(f),
Error::InvalidHeader(e) => e.fmt(f),
Error::InvalidHeaderName => write!(f, "failed parse header name"),
Error::InvalidHeaderValue => write!(f, "failed parse header value"),
Error::InvalidMethod(m) => write!(f, "unrecognised method requested: '{}'", m),
Error::InvalidStatusCode(code) => write!(f, "unrecognised status code in response: '{}'", code),
Error::EmptyResponse => write!(f, "Empty response received from server"),
Error::UnexpectedResponse(s) => write!(f, "Unexpected response: {}", s),
Error::HttpsRootCertError(e) => write!(
@@ -355,7 +384,8 @@ impl From<serde_json::Error> for Error {
}
impl From<InvalidHeaderValue> for Error {
fn from(v: InvalidHeaderValue) -> Self {
Error::InvalidHeader(v)
fn from(_v: InvalidHeaderValue) -> Self {
// note InvalidHeaderValue is a marker and contains no further information.
Error::InvalidHeaderValue
}
}
+83 -35
View File
@@ -14,12 +14,15 @@ use crate::daemon::config::Config;
use crate::daemon::http::HttpResponse;
use crate::{commons::api::Token, daemon::auth::common::permissions::Permission};
#[cfg(feature = "multi-user")]
use crate::daemon::auth::providers::{ConfigFileAuthProvider, OpenIDConnectAuthProvider};
//------------ Authorizer ----------------------------------------------------
/// An AuthProvider authenticates and authorizes a given token.
///
/// An AuthProvider is expected to configure itself using the global Krill
/// [`CONFIG`] object. This avoids propagatation of potentially many provider
/// [`CONFIG`] object. This avoids propagation of potentially many provider
/// specific configuration values from the calling code to the provider
/// implementation.
///
@@ -30,34 +33,82 @@ use crate::{commons::api::Token, daemon::auth::common::permissions::Permission};
/// * discovery - as an interactive client where should I send my users to
/// login and logout?
/// * introspection - who is the currently "logged in" user?
pub trait AuthProvider: Send + Sync {
fn get_bearer_token(&self, request: &hyper::Request<hyper::Body>) -> Option<Token> {
if let Some(header) = request.headers().get("Authorization") {
if let Ok(header) = header.to_str() {
if header.len() > 6 {
let (bearer, token) = header.split_at(6);
let bearer = bearer.trim();
pub enum AuthProvider {
Token(AdminTokenAuthProvider),
if "Bearer" == bearer {
return Some(Token::from(token.trim()));
}
}
}
#[cfg(feature = "multi-user")]
ConfigFile(ConfigFileAuthProvider),
#[cfg(feature = "multi-user")]
OpenIdConnect(OpenIDConnectAuthProvider),
}
impl From<AdminTokenAuthProvider> for AuthProvider {
fn from(provider: AdminTokenAuthProvider) -> Self {
AuthProvider::Token(provider)
}
}
#[cfg(feature = "multi-user")]
impl From<ConfigFileAuthProvider> for AuthProvider {
fn from(provider: ConfigFileAuthProvider) -> Self {
AuthProvider::ConfigFile(provider)
}
}
#[cfg(feature = "multi-user")]
impl From<OpenIDConnectAuthProvider> for AuthProvider {
fn from(provider: OpenIDConnectAuthProvider) -> Self {
AuthProvider::OpenIdConnect(provider)
}
}
impl AuthProvider {
pub async fn authenticate(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<Option<ActorDef>> {
match &self {
AuthProvider::Token(provider) => provider.authenticate(request),
#[cfg(feature = "multi-user")]
AuthProvider::ConfigFile(provider) => provider.authenticate(request),
#[cfg(feature = "multi-user")]
AuthProvider::OpenIdConnect(provider) => provider.authenticate(request).await,
}
None
}
fn authenticate(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<Option<ActorDef>>;
fn get_login_url(&self) -> KrillResult<HttpResponse>;
fn login(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<LoggedInUser>;
fn logout(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<HttpResponse>;
pub async fn get_login_url(&self) -> KrillResult<HttpResponse> {
match &self {
AuthProvider::Token(provider) => provider.get_login_url(),
#[cfg(feature = "multi-user")]
AuthProvider::ConfigFile(provider) => provider.get_login_url(),
#[cfg(feature = "multi-user")]
AuthProvider::OpenIdConnect(provider) => provider.get_login_url().await,
}
}
pub async fn login(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<LoggedInUser> {
match &self {
AuthProvider::Token(provider) => provider.login(request),
#[cfg(feature = "multi-user")]
AuthProvider::ConfigFile(provider) => provider.login(request),
#[cfg(feature = "multi-user")]
AuthProvider::OpenIdConnect(provider) => provider.login(request).await,
}
}
pub async fn logout(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<HttpResponse> {
match &self {
AuthProvider::Token(provider) => provider.logout(request),
#[cfg(feature = "multi-user")]
AuthProvider::ConfigFile(provider) => provider.logout(request),
#[cfg(feature = "multi-user")]
AuthProvider::OpenIdConnect(provider) => provider.logout(request).await,
}
}
}
/// This type is responsible for checking authorizations when the API is
/// accessed.
pub struct Authorizer {
primary_provider: Box<dyn AuthProvider>,
primary_provider: AuthProvider,
legacy_provider: Option<AdminTokenAuthProvider>,
policy: AuthPolicy,
private_attributes: Vec<String>,
@@ -77,11 +128,8 @@ impl Authorizer {
/// `P` an instance of some other provider, an instance of
/// [AdminTokenAuthProvider] will also be created. This will be used as a
/// fallback when Lagosta is configured to use some other [AuthProvider].
pub fn new<P>(config: Arc<Config>, provider: P) -> KrillResult<Self>
where
P: AuthProvider + Any,
{
let value_any = &provider as &dyn Any;
pub fn new(config: Arc<Config>, primary_provider: AuthProvider) -> KrillResult<Self> {
let value_any = &primary_provider as &dyn Any;
let is_admin_token_provider = value_any.downcast_ref::<AdminTokenAuthProvider>().is_some();
let legacy_provider = if is_admin_token_provider {
@@ -103,14 +151,14 @@ impl Authorizer {
let private_attributes = vec!["role".to_string()];
Ok(Authorizer {
primary_provider: Box::new(provider),
primary_provider,
legacy_provider,
policy: AuthPolicy::new(config)?,
private_attributes,
})
}
pub fn actor_from_request(&self, request: &hyper::Request<hyper::Body>) -> Actor {
pub async fn actor_from_request(&self, request: &hyper::Request<hyper::Body>) -> Actor {
trace!("Determining actor for request {:?}", &request);
// Try the legacy provider first, if any
@@ -122,7 +170,7 @@ impl Authorizer {
// Try the real provider if we did not already successfully authenticate
authenticate_res = match authenticate_res {
Ok(Some(res)) => Ok(Some(res)),
_ => self.primary_provider.authenticate(request),
_ => self.primary_provider.authenticate(request).await,
};
// Create an actor based on the authentication result
@@ -135,7 +183,7 @@ impl Authorizer {
// error during authentication
Err(err) => {
// reveives a commons::error::Error, but we need an ApiAuthError
// receives a commons::error::Error, but we need an ApiAuthError
self.actor_from_def(ACTOR_DEF_ANON.with_auth_error(err))
}
};
@@ -151,14 +199,14 @@ impl Authorizer {
/// Return the URL at which an end-user should be directed to login with the
/// configured provider.
pub fn get_login_url(&self) -> KrillResult<HttpResponse> {
self.primary_provider.get_login_url()
pub async fn get_login_url(&self) -> KrillResult<HttpResponse> {
self.primary_provider.get_login_url().await
}
/// Submit credentials directly to the configured provider to establish a
/// login session, if supported by the configured provider.
pub fn login(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<LoggedInUser> {
let user = self.primary_provider.login(request)?;
pub async fn login(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<LoggedInUser> {
let user = self.primary_provider.login(request).await?;
// The user has passed authentication, but may still not be
// authorized to login as that requires a check against the policy
@@ -197,8 +245,8 @@ impl Authorizer {
/// Return the URL at which an end-user should be directed to logout with
/// the configured provider.
pub fn logout(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<HttpResponse> {
self.primary_provider.logout(request)
pub async fn logout(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<HttpResponse> {
self.primary_provider.logout(request).await
}
}
+8 -7
View File
@@ -1,10 +1,11 @@
use std::sync::Arc;
use crate::commons::error::Error;
use crate::commons::util::httpclient;
use crate::commons::KrillResult;
use crate::commons::{actor::ActorDef, api::Token};
use crate::constants::ACTOR_DEF_ADMIN_TOKEN;
use crate::daemon::auth::{AuthProvider, LoggedInUser};
use crate::daemon::auth::LoggedInUser;
use crate::daemon::config::Config;
use crate::daemon::http::HttpResponse;
@@ -27,13 +28,13 @@ impl AdminTokenAuthProvider {
}
}
impl AuthProvider for AdminTokenAuthProvider {
fn authenticate(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<Option<ActorDef>> {
impl AdminTokenAuthProvider {
pub fn authenticate(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<Option<ActorDef>> {
if log_enabled!(log::Level::Trace) {
trace!("Attempting to authenticate the request..");
}
let res = match self.get_bearer_token(request) {
let res = match httpclient::get_bearer_token(request) {
Some(token) if token == self.required_token => Ok(Some(ACTOR_DEF_ADMIN_TOKEN)),
Some(_) => Err(Error::ApiInvalidCredentials("Invalid bearer token".to_string())),
None => Ok(None),
@@ -46,12 +47,12 @@ impl AuthProvider for AdminTokenAuthProvider {
res
}
fn get_login_url(&self) -> KrillResult<HttpResponse> {
pub fn get_login_url(&self) -> KrillResult<HttpResponse> {
// Direct Lagosta to show the user the Lagosta API token login form
Ok(HttpResponse::text_no_cache(LAGOSTA_LOGIN_ROUTE_PATH.into()))
}
fn login(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<LoggedInUser> {
pub fn login(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<LoggedInUser> {
match self.authenticate(request)? {
Some(actor_def) => Ok(LoggedInUser {
token: self.required_token.clone(),
@@ -62,7 +63,7 @@ impl AuthProvider for AdminTokenAuthProvider {
}
}
fn logout(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<HttpResponse> {
pub fn logout(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<HttpResponse> {
if let Ok(Some(actor)) = self.authenticate(request) {
info!("User logged out: {}", actor.name.as_str());
}
@@ -2,12 +2,13 @@ use std::{collections::HashMap, sync::Arc};
use urlparse::{urlparse, GetQuery};
use crate::commons::util::httpclient;
use crate::commons::KrillResult;
use crate::commons::{actor::ActorDef, api::Token};
use crate::daemon::auth::common::crypt;
use crate::daemon::auth::common::session::*;
use crate::daemon::auth::providers::config_file::config::ConfigUserDetails;
use crate::daemon::auth::{Auth, AuthProvider, LoggedInUser};
use crate::daemon::auth::{Auth, LoggedInUser};
use crate::daemon::config::Config;
use crate::daemon::http::HttpResponse;
use crate::{commons::error::Error, daemon::auth::common::crypt::CryptState};
@@ -86,7 +87,7 @@ impl ConfigFileAuthProvider {
}
fn get_auth(&self, request: &hyper::Request<hyper::Body>) -> Option<Auth> {
if let Some(password_hash) = self.get_bearer_token(request) {
if let Some(password_hash) = httpclient::get_bearer_token(request) {
if let Some(query) = urlparse(request.uri().to_string()).get_parsed_query() {
if let Some(id) = query.get_first_from_str("id") {
return Some(Auth::IdAndPasswordHash { id, password_hash });
@@ -97,13 +98,13 @@ impl ConfigFileAuthProvider {
}
}
impl AuthProvider for ConfigFileAuthProvider {
fn authenticate(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<Option<ActorDef>> {
impl ConfigFileAuthProvider {
pub fn authenticate(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<Option<ActorDef>> {
if log_enabled!(log::Level::Trace) {
trace!("Attempting to authenticate the request..");
}
let res = match self.get_bearer_token(request) {
let res = match httpclient::get_bearer_token(request) {
Some(token) => {
// see if we can decode, decrypt and deserialize the users token
// into a login session structure
@@ -123,12 +124,12 @@ impl AuthProvider for ConfigFileAuthProvider {
res
}
fn get_login_url(&self) -> KrillResult<HttpResponse> {
pub fn get_login_url(&self) -> KrillResult<HttpResponse> {
// Direct Lagosta to show the user the Lagosta API token login form
Ok(HttpResponse::text_no_cache(LAGOSTA_LOGIN_ROUTE_PATH.into()))
}
fn login(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<LoggedInUser> {
pub fn login(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<LoggedInUser> {
if let Some(Auth::IdAndPasswordHash { id, password_hash }) = self.get_auth(request) {
use scrypt::scrypt;
@@ -184,8 +185,8 @@ impl AuthProvider for ConfigFileAuthProvider {
}
}
fn logout(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<HttpResponse> {
match self.get_bearer_token(request) {
pub fn logout(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<HttpResponse> {
match httpclient::get_bearer_token(request) {
Some(token) => {
self.session_cache.remove(&token);
@@ -1,125 +1,16 @@
use std::{env, path::PathBuf, str::FromStr, time::Duration};
use std::{str::FromStr, time::Duration};
use reqwest::Response;
use crate::{
commons::util::file,
constants::KRILL_HTTPS_ROOT_CERTS_ENV,
commons::error::Error,
commons::util::httpclient,
constants::{test_mode_enabled, OPENID_CONNECT_HTTP_CLIENT_TIMEOUT_SECS},
};
use crate::commons::error::Error;
use crate::commons::util::httpclient;
// Based on httpclient::load_root_cert(). We can't just use the original function as the invoked functions are specific
// to types in the reqwest crate version being used.
fn load_root_cert(path: &str) -> Result<reqwestblocking::Certificate, httpclient::Error> {
let path = PathBuf::from_str(path).map_err(httpclient::Error::https_root_cert_error)?;
let file = file::read(&path).map_err(httpclient::Error::https_root_cert_error)?;
reqwestblocking::Certificate::from_pem(file.as_ref()).map_err(httpclient::Error::https_root_cert_error)
}
fn openid_connect_provider_timeout() -> Duration {
if test_mode_enabled() {
Duration::from_secs(5)
} else {
Duration::from_secs(OPENID_CONNECT_HTTP_CLIENT_TIMEOUT_SECS)
}
}
// Based on httpclient::client(). We can't just use the original function as the invoked functions are specific to
// types in the reqwest crate version being used.
fn configure_http_client_for_krill(
mut builder: reqwestblocking::ClientBuilder,
uri: &str,
) -> Result<reqwestblocking::ClientBuilder, httpclient::Error> {
builder = builder.timeout(openid_connect_provider_timeout());
if let Ok(cert_list) = env::var(KRILL_HTTPS_ROOT_CERTS_ENV) {
for path in cert_list.split(':') {
let cert = load_root_cert(path)?;
builder = builder.add_root_certificate(cert);
}
}
if uri.starts_with("https://localhost") || uri.starts_with("https://127.0.0.1") {
builder = builder.danger_accept_invalid_certs(true);
}
Ok(builder)
}
// This is basically a copy of oauth2::reqwest::blocking::http_client() with the addition of the same logic Krill uses
// in its main HTTP client configuration (to permit insecure TLS server certificates if the server host is "localhost",
// useful when testing with a local OpenID Connect provider with a self-signed certificate, e.g. our mock provider, and
// to support custom TLS root certificates).
//
// NOTE: Why does this use a second aliased reqwest dependency as reqwestblocking?
// This is due to the reqwest 0.10.x blocking implementation actually using a futures runtime and that you can't use a
// futures runtime inside another futures runtime otherwise you get error:
// "panicked at 'Cannot drop a runtime in a context where blocking is not allowed. This happens when a runtime is
// dropped from within an asynchronous context.' reqwest blocking"
// And we can't move to using async reqwest because async Rust doesn't work with traits and AuthProvider is a trait.
// Unless we use the async_traits crate...
//
// NOTE: We don't return reqwest::Error as the oauth2-rs implementation of `fn http_client()` does because that is a
// type in the oauth2-rs crate and all of the constructors for that type are private to the crate and so we cannot use
// map_err(reqwest::Error).
fn http_client(request: openidconnect::HttpRequest) -> Result<openidconnect::HttpResponse, Error> {
let mut client_builder = reqwestblocking::Client::builder()
// Following redirects opens the client up to SSRF vulnerabilities.
.redirect(reqwestblocking::RedirectPolicy::none());
client_builder = configure_http_client_for_krill(client_builder, request.url.as_str())
.map_err(|err| Error::custom(format!("Failed to configure HTTP client: {}", err)))?;
let client = client_builder.build().map_err(Error::custom)?;
let mut request_builder = client
.request(
reqwestblocking::Method::from_bytes(request.method.as_str().as_ref())
.expect("failed to convert Method from http 0.2 to 0.1"),
request.url.as_str(),
)
.body(request.body);
for (name, value) in &request.headers {
request_builder = request_builder.header(name.as_str(), value.as_bytes());
}
let request = request_builder.build().map_err(Error::custom)?;
let mut response = client.execute(request).map_err(Error::custom)?;
let mut body = Vec::new();
{
use std::io::Read;
response.read_to_end(&mut body).map_err(Error::custom)?;
}
let headers = response
.headers()
.iter()
.map(|(name, value)| {
(
openidconnect::http::header::HeaderName::from_bytes(name.as_str().as_ref())
.expect("failed to convert HeaderName from http 0.2 to 0.1"),
openidconnect::http::header::HeaderValue::from_bytes(value.as_bytes())
.expect("failed to convert HeaderValue from http 0.2 to 0.1"),
)
})
.collect::<openidconnect::http::HeaderMap>();
Ok(openidconnect::HttpResponse {
status_code: openidconnect::http::StatusCode::from_u16(response.status().as_u16())
.expect("failed to convert StatusCode from http 0.2 to 0.1"),
headers,
body,
})
}
// Wrap the httpclient produced above with optional logging of requests to and responses from the OpenID Connect
// provider.
pub fn logging_http_client(req: openidconnect::HttpRequest) -> Result<openidconnect::HttpResponse, Error> {
pub async fn logging_http_client(req: openidconnect::HttpRequest) -> Result<openidconnect::HttpResponse, Error> {
if log_enabled!(log::Level::Trace) {
// Don't {:?} log the openidconnect::HTTPRequest req object
// because that renders the body as an unreadable integer byte
@@ -134,7 +25,7 @@ pub fn logging_http_client(req: openidconnect::HttpRequest) -> Result<openidconn
);
}
let res = http_client(req);
let res = dispatch_openid_request(req).await;
if log_enabled!(log::Level::Trace) {
match &res {
@@ -158,5 +49,80 @@ pub fn logging_http_client(req: openidconnect::HttpRequest) -> Result<openidconn
}
}
res
res.map_err(Error::HttpClientError)
}
async fn dispatch_openid_request(
request: openidconnect::HttpRequest,
) -> Result<openidconnect::HttpResponse, httpclient::Error> {
let request_uri = request.url.as_str();
let client = {
let timeout = openid_connect_provider_timeout();
let allow_redirects = false; // Following redirects opens the client up to SSRF vulnerabilities.
httpclient::client_with_tweaks(request_uri, timeout, allow_redirects)
}?;
let request = convert_openid_request(request, &client)?;
let response = client.execute(request).await?;
convert_to_openid_response(response).await
}
fn convert_openid_request(
request: openidconnect::HttpRequest,
client: &reqwest::Client,
) -> Result<reqwest::Request, httpclient::Error> {
let request_uri = request.url.as_str();
let request_method = reqwest::Method::from_str(request.method.as_str())
.map_err(|_| httpclient::Error::InvalidMethod(request.method.to_string()))?;
let mut request_builder = client.request(request_method, request_uri).body(request.body);
// map openid connect headers to the request builder
for (name, value) in &request.headers {
request_builder = request_builder.header(name.as_str(), value.as_bytes());
}
Ok(request_builder.build()?)
}
async fn convert_to_openid_response(response: Response) -> Result<openidconnect::HttpResponse, httpclient::Error> {
let response_code = response.status().as_u16();
let response_status = openidconnect::http::StatusCode::from_u16(response_code)
.map_err(|_| httpclient::Error::InvalidStatusCode(response_code))?;
let response_headers = {
let mut headers = openidconnect::http::HeaderMap::new();
for (name, value) in response.headers() {
let name = openidconnect::http::header::HeaderName::from_str(name.as_str())
.map_err(|_| httpclient::Error::InvalidHeaderName)?;
let value = openidconnect::http::header::HeaderValue::from_bytes(value.as_bytes())
.map_err(|_| httpclient::Error::InvalidHeaderValue)?;
headers.append(name, value);
}
headers
};
let response_body = response.bytes().await?;
Ok(openidconnect::HttpResponse {
status_code: response_status,
headers: response_headers,
body: response_body.to_vec(),
})
}
fn openid_connect_provider_timeout() -> Duration {
if test_mode_enabled() {
Duration::from_secs(5)
} else {
Duration::from_secs(OPENID_CONNECT_HTTP_CLIENT_TIMEOUT_SECS)
}
}
@@ -27,9 +27,11 @@ use std::{
},
ops::Deref,
path::Path,
sync::{Arc, RwLock, RwLockReadGuard},
sync::Arc,
};
use tokio::sync::{RwLock, RwLockReadGuard};
use basic_cookies::Cookie;
use hyper::header::{HeaderValue, SET_COOKIE};
use jmespatch as jmespath;
@@ -52,15 +54,15 @@ use openidconnect::{
use urlparse::{urlparse, GetQuery};
use crate::commons::util::sha256;
use crate::commons::KrillResult;
use crate::commons::{actor::ActorDef, api::Token};
use crate::commons::{util::httpclient, KrillResult};
use crate::daemon::auth::common::crypt;
use crate::daemon::auth::common::session::*;
use crate::daemon::auth::providers::config_file::config::ConfigUserDetails;
use crate::daemon::auth::providers::openid_connect::config::ConfigAuthOpenIDConnectClaims;
use crate::daemon::auth::providers::openid_connect::httpclient::logging_http_client;
use crate::daemon::auth::providers::openid_connect::jmespathext;
use crate::daemon::auth::{Auth, AuthProvider, LoggedInUser};
use crate::daemon::auth::{Auth, LoggedInUser};
use crate::daemon::config::Config;
use crate::daemon::http::auth::url_encode;
use crate::daemon::http::auth::AUTH_CALLBACK_ENDPOINT;
@@ -151,19 +153,19 @@ impl OpenIDConnectAuthProvider {
})
}
fn initialize_connection_if_needed(&self) -> KrillResult<()> {
let mut conn_guard = self.conn.write().unwrap(); // should never fail, better to panic and crash out if it does
async fn initialize_connection_if_needed(&self) -> KrillResult<()> {
let mut conn_guard = self.conn.write().await;
if conn_guard.is_none() {
*conn_guard = Some(self.initialize_connection()?);
*conn_guard = Some(self.initialize_connection().await?);
}
Ok(())
}
fn initialize_connection(&self) -> KrillResult<ProviderConnectionProperties> {
async fn initialize_connection(&self) -> KrillResult<ProviderConnectionProperties> {
trace!("OpenID Connect: Initializing provider connection...");
let meta = self.discover()?;
let meta = self.discover().await?;
let (email_scope_supported, userinfo_endpoint_supported, logout_mode) =
self.check_provider_capabilities(&meta)?;
let client = self.build_client(meta, &logout_mode)?;
@@ -182,7 +184,7 @@ impl OpenIDConnectAuthProvider {
/// discovery endpoint of the provider, e.g.
/// https://<provider.domain>/<something/.well-known/openid-configuration
/// Via which we can discover both endpoint URIs and capability flags.
fn discover(&self) -> KrillResult<WantedMeta> {
async fn discover(&self) -> KrillResult<WantedMeta> {
// Read from config the OpenID Connect identity provider discovery URL.
// Strip off /.well-known/openid-configuration because the openid-connect
// crate wants to add this itself and will fail if it is already present
@@ -198,13 +200,15 @@ impl OpenIDConnectAuthProvider {
// Contact the OpenID Connect: identity provider discovery endpoint to
// learn about and configure ourselves to talk to it.
let meta = WantedMeta::discover(&issuer, logging_http_client).map_err(|e| {
Error::custom(format!(
"OpenID Connect: Discovery failed with issuer {}, {}",
issuer.as_str(),
stringify_cause_chain(e)
))
})?;
let meta = WantedMeta::discover_async(issuer.clone(), logging_http_client)
.await
.map_err(|e| {
Error::custom(format!(
"OpenID Connect: Discovery failed with issuer {}, {}",
issuer.as_str(),
stringify_cause_chain(e)
))
})?;
Ok(meta)
}
@@ -440,7 +444,7 @@ impl OpenIDConnectAuthProvider {
))
}
fn try_revoke_token(&self, session: &ClientSession) -> Result<(), RevocationErrorResponseType> {
async fn try_revoke_token(&self, session: &ClientSession) -> Result<(), RevocationErrorResponseType> {
// Connect to the OpenID Connect provider OAuth 2.0 token revocation endpoint to terminate the
// provider session
// From: https://tools.ietf.org/html/rfc7009#section-2
@@ -460,6 +464,7 @@ impl OpenIDConnectAuthProvider {
trace!("OpenID Connect: Submitting RFC-7009 section 2 Token Revocation request");
let lock_guard = self
.get_connection()
.await
.map_err(|err| RevocationErrorResponseType::Basic(CoreErrorResponseType::Extension(err.to_string())))?;
let conn = lock_guard.deref().as_ref().unwrap(); // safe to unwrap as was tested in get_connection()
@@ -472,7 +477,8 @@ impl OpenIDConnectAuthProvider {
err.to_string()
)))
})?
.request(logging_http_client)
.request_async(logging_http_client)
.await
{
Ok(_) => Ok(()),
Err(err) => match &err {
@@ -515,7 +521,7 @@ impl OpenIDConnectAuthProvider {
/// Try refreshing the token once with the OIDC Provider and return either the new token, or the Error received from
/// the OpenID Connect Provider. This Error is FOR INTERNAL CONSUMPTION only. The caller of this function is
/// responsible for creating end-user error messages, logging and (optionally) retrying.
fn try_refresh_token(&self, session: &ClientSession) -> Result<Auth, CoreErrorResponseType> {
async fn try_refresh_token(&self, session: &ClientSession) -> Result<Auth, CoreErrorResponseType> {
let refresh_token = &session.secrets.get(TokenKind::RefreshToken.into()).ok_or_else(|| {
CoreErrorResponseType::Extension(
"Internal error: Token refresh attempted without a refresh token".to_string(),
@@ -527,13 +533,15 @@ impl OpenIDConnectAuthProvider {
let lock_guard = self
.get_connection()
.await
.map_err(|err| CoreErrorResponseType::Extension(err.to_string()))?;
let conn = lock_guard.deref().as_ref().unwrap(); // safe to unwrap as was tested in get_connection()
let token_response = conn
.client
.exchange_refresh_token(&RefreshToken::new(refresh_token.to_string()))
.request(logging_http_client);
.request_async(logging_http_client)
.await;
match token_response {
Ok(token_response) => {
@@ -810,8 +818,8 @@ impl OpenIDConnectAuthProvider {
None
}
fn get_connection(&self) -> KrillResult<RwLockReadGuard<Option<ProviderConnectionProperties>>> {
let conn_guard = self.conn.read().unwrap(); // should never fail, better to panic and crash out if it does
async fn get_connection<'a>(&'_ self) -> KrillResult<RwLockReadGuard<'_, Option<ProviderConnectionProperties>>> {
let conn_guard = self.conn.read().await;
conn_guard.as_ref().ok_or_else(|| {
OpenIDConnectAuthProvider::internal_error("Connection to provider not yet established", None)
@@ -856,14 +864,15 @@ impl OpenIDConnectAuthProvider {
}
}
fn get_token_response(&self, code: Token) -> KrillResult<FlexibleTokenResponse> {
let lock_guard = self.get_connection()?;
async fn get_token_response(&self, code: Token) -> KrillResult<FlexibleTokenResponse> {
let lock_guard = self.get_connection().await?;
let conn = lock_guard.deref().as_ref().unwrap(); // safe to unwrap as was tested in get_connection()
let token_response: FlexibleTokenResponse = conn
.client
.exchange_code(AuthorizationCode::new(code.to_string()))
.request(logging_http_client)
.request_async(logging_http_client)
.await
.map_err(|e| {
let (msg, additional_info) = match e {
RequestTokenError::ServerResponse(ref provider_err) => {
@@ -904,12 +913,13 @@ impl OpenIDConnectAuthProvider {
Ok(token_response)
}
fn get_token_id_claims<'a>(
#[allow(clippy::needless_lifetimes)] // clippy says it can be elided, but.. it seems like it's a false positive.
async fn get_token_id_claims<'a>(
&self,
token_response: &'a FlexibleTokenResponse,
nonce_hash: Nonce,
) -> KrillResult<&'a FlexibleIdTokenClaims> {
let lock_guard = self.get_connection()?;
let lock_guard = self.get_connection().await?;
let conn = lock_guard.deref().as_ref().unwrap(); // safe to unwrap as was tested in get_connection()
let mut id_token_verifier: CoreIdTokenVerifier = conn.client.id_token_verifier();
@@ -946,11 +956,11 @@ impl OpenIDConnectAuthProvider {
Ok(id_token_claims)
}
fn get_user_info_claims(
async fn get_user_info_claims(
&self,
token_response: &FlexibleTokenResponse,
) -> KrillResult<Option<FlexibleUserInfoClaims>> {
let lock_guard = self.get_connection()?;
let lock_guard = self.get_connection().await?;
let conn = lock_guard.deref().as_ref().unwrap(); // safe to unwrap as was tested in get_connection()
let user_info_claims: Option<FlexibleUserInfoClaims> = if conn.userinfo_endpoint_supported {
@@ -970,7 +980,8 @@ impl OpenIDConnectAuthProvider {
// don't require the response to be signed as the spec says
// signing it is optional: See: https://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse
.require_signed_response(false)
.request(logging_http_client)
.request_async(logging_http_client)
.await
.map_err(|e| {
let msg = match e {
UserInfoError::ClaimsVerification(ref provider_err) => {
@@ -1078,7 +1089,7 @@ impl OpenIDConnectAuthProvider {
}
}
impl AuthProvider for OpenIDConnectAuthProvider {
impl OpenIDConnectAuthProvider {
// Connect Core 1.0 section 3.1.26 Authentication Error Response
// OAuth 2.0 RFC-674 4.1.2.1 (Authorization Request Errors) & 5.2 (Access Token Request Errors)
@@ -1087,17 +1098,17 @@ impl AuthProvider for OpenIDConnectAuthProvider {
/// an error to report back to the user (one of the ApiAuth* Error types).
/// Make sure to not leak any OIDC implementation details into the Error result!
/// This function is also responsible for all logging around refreshing the token / extending the session.
fn authenticate(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<Option<ActorDef>> {
pub async fn authenticate(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<Option<ActorDef>> {
trace!("Attempting to authenticate the request..");
self.initialize_connection_if_needed().map_err(|err| {
self.initialize_connection_if_needed().await.map_err(|err| {
OpenIDConnectAuthProvider::internal_error(
"OpenID Connect: Cannot authenticate request: Failed to connect to provider",
Some(&stringify_cause_chain(err)),
)
})?;
let res = match self.get_bearer_token(request) {
let res = match httpclient::get_bearer_token(request) {
Some(token) => {
// see if we can decode, decrypt and deserialize the users token
// into a login session structure
@@ -1126,7 +1137,7 @@ impl AuthProvider for OpenIDConnectAuthProvider {
}
// Token needs refresh and we have a refresh token, try to refresh
let new_auth = match self.try_refresh_token(&session) {
let new_auth = match self.try_refresh_token(&session).await {
Ok(auth) => {
trace!(
"OpenID Connect: Successfully refreshed token for user \"{}\"",
@@ -1207,7 +1218,7 @@ impl AuthProvider for OpenIDConnectAuthProvider {
/// URL should be requested by the client on every login as the intention is
/// that it contains randomly generated CSFF token and nonce values which
/// can be used to protect against certain cross-site and replay attacks.
fn get_login_url(&self) -> KrillResult<HttpResponse> {
pub async fn get_login_url(&self) -> KrillResult<HttpResponse> {
// TODO: we probably should do some more work here to ensure we get the
// proper security benefits of the CSRF token, currently we are
// discarding the CSRF token instead of checking it.
@@ -1226,7 +1237,7 @@ impl AuthProvider for OpenIDConnectAuthProvider {
// hash of the session cookie to detect ID Token replay by third
// parties"
self.initialize_connection_if_needed().map_err(|err| {
self.initialize_connection_if_needed().await.map_err(|err| {
OpenIDConnectAuthProvider::internal_error(
"OpenID Connect: Cannot get login URL: Failed to connect to provider",
Some(&stringify_cause_chain(err)),
@@ -1240,7 +1251,7 @@ impl AuthProvider for OpenIDConnectAuthProvider {
let nonce_b64_str = random_value.secret();
let nonce_hash = sha256(nonce_b64_str.as_bytes());
let lock_guard = self.get_connection()?;
let lock_guard = self.get_connection().await?;
let conn = lock_guard.deref().as_ref().unwrap(); // safe to unwrap as was tested in get_connection()
// At the time of writing the underlying oauth2 crate CsrfToken::new_random() function is used to
@@ -1402,8 +1413,8 @@ impl AuthProvider for OpenIDConnectAuthProvider {
Ok(HttpResponse::new(res))
}
fn login(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<LoggedInUser> {
self.initialize_connection_if_needed().map_err(|err| {
pub async fn login(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<LoggedInUser> {
self.initialize_connection_if_needed().await.map_err(|err| {
OpenIDConnectAuthProvider::internal_error(
"OpenID Connect: Cannot login user: Failed to connect to provider",
Some(&stringify_cause_chain(err)),
@@ -1434,7 +1445,7 @@ impl AuthProvider for OpenIDConnectAuthProvider {
// ==========================================================================================
trace!("OpenID Connect: Submitting RFC-6749 section 4.1.3 Access Token Request");
let token_response = self.get_token_response(code)?;
let token_response = self.get_token_response(code).await?;
// TODO: extract and keep the access token and refresh token so
// that we can extend the login session later. These are
@@ -1488,7 +1499,7 @@ impl AuthProvider for OpenIDConnectAuthProvider {
// https://openid.net/specs/openid-connect-core-1_0.html#NonceNotes
let nonce_hash = Nonce::new(base64::encode_config(sha256(nonce.as_bytes()), base64::URL_SAFE_NO_PAD));
let id_token_claims = self.get_token_id_claims(&token_response, nonce_hash)?;
let id_token_claims = self.get_token_id_claims(&token_response, nonce_hash).await?;
// TODO: There's also a suggestion to verify the access token
// received above using the at_hash claim in the ID token, if
@@ -1500,7 +1511,7 @@ impl AuthProvider for OpenIDConnectAuthProvider {
// See: https://openid.net/specs/openid-connect-core-1_0.html#UserInfo
// ==========================================================================================
let user_info_claims = self.get_user_info_claims(&token_response)?;
let user_info_claims = self.get_user_info_claims(&token_response).await?;
// ==========================================================================================
// Step 4: Extract and validate the "claims" that tells us which
@@ -1602,9 +1613,9 @@ impl AuthProvider for OpenIDConnectAuthProvider {
/// logout page is not possible, instead from the end-user's perspective they are returned to the Lagosta web UI
/// index page (which currently immediately redirects the user to the 3rd party OpenID Connect provider login page)
/// but before that Krill contacts the provider on the logged-in users behalf to revoke their token at the provider.
fn logout(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<HttpResponse> {
pub async fn logout(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<HttpResponse> {
// verify the bearer token indeed represents a logged-in Krill OpenID Connect provider session
let token = self.get_bearer_token(request).ok_or_else(|| {
let token = httpclient::get_bearer_token(request).ok_or_else(|| {
warn!("Unexpectedly received a logout request without a session token.");
Error::ApiInvalidCredentials("Invalid session token".to_string())
})?;
@@ -1624,7 +1635,7 @@ impl AuthProvider for OpenIDConnectAuthProvider {
// 2. verify that the provider is at least to some extent available, there's no point trying to log the token
// out of the provider if we know there's a problem with the provider
self.initialize_connection_if_needed().map_err(|err| {
self.initialize_connection_if_needed().await.map_err(|err| {
OpenIDConnectAuthProvider::internal_error(
"OpenID Connect: Cannot logout with provider: Failed to connect to provider",
Some(&stringify_cause_chain(err)),
@@ -1632,7 +1643,7 @@ impl AuthProvider for OpenIDConnectAuthProvider {
})?;
// 3. use the provider connection details to contact the provider to terminate the client session
let lock_guard = self.get_connection()?;
let lock_guard = self.get_connection().await?;
let conn = lock_guard.deref().as_ref().unwrap(); // safe to unwrap as was tested in get_connection()
let go_to_url = match &conn.logout_mode {
@@ -1640,7 +1651,7 @@ impl AuthProvider for OpenIDConnectAuthProvider {
post_revocation_redirect_url,
..
} => {
if let Err(err) = self.try_revoke_token(&session) {
if let Err(err) = self.try_revoke_token(&session).await {
OpenIDConnectAuthProvider::internal_error(
format!("Error while revoking token for user '{}'", session.id),
Some(err.to_string()),
+4 -4
View File
@@ -341,7 +341,7 @@ pub struct Request {
impl Request {
pub async fn new(request: hyper::Request<hyper::Body>, state: State) -> Self {
let path = RequestPath::from_request(&request);
let actor = state.actor_from_request(&request);
let actor = state.actor_from_request(&request).await;
Request {
request,
@@ -505,15 +505,15 @@ impl Request {
}
pub async fn get_login_url(&self) -> KrillResult<HttpResponse> {
self.state.get_login_url()
self.state.get_login_url().await
}
pub async fn login(&self) -> KrillResult<LoggedInUser> {
self.state.login(&self.request)
self.state.login(&self.request).await
}
pub async fn logout(&self) -> KrillResult<HttpResponse> {
self.state.logout(&self.request)
self.state.logout(&self.request).await
}
}
+13 -11
View File
@@ -136,16 +136,18 @@ impl KrillServer {
// dyn AuthProvider, or concrete type needs to be known in async fn,
// etc.
let authorizer = match config.auth_type {
AuthType::AdminToken => Authorizer::new(config.clone(), AdminTokenAuthProvider::new(config.clone()))?,
AuthType::AdminToken => {
Authorizer::new(config.clone(), AdminTokenAuthProvider::new(config.clone()).into())?
}
#[cfg(feature = "multi-user")]
AuthType::ConfigFile => Authorizer::new(
config.clone(),
ConfigFileAuthProvider::new(config.clone(), login_session_cache.clone())?,
ConfigFileAuthProvider::new(config.clone(), login_session_cache.clone())?.into(),
)?,
#[cfg(feature = "multi-user")]
AuthType::OpenIDConnect => Authorizer::new(
config.clone(),
OpenIDConnectAuthProvider::new(config.clone(), login_session_cache.clone())?,
OpenIDConnectAuthProvider::new(config.clone(), login_session_cache.clone())?.into(),
)?,
};
let system_actor = authorizer.actor_from_def(ACTOR_DEF_KRILL);
@@ -268,24 +270,24 @@ impl KrillServer {
&self.system_actor
}
pub fn actor_from_request(&self, request: &hyper::Request<hyper::Body>) -> Actor {
self.authorizer.actor_from_request(request)
pub async fn actor_from_request(&self, request: &hyper::Request<hyper::Body>) -> Actor {
self.authorizer.actor_from_request(request).await
}
pub fn actor_from_def(&self, actor_def: ActorDef) -> Actor {
self.authorizer.actor_from_def(actor_def)
}
pub fn get_login_url(&self) -> KrillResult<HttpResponse> {
self.authorizer.get_login_url()
pub async fn get_login_url(&self) -> KrillResult<HttpResponse> {
self.authorizer.get_login_url().await
}
pub fn login(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<LoggedInUser> {
self.authorizer.login(request)
pub async fn login(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<LoggedInUser> {
self.authorizer.login(request).await
}
pub fn logout(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<HttpResponse> {
self.authorizer.logout(request)
pub async fn logout(&self, request: &hyper::Request<hyper::Body>) -> KrillResult<HttpResponse> {
self.authorizer.logout(request).await
}
pub fn limit_api(&self) -> u64 {
-3
View File
@@ -139,9 +139,6 @@ impl MessageQueue {
}
}
unsafe impl Send for MessageQueue {}
unsafe impl Sync for MessageQueue {}
/// Implement listening for CertAuth Published events.
impl eventsourcing::PostSaveEventListener<CertAuth> for MessageQueue {
fn listen(&self, ca: &CertAuth, events: &[CaEvt]) {