Strict redirect URL enforcement (#2917)

Add strict OAuth2 URL enforcement per the RFC. This includes a transition process for the next release so that Admins can come into compliance.
This commit is contained in:
Firstyear 2024-07-20 12:09:50 +10:00 committed by GitHub
parent f82242fd37
commit c7fcdc3e4e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
22 changed files with 626 additions and 126 deletions

View file

@ -20,8 +20,8 @@ possible harm that an attacker may make if they gain access to these roles.
Kanidm supports [privilege access mode](../accounts/authentication_and_credentials.md) so that
high-level permissions can be assigned to users who must reauthenticate before using those
privileges. The privileges then are only accessible for a short period of time. This can allow you
to assign high level permissions to regular person accounts rather than requiring separate
privilege access accounts (PAA) or privileged access workstations (PAW).
to assign high level permissions to regular person accounts rather than requiring separate privilege
access accounts (PAA) or privileged access workstations (PAW).
## Assigning Permissions to Service Accounts

View file

@ -1,8 +1,7 @@
# POSIX Accounts and Groups
Kanidm has features that enable its accounts and groups to be consumed on POSIX-like machines, such
as Linux, FreeBSD or others. Both service accounts and person accounts can be used on POSIX
systems.
as Linux, FreeBSD or others. Both service accounts and person accounts can be used on POSIX systems.
## Notes on POSIX Features

View file

@ -8,7 +8,8 @@
3. Client requests auth with a method (`AuthStep::Begin(AuthMech)`)
4. Server responds with an acknowledgement (`AuthState::Continue(Vec<AuthAllowed>)`). This is so the
challenge can be included in the response, for Passkeys or other challenge-response methods.
- If required, this challenge/response continues in a loop until the requirements are satisfied. For example, TOTP and then Password.
- If required, this challenge/response continues in a loop until the requirements are satisfied.
For example, TOTP and then Password.
5. The result is returned, either:
- Success, with the User Auth Token as a `String`.
- Denied, with a reason as a `String`.

View file

@ -241,8 +241,8 @@ Imagine the following example. Here, Node A is acting as the KRC.
This would allow Node A to be aware of B, C, D and then create a full mesh.
We wish to decommission Node A and promote Node B to become the new KRC. Imagine at this point we cut
over Node D to point its KRC at Node B.
We wish to decommission Node A and promote Node B to become the new KRC. Imagine at this point we
cut over Node D to point its KRC at Node B.
```
┌─────────────────┐ ┌─────────────────┐

View file

@ -54,10 +54,11 @@ has an excellent explanation of the attack. Additionally, this threat is discuss
[RFC6819 Section 4.4.1](https://www.rfc-editor.org/rfc/rfc6819#section-4.4.1).
As Kanidm aims for "secure by default" design, even with _confidential_ clients, we deem it
important to raise the bar for attackers. For example, an attacker may have access to the `client_id`
and `client_secret` of a confidential client as it was mishandled by a system administrator. While
they may not have direct access to the client/application systems, they could still use this
`client_id+secret` to then carry out the authorisation code interception attack listed.
important to raise the bar for attackers. For example, an attacker may have access to the
`client_id` and `client_secret` of a confidential client as it was mishandled by a system
administrator. While they may not have direct access to the client/application systems, they could
still use this `client_id+secret` to then carry out the authorisation code interception attack
listed.
For confidential clients (refered to as a `basic` client in Kanidm due to the use of HTTP Basic for
`client_id+secret` presentation) PKCE may optionally be disabled. This can allow authorisation code

View file

@ -31,8 +31,8 @@ key-values on objects which are all UTF8 strings (or subsets thereof) based on v
rules. Kanidm internally implements complex structured data types such as tagging on SSH keys, or
multi-value credentials. These can not be represented in LDAP.
Many of the structures in Kanidm do not correlate closely to LDAP. For example, Kanidm only has a GID
number, where LDAP's schemas define both a UID number and a GID number.
Many of the structures in Kanidm do not correlate closely to LDAP. For example, Kanidm only has a
GID number, where LDAP's schemas define both a UID number and a GID number.
Entries in the database also have a specific name in LDAP, related to their path in the directory
tree. Kanidm is a flat model, so we have to emulate some tree-like elements, and ignore others.

View file

@ -130,13 +130,22 @@ After you have understood your service requirements you first need to configure
members of `system_admins` or `idm_hp_oauth2_manage_priv` are able to create or manage OAuth2 client
integrations.
You can create a new client with:
You can create a new client by specifying its client name, application display name and the landing
page (home page) of the client. The landing page is where users will be redirected to in order to
begin an OAuth2 flow from the application portal.
```bash
kanidm system oauth2 create <name> <displayname> <origin>
kanidm system oauth2 create <name> <displayname> <landing page url>
kanidm system oauth2 create nextcloud "Nextcloud Production" https://nextcloud.example.com
```
You must now configure the redirect URL where the application expects OAuth2 requests to be sent.
```bash
kanidm system oauth2 add-redirect-url <name> <redirect url>
kanidm system oauth2 add-redirect-url nextcloud https://nextcloud.example.com/oauth2/handler
```
You can create a scope map with:
```bash
@ -286,25 +295,69 @@ kanidm system oauth2 disable-localhost-redirects <name>
kanidm system oauth2 enable-localhost-redirects mywebapp
```
## Alternate Redirect Origins
## Alternate Redirect URLs
Some services may have a website URL as well as native applications. These native applications
require alternate redirection URLs to be configured so that after an OAuth2 exchange, the system can
redirect to the native application.
{{#template ../templates/kani-warning.md imagepath=../images title=WARNING text=SECURITY RISK }}
To support this Kanidm allows supplemental origins to be configured on clients.
> You **MUST NOT** share a single OAuth2 client definition between multiple applications.
>
> The ability to configure multiple redirect URLs is **NOT** to allow you to share a single Kanidm
> client definition between multiple OAuth2 clients.
>
> Sharing OAuth2 client configurations between applications **FUNDAMENTALLY BREAKS** the OAuth2
> security model and is **NOT SUPPORTED** as a configuration. The Kanidm Project **WILL NOT**
> support you if you attempt this.
>
> Multiple origins are **ONLY** to allow supplemental redirects within the _same_ client
> application.
{{#template ../templates/kani-warning.md imagepath=../images title=WARNING text=The ability to configure multiple origins is NOT intended to allow you to share a single Kanidm client definition between multiple OAuth2 clients. This fundamentally breaks the OAuth2 security model and is NOT SUPPORTED as a configuration. Multiple origins is only to allow supplemental redirects within the _same_ client application. }}
Some services may have a website URL as well as native applications with opaque origins. These
native applications require alternate redirection URLs to be configured so that after an OAuth2
exchange, the system can redirect to the native application.
To support this Kanidm allows supplemental opaque origins to be configured on clients.
```bash
kanidm system oauth2 add-origin <name> <origin>
kanidm system oauth2 remove-origin <name> <origin>
kanidm system oauth2 add-redirect-url <name> <url>
kanidm system oauth2 remove-redirect-url <name> <url>
kanidm system oauth2 add-origin nextcloud app://ios-nextcloud
kanidm system oauth2 add-redirect-url nextcloud app://ios-nextcloud
```
Supplemental URLs are shown in the OAuth2 client configuration in the `oauth2_rs_origin` attribute.
### Strict Redirect URLs
Kanidm previously enforce that redirection targets only matched by _origin_, not the full URL. In
1.4.0 these URLs will enforce a full URL match instead.
To indicate your readiness for this transition, all OAuth2 clients must have the field
`strict-redirect-url` enabled. Once enabled, the client will begin to enforce the 1.4.0 strict
validation behaviour.
If you have not enabled `strict-redirect-url` on all OAuth2 clients the upgrade to 1.4.0 will refuse
to proceed.
To enable or disable strict validation:
```bash
kanidm system oauth2 enable-strict-redirect-url <name>
kanidm system oauth2 disable-strict-redirect-url <name>
```
### Why Does Sharing a Client Weaken OAuth2?
By sharing a client ID between multiple applications this implies that all of these applications are
a singular client in Kanidm's view. This means tokens issued to one application can be reused on any
other application.
This limits your ability to enforce scopes, presents a large compromise blast radius if a token is
stolen, and also increases the damage radius if the client credentials are stolen.
Generally this also provides a bad user experience since the Kanidm application portal only lists a
single landing URL of the client, so subsequent applications that "piggy back" can not be redirected
to from the application portal meaning that users will not easily be able to access the application.
## Extended Options for Legacy Clients
Not all clients support modern standards like PKCE or ECDSA. In these situations it may be necessary

View file

@ -4,7 +4,7 @@ use kanidm_proto::constants::{
ATTR_OAUTH2_ALLOW_INSECURE_CLIENT_DISABLE_PKCE, ATTR_OAUTH2_ALLOW_LOCALHOST_REDIRECT,
ATTR_OAUTH2_JWT_LEGACY_CRYPTO_ENABLE, ATTR_OAUTH2_PREFER_SHORT_USERNAME,
ATTR_OAUTH2_RS_BASIC_SECRET, ATTR_OAUTH2_RS_ORIGIN, ATTR_OAUTH2_RS_ORIGIN_LANDING,
ATTR_OAUTH2_RS_TOKEN_KEY, ATTR_RS256_PRIVATE_KEY_DER,
ATTR_OAUTH2_RS_TOKEN_KEY, ATTR_OAUTH2_STRICT_REDIRECT_URI, ATTR_RS256_PRIVATE_KEY_DER,
};
use kanidm_proto::internal::{ImageValue, Oauth2ClaimMapJoin};
use kanidm_proto::v1::Entry;
@ -350,6 +350,36 @@ impl KanidmClient {
.await
}
pub async fn idm_oauth2_rs_enable_strict_redirect_uri(
&self,
id: &str,
) -> Result<(), ClientError> {
let mut update_oauth2_rs = Entry {
attrs: BTreeMap::new(),
};
update_oauth2_rs.attrs.insert(
ATTR_OAUTH2_STRICT_REDIRECT_URI.to_string(),
vec!["true".to_string()],
);
self.perform_patch_request(format!("/v1/oauth2/{}", id).as_str(), update_oauth2_rs)
.await
}
pub async fn idm_oauth2_rs_disable_strict_redirect_uri(
&self,
id: &str,
) -> Result<(), ClientError> {
let mut update_oauth2_rs = Entry {
attrs: BTreeMap::new(),
};
update_oauth2_rs.attrs.insert(
ATTR_OAUTH2_STRICT_REDIRECT_URI.to_string(),
vec!["false".to_string()],
);
self.perform_patch_request(format!("/v1/oauth2/{}", id).as_str(), update_oauth2_rs)
.await
}
pub async fn idm_oauth2_rs_update_claim_map(
&self,
id: &str,

View file

@ -158,6 +158,7 @@ pub const ATTR_OAUTH2_RS_SCOPE_MAP: &str = "oauth2_rs_scope_map";
pub const ATTR_OAUTH2_RS_SUP_SCOPE_MAP: &str = "oauth2_rs_sup_scope_map";
pub const ATTR_OAUTH2_RS_TOKEN_KEY: &str = "oauth2_rs_token_key";
pub const ATTR_OAUTH2_SESSION: &str = "oauth2_session";
pub const ATTR_OAUTH2_STRICT_REDIRECT_URI: &str = "oauth2_strict_redirect_uri";
pub const ATTR_OBJECTCLASS: &str = "objectclass";
pub const ATTR_OTHER_NO_INDEX: &str = "other-no-index";
pub const ATTR_PASSKEYS: &str = "passkeys";

View file

@ -152,6 +152,7 @@ pub enum OperationError {
MG0004DomainLevelInDevelopment,
MG0005GidConstraintsNotMet,
MG0006SKConstraintsNotMet,
MG0007Oauth2StrictConstraintsNotMet,
//
KP0001KeyProviderNotLoaded,
KP0002KeyProviderInvalidClass,
@ -304,6 +305,7 @@ impl OperationError {
Self::MG0004DomainLevelInDevelopment => None,
Self::MG0005GidConstraintsNotMet => None,
Self::MG0006SKConstraintsNotMet => Some("Migration Constraints Not Met - Security Keys should not be present."),
Self::MG0007Oauth2StrictConstraintsNotMet => Some("Migration Constraints Not Met - All OAuth2 clients must have strict-redirect-uri mode enabled."),
Self::KP0001KeyProviderNotLoaded => None,
Self::KP0002KeyProviderInvalidClass => None,
Self::KP0003KeyProviderInvalidType => None,

View file

@ -245,6 +245,9 @@ pub enum DomainUpgradeCheckStatus {
Pass7To8SecurityKeys,
Fail7To8SecurityKeys,
Pass7To8Oauth2StrictRedirectUri,
Fail7To8Oauth2StrictRedirectUri,
}
#[derive(Debug, Serialize, Deserialize, Clone)]

View file

@ -180,7 +180,8 @@ async fn submit_admin_req(path: &str, req: AdminTaskRequest, output_mode: Consol
}
},
Some(Ok(AdminTaskResponse::DomainUpgradeCheck { report })) => match output_mode {
Some(Ok(AdminTaskResponse::DomainUpgradeCheck { report })) => {
match output_mode {
ConsoleOutputMode::JSON => {
let json_output = serde_json::json!({
"domain_upgrade_check": report
@ -221,6 +222,7 @@ async fn submit_admin_req(path: &str, req: AdminTaskRequest, output_mode: Consol
info!("affected_entry : {}", entry_id);
}
}
// ===========
ProtoDomainUpgradeCheckStatus::Pass7To8SecurityKeys => {
info!("upgrade_item : security key usage");
debug!("from_level : {}", item.from_level);
@ -238,10 +240,29 @@ async fn submit_admin_req(path: &str, req: AdminTaskRequest, output_mode: Consol
info!("affected_entry : {}", entry_id);
}
}
// ===========
ProtoDomainUpgradeCheckStatus::Pass7To8Oauth2StrictRedirectUri => {
info!("upgrade_item : oauth2 strict redirect uri enforcement");
debug!("from_level : {}", item.from_level);
debug!("to_level : {}", item.to_level);
info!("status : PASS");
}
ProtoDomainUpgradeCheckStatus::Fail7To8Oauth2StrictRedirectUri => {
info!("upgrade_item : oauth2 strict redirect uri enforcement");
debug!("from_level : {}", item.from_level);
debug!("to_level : {}", item.to_level);
info!("status : FAIL");
info!("description : To harden against possible public client open redirection vulnerabilities, redirect uris must now be registered ahead of time and are validated rather than the former origin verification process.");
info!("action : Verify the redirect uri's for OAuth2 clients and then enable strict-redirect-uri on each client.");
for entry_id in item.affected_entries {
info!("affected_entry : {}", entry_id);
}
}
}
}
}
}
}
},
Some(Ok(AdminTaskResponse::DomainRaise { level })) => match output_mode {
ConsoleOutputMode::JSON => {

View file

@ -881,6 +881,112 @@ lazy_static! {
};
}
lazy_static! {
pub static ref IDM_ACP_OAUTH2_MANAGE_DL7: BuiltinAcp = BuiltinAcp {
classes: vec![
EntryClass::Object,
EntryClass::AccessControlProfile,
EntryClass::AccessControlCreate,
EntryClass::AccessControlDelete,
EntryClass::AccessControlModify,
EntryClass::AccessControlSearch
],
name: "idm_acp_hp_oauth2_manage_priv",
uuid: UUID_IDM_ACP_OAUTH2_MANAGE_V1,
description: "Builtin IDM Control for managing oauth2 resource server integrations.",
receiver: BuiltinAcpReceiver::Group(vec![UUID_IDM_OAUTH2_ADMINS]),
target: BuiltinAcpTarget::Filter(ProtoFilter::And(vec![
match_class_filter!(EntryClass::OAuth2ResourceServer),
FILTER_ANDNOT_TOMBSTONE_OR_RECYCLED.clone(),
])),
search_attrs: vec![
Attribute::Class,
Attribute::Description,
Attribute::DisplayName,
Attribute::Name,
Attribute::Spn,
Attribute::OAuth2Session,
Attribute::OAuth2RsOrigin,
Attribute::OAuth2RsOriginLanding,
Attribute::OAuth2RsScopeMap,
Attribute::OAuth2RsSupScopeMap,
Attribute::OAuth2RsBasicSecret,
Attribute::OAuth2RsTokenKey,
Attribute::Es256PrivateKeyDer,
Attribute::OAuth2AllowInsecureClientDisablePkce,
Attribute::Rs256PrivateKeyDer,
Attribute::OAuth2JwtLegacyCryptoEnable,
Attribute::OAuth2PreferShortUsername,
Attribute::OAuth2AllowLocalhostRedirect,
Attribute::OAuth2RsClaimMap,
Attribute::Image,
Attribute::OAuth2StrictRedirectUri,
],
modify_removed_attrs: vec![
Attribute::Description,
Attribute::DisplayName,
Attribute::Name,
Attribute::OAuth2Session,
Attribute::OAuth2RsOrigin,
Attribute::OAuth2RsOriginLanding,
Attribute::OAuth2RsScopeMap,
Attribute::OAuth2RsSupScopeMap,
Attribute::OAuth2RsBasicSecret,
Attribute::OAuth2RsTokenKey,
Attribute::Es256PrivateKeyDer,
Attribute::OAuth2AllowInsecureClientDisablePkce,
Attribute::Rs256PrivateKeyDer,
Attribute::OAuth2JwtLegacyCryptoEnable,
Attribute::OAuth2PreferShortUsername,
Attribute::OAuth2AllowLocalhostRedirect,
Attribute::OAuth2RsClaimMap,
Attribute::Image,
Attribute::OAuth2StrictRedirectUri,
],
modify_present_attrs: vec![
Attribute::Description,
Attribute::DisplayName,
Attribute::Name,
Attribute::OAuth2RsOrigin,
Attribute::OAuth2RsOriginLanding,
Attribute::OAuth2RsSupScopeMap,
Attribute::OAuth2RsScopeMap,
Attribute::OAuth2AllowInsecureClientDisablePkce,
Attribute::OAuth2JwtLegacyCryptoEnable,
Attribute::OAuth2PreferShortUsername,
Attribute::OAuth2AllowLocalhostRedirect,
Attribute::OAuth2RsClaimMap,
Attribute::Image,
Attribute::OAuth2StrictRedirectUri,
],
create_attrs: vec![
Attribute::Class,
Attribute::Description,
Attribute::Name,
Attribute::OAuth2RsName,
Attribute::OAuth2RsOrigin,
Attribute::OAuth2RsOriginLanding,
Attribute::OAuth2RsSupScopeMap,
Attribute::OAuth2RsScopeMap,
Attribute::OAuth2AllowInsecureClientDisablePkce,
Attribute::OAuth2JwtLegacyCryptoEnable,
Attribute::OAuth2PreferShortUsername,
Attribute::OAuth2AllowLocalhostRedirect,
Attribute::OAuth2RsClaimMap,
Attribute::Image,
Attribute::OAuth2StrictRedirectUri,
],
create_classes: vec![
EntryClass::Object,
EntryClass::Account,
EntryClass::OAuth2ResourceServer,
EntryClass::OAuth2ResourceServerBasic,
EntryClass::OAuth2ResourceServerPublic,
],
..Default::default()
};
}
lazy_static! {
pub static ref IDM_ACP_DOMAIN_ADMIN_V1: BuiltinAcp = BuiltinAcp {
classes: vec![

View file

@ -143,6 +143,7 @@ pub enum Attribute {
OAuth2RsSupScopeMap,
OAuth2RsTokenKey,
OAuth2Session,
OAuth2StrictRedirectUri,
ObjectClass,
OtherNoIndex,
PassKeys,
@ -335,6 +336,7 @@ impl<'a> TryFrom<&'a str> for Attribute {
ATTR_OAUTH2_RS_SUP_SCOPE_MAP => Attribute::OAuth2RsSupScopeMap,
ATTR_OAUTH2_RS_TOKEN_KEY => Attribute::OAuth2RsTokenKey,
ATTR_OAUTH2_SESSION => Attribute::OAuth2Session,
ATTR_OAUTH2_STRICT_REDIRECT_URI => Attribute::OAuth2StrictRedirectUri,
ATTR_OBJECTCLASS => Attribute::ObjectClass,
ATTR_OTHER_NO_INDEX => Attribute::OtherNoIndex,
ATTR_PASSKEYS => Attribute::PassKeys,
@ -510,6 +512,7 @@ impl From<Attribute> for &'static str {
Attribute::OAuth2RsSupScopeMap => ATTR_OAUTH2_RS_SUP_SCOPE_MAP,
Attribute::OAuth2RsTokenKey => ATTR_OAUTH2_RS_TOKEN_KEY,
Attribute::OAuth2Session => ATTR_OAUTH2_SESSION,
Attribute::OAuth2StrictRedirectUri => ATTR_OAUTH2_STRICT_REDIRECT_URI,
Attribute::ObjectClass => ATTR_OBJECTCLASS,
Attribute::OtherNoIndex => ATTR_OTHER_NO_INDEX,
Attribute::PassKeys => ATTR_PASSKEYS,

View file

@ -84,13 +84,15 @@ pub const DOMAIN_MIN_REMIGRATION_LEVEL: DomainVersion = DOMAIN_LEVEL_5;
pub const DOMAIN_MIN_LEVEL: DomainVersion = DOMAIN_TGT_LEVEL;
// The previous releases domain functional level
pub const DOMAIN_PREVIOUS_TGT_LEVEL: DomainVersion = DOMAIN_LEVEL_6;
// The target supported domain functional level
// The target supported domain functional level. During development this is
// the NEXT level that users will upgrade too.
pub const DOMAIN_TGT_LEVEL: DomainVersion = DOMAIN_LEVEL_7;
// The current patch level if any out of band fixes are required.
pub const DOMAIN_TGT_PATCH_LEVEL: u32 = PATCH_LEVEL_1;
// The target domain functional level for the SUBSEQUENT release/dev cycle.
pub const DOMAIN_TGT_NEXT_LEVEL: DomainVersion = DOMAIN_LEVEL_8;
// The maximum supported domain functional level
pub const DOMAIN_MAX_LEVEL: DomainVersion = DOMAIN_LEVEL_7;
// The next maximum supported domain functional level
pub const DOMAIN_NEXT_LEVEL: DomainVersion = DOMAIN_LEVEL_8;
pub const DOMAIN_MAX_LEVEL: DomainVersion = DOMAIN_LEVEL_8;
// On test builds define to 60 seconds
#[cfg(test)]

View file

@ -424,6 +424,15 @@ pub static ref SCHEMA_ATTR_OAUTH2_CONSENT_SCOPE_MAP: SchemaAttribute = SchemaAtt
..Default::default()
};
pub static ref SCHEMA_ATTR_OAUTH2_STRICT_REDIRECT_URI_DL7: SchemaAttribute = SchemaAttribute {
uuid: UUID_SCHEMA_ATTR_OAUTH2_STRICT_REDIRECT_URI,
name: Attribute::OAuth2StrictRedirectUri.into(),
description: "Represents if strict redirect uri enforcement is enabled.".to_string(),
syntax: SyntaxType::Boolean,
..Default::default()
};
pub static ref SCHEMA_ATTR_ES256_PRIVATE_KEY_DER: SchemaAttribute = SchemaAttribute {
uuid: UUID_SCHEMA_ATTR_ES256_PRIVATE_KEY_DER,
name: Attribute::Es256PrivateKeyDer.into(),
@ -1261,6 +1270,7 @@ pub static ref SCHEMA_CLASS_OAUTH2_RS_DL7: SchemaClass = SchemaClass {
Attribute::OAuth2RsClaimMap.into(),
Attribute::OAuth2Session.into(),
Attribute::OAuth2RsOrigin.into(),
Attribute::OAuth2StrictRedirectUri.into(),
],
systemmust: vec![
Attribute::OAuth2RsOriginLanding.into(),

View file

@ -309,6 +309,8 @@ pub const UUID_SCHEMA_ATTR_REFERS: Uuid = uuid!("00000000-0000-0000-0000-ffff000
pub const UUID_SCHEMA_ATTR_CERTIFICATE: Uuid = uuid!("00000000-0000-0000-0000-ffff00000178");
pub const UUID_SCHEMA_CLASS_CLIENT_CERTIFICATE: Uuid =
uuid!("00000000-0000-0000-0000-ffff00000179");
pub const UUID_SCHEMA_ATTR_OAUTH2_STRICT_REDIRECT_URI: Uuid =
uuid!("00000000-0000-0000-0000-ffff00000180");
// System and domain infos
// I'd like to strongly criticise william of the past for making poor choices about these allocations.

View file

@ -273,7 +273,9 @@ pub struct Oauth2RS {
origins: HashSet<Origin>,
opaque_origins: HashSet<Url>,
redirect_uris: HashSet<Url>,
origin_https_required: bool,
strict_redirect_uri: bool,
claim_map: BTreeMap<Uuid, Vec<(String, ClaimValue)>>,
scope_maps: BTreeMap<Uuid, BTreeSet<String>>,
@ -335,11 +337,13 @@ pub struct Oauth2ResourceServersWriteTransaction<'a> {
inner: CowCellWriteTxn<'a, Oauth2RSInner>,
}
impl TryFrom<(Vec<Arc<EntrySealedCommitted>>, Url)> for Oauth2ResourceServers {
impl TryFrom<(Vec<Arc<EntrySealedCommitted>>, Url, DomainVersion)> for Oauth2ResourceServers {
type Error = OperationError;
fn try_from(value: (Vec<Arc<EntrySealedCommitted>>, Url)) -> Result<Self, Self::Error> {
let (value, origin) = value;
fn try_from(
value: (Vec<Arc<EntrySealedCommitted>>, Url, DomainVersion),
) -> Result<Self, Self::Error> {
let (value, origin, domain_level) = value;
let fernet =
Fernet::new(&Fernet::generate_key()).ok_or(OperationError::CryptographyError)?;
let oauth2rs = Oauth2ResourceServers {
@ -351,7 +355,7 @@ impl TryFrom<(Vec<Arc<EntrySealedCommitted>>, Url)> for Oauth2ResourceServers {
};
let mut oauth2rs_wr = oauth2rs.write();
oauth2rs_wr.reload(value)?;
oauth2rs_wr.reload(value, domain_level)?;
oauth2rs_wr.commit();
Ok(oauth2rs)
}
@ -372,7 +376,11 @@ impl Oauth2ResourceServers {
}
impl<'a> Oauth2ResourceServersWriteTransaction<'a> {
pub fn reload(&mut self, value: Vec<Arc<EntrySealedCommitted>>) -> Result<(), OperationError> {
pub fn reload(
&mut self,
value: Vec<Arc<EntrySealedCommitted>>,
domain_level: DomainVersion,
) -> Result<(), OperationError> {
let rs_set: Result<HashMap<_, _>, _> = value
.into_iter()
.map(|ent| {
@ -432,18 +440,24 @@ impl<'a> Oauth2ResourceServersWriteTransaction<'a> {
.cloned()
.ok_or(OperationError::InvalidValueState)?;
let maybe_extra_origins = ent.get_ava_set(Attribute::OAuth2RsOrigin).and_then(|s| s.as_url_set());
let maybe_extra_urls = ent.get_ava_set(Attribute::OAuth2RsOrigin).and_then(|s| s.as_url_set());
let len_uris = maybe_extra_origins.map(|s| s.len() + 1).unwrap_or(1);
let len_uris = maybe_extra_urls.map(|s| s.len() + 1).unwrap_or(1);
// If we are DL8, then strict enforcement is always required.
let strict_redirect_uri = cfg!(test) ||
domain_level >= DOMAIN_LEVEL_8 ||
ent.get_ava_single_bool(Attribute::OAuth2StrictRedirectUri)
.unwrap_or(false);
// The reason we have to allocate this is that we need to do some processing on these
// urls to determine if they are opaque or not.
let mut redirect_uris = Vec::with_capacity(len_uris);
let mut redirect_uris_v = Vec::with_capacity(len_uris);
redirect_uris.push(landing_url);
if let Some(extra_origins) = maybe_extra_origins {
redirect_uris_v.push(landing_url);
if let Some(extra_origins) = maybe_extra_urls {
for x_origin in extra_origins {
redirect_uris.push(x_origin.clone());
redirect_uris_v.push(x_origin.clone());
}
}
@ -451,16 +465,22 @@ impl<'a> Oauth2ResourceServersWriteTransaction<'a> {
// that may or may not be an opaque origin. We need to split these up now.
let mut origins = HashSet::with_capacity(len_uris);
let mut redirect_uris = HashSet::with_capacity(len_uris);
let mut opaque_origins = HashSet::with_capacity(len_uris);
let mut origin_https_required = false;
for uri in redirect_uris.into_iter() {
for mut uri in redirect_uris_v.into_iter() {
// https://www.rfc-editor.org/rfc/rfc6749#section-3.1.2
// Must not include a fragment.
uri.set_fragment(None);
// Given the presence of a single https url, then all other urls must be https.
if uri.scheme() == "https" {
origin_https_required = true;
origins.insert(uri.origin());
redirect_uris.insert(uri);
} else if uri.scheme() == "http" {
origins.insert(uri.origin());
redirect_uris.insert(uri);
} else {
opaque_origins.insert(uri);
}
@ -640,7 +660,9 @@ impl<'a> Oauth2ResourceServersWriteTransaction<'a> {
uuid,
origins,
opaque_origins,
redirect_uris,
origin_https_required,
strict_redirect_uri,
scope_maps,
sup_scope_maps,
client_scopes,
@ -1657,16 +1679,35 @@ impl<'a> IdmServerProxyReadTransaction<'a> {
.map(|domain| domain == "localhost")
.unwrap_or_default();
// Strict uri validation is in use.
let strict_redirect_uri_matched =
o2rs.strict_redirect_uri && o2rs.redirect_uris.contains(&auth_req.redirect_uri);
// The legacy origin match is in use.
let origin_uri_matched =
!o2rs.strict_redirect_uri && o2rs.origins.contains(&auth_req.redirect_uri.origin());
// Allow opaque origins such as app uris.
let opaque_origin_matched = o2rs.opaque_origins.contains(&auth_req.redirect_uri);
// redirect_uri must be part of the client_id origin, unless the client is public and then it MAY
// be localhost exempting it from this check and enforcement.
if !(o2rs.origins.contains(&auth_req.redirect_uri.origin())
|| o2rs.opaque_origins.contains(&auth_req.redirect_uri)
|| (allow_localhost_redirect && localhost_redirect))
let localhost_redirect_matched = allow_localhost_redirect && localhost_redirect;
// At least one of these conditions must hold true to proceed.
if !(strict_redirect_uri_matched
|| origin_uri_matched
|| opaque_origin_matched
|| localhost_redirect_matched)
{
if o2rs.strict_redirect_uri {
warn!(
"Invalid OAuth2 redirect_uri (must be an exact match to a redirect-url) - got {}",
auth_req.redirect_uri.as_str()
);
} else {
warn!(
"Invalid OAuth2 redirect_uri (must be related to origin) - got {:?}",
auth_req.redirect_uri.origin()
);
}
return Err(Oauth2Error::InvalidOrigin);
}
@ -2759,6 +2800,10 @@ mod tests {
Value::new_url_s("https://demo.example.com").unwrap()
),
// Supplemental origins
(
Attribute::OAuth2RsOrigin,
Value::new_url_s("https://demo.example.com/oauth2/result").unwrap()
),
(
Attribute::OAuth2RsOrigin,
Value::new_url_s("https://portal.example.com").unwrap()
@ -2920,6 +2965,10 @@ mod tests {
Attribute::OAuth2RsOriginLanding,
Value::new_url_s("https://demo.example.com").unwrap()
),
(
Attribute::OAuth2RsOrigin,
Value::new_url_s("https://demo.example.com/oauth2/result").unwrap()
),
// System admins
(
Attribute::OAuth2RsScopeMap,
@ -3267,6 +3316,26 @@ mod tests {
== Oauth2Error::InvalidOrigin
);
// * invalid uri in the redirect
let auth_req = AuthorisationRequest {
response_type: "code".to_string(),
client_id: "test_resource_server".to_string(),
state: "123".to_string(),
pkce_request: pkce_request.clone(),
redirect_uri: Url::parse("https://demo.example.com/oauth2/wrong_place").unwrap(),
scope: OAUTH2_SCOPE_OPENID.to_string(),
nonce: None,
oidc_ext: Default::default(),
unknown_keys: Default::default(),
};
assert!(
idms_prox_read
.check_oauth2_authorisation(&ident, &auth_req, ct)
.unwrap_err()
== Oauth2Error::InvalidOrigin
);
// Requested scope is not available
let auth_req = AuthorisationRequest {
response_type: "code".to_string(),
@ -3612,7 +3681,7 @@ mod tests {
code_challenge: code_challenge.clone().into(),
code_challenge_method: CodeChallengeMethod::S256,
}),
redirect_uri: Url::parse("https://portal.example.com/oauth2/result").unwrap(),
redirect_uri: Url::parse("https://portal.example.com").unwrap(),
scope: OAUTH2_SCOPE_OPENID.to_string(),
nonce: Some("abcdef".to_string()),
oidc_ext: Default::default(),
@ -3649,7 +3718,7 @@ mod tests {
let token_req = AccessTokenRequest {
grant_type: GrantTypeReq::AuthorizationCode {
code: permit_success.code,
redirect_uri: Url::parse("https://portal.example.com/oauth2/result").unwrap(),
redirect_uri: Url::parse("https://portal.example.com").unwrap(),
// From the first step.
code_verifier: code_verifier.clone(),
},

View file

@ -145,11 +145,12 @@ impl IdmServer {
let (audit_tx, audit_rx) = unbounded();
// Get the domain name, as the relying party id.
let (rp_id, rp_name, oauth2rs_set) = {
let (rp_id, rp_name, domain_level, oauth2rs_set) = {
let mut qs_read = qs.read().await;
(
qs_read.get_domain_name().to_string(),
qs_read.get_domain_display_name().to_string(),
qs_read.get_domain_version(),
// Add a read/reload of all oauth2 configurations.
qs_read.get_oauth2rs_set()?,
)
@ -186,8 +187,8 @@ impl IdmServer {
OperationError::InvalidState
})?;
let oauth2rs =
Oauth2ResourceServers::try_from((oauth2rs_set, origin_url)).map_err(|e| {
let oauth2rs = Oauth2ResourceServers::try_from((oauth2rs_set, origin_url, domain_level))
.map_err(|e| {
admin_error!("Failed to load oauth2 resource servers - {:?}", e);
e
})?;
@ -2052,9 +2053,10 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
#[instrument(level = "debug", skip_all)]
pub fn commit(mut self) -> Result<(), OperationError> {
if self.qs_write.get_changed_oauth2() {
let domain_level = self.qs_write.get_domain_version();
self.qs_write
.get_oauth2rs_set()
.and_then(|oauth2rs_set| self.oauth2rs.reload(oauth2rs_set))?;
.and_then(|oauth2rs_set| self.oauth2rs.reload(oauth2rs_set, domain_level))?;
// Clear the flag to indicate we completed the reload.
self.qs_write.clear_changed_oauth2();
}

View file

@ -717,6 +717,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
SCHEMA_ATTR_REFERS_DL7.clone().into(),
SCHEMA_ATTR_CERTIFICATE_DL7.clone().into(),
SCHEMA_ATTR_OAUTH2_RS_ORIGIN_DL7.clone().into(),
SCHEMA_ATTR_OAUTH2_STRICT_REDIRECT_URI_DL7.clone().into(),
SCHEMA_CLASS_DOMAIN_INFO_DL7.clone().into(),
SCHEMA_CLASS_SERVICE_ACCOUNT_DL7.clone().into(),
SCHEMA_CLASS_SYNC_ACCOUNT_DL7.clone().into(),
@ -760,6 +761,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
IDM_ACP_SELF_WRITE_DL7.clone().into(),
IDM_ACP_SELF_NAME_WRITE_DL7.clone().into(),
IDM_ACP_HP_CLIENT_CERTIFICATE_MANAGER_DL7.clone().into(),
IDM_ACP_OAUTH2_MANAGE_DL7.clone().into(),
];
idm_data
@ -828,6 +830,27 @@ impl<'a> QueryServerWriteTransaction<'a> {
return Err(OperationError::MG0006SKConstraintsNotMet);
}
// Check oauth2 strict uri
let filter = filter!(f_and!([
f_eq(Attribute::Class, EntryClass::OAuth2ResourceServer.into()),
f_andnot(f_pres(Attribute::OAuth2StrictRedirectUri)),
]));
let results = self.internal_search(filter)?;
let affected_entries = results
.into_iter()
.map(|entry| entry.get_display_id())
.collect::<Vec<_>>();
if !affected_entries.is_empty() {
error!("Unable to proceed. Not all oauth2 clients have strict redirect verification enabled.");
for missing_oauth2_strict_redirect_uri in affected_entries {
error!(%missing_oauth2_strict_redirect_uri);
}
return Err(OperationError::MG0007Oauth2StrictConstraintsNotMet);
}
// =========== Apply changes ==============
Ok(())
@ -1111,7 +1134,7 @@ impl<'a> QueryServerReadTransaction<'a> {
let name = d_info.d_name.clone();
let uuid = d_info.d_uuid;
let current_level = d_info.d_vers;
let upgrade_level = DOMAIN_NEXT_LEVEL;
let upgrade_level = DOMAIN_TGT_NEXT_LEVEL;
let mut report_items = Vec::with_capacity(1);
@ -1139,6 +1162,17 @@ impl<'a> QueryServerReadTransaction<'a> {
err
})?;
report_items.push(item);
let item = self
.domain_upgrade_check_7_to_8_oauth2_strict_redirect_uri()
.map_err(|err| {
error!(
?err,
"Failed to perform domain upgrade check 7 to 8 - oauth2-strict-redirect_uri"
);
err
})?;
report_items.push(item);
}
Ok(ProtoDomainUpgradeCheckReport {
@ -1276,10 +1310,40 @@ impl<'a> QueryServerReadTransaction<'a> {
affected_entries,
})
}
pub(crate) fn domain_upgrade_check_7_to_8_oauth2_strict_redirect_uri(
&mut self,
) -> Result<ProtoDomainUpgradeCheckItem, OperationError> {
let filter = filter!(f_and!([
f_eq(Attribute::Class, EntryClass::OAuth2ResourceServer.into()),
f_andnot(f_pres(Attribute::OAuth2StrictRedirectUri)),
]));
let results = self.internal_search(filter)?;
let affected_entries = results
.into_iter()
.map(|entry| entry.get_display_id())
.collect::<Vec<_>>();
let status = if affected_entries.is_empty() {
ProtoDomainUpgradeCheckStatus::Pass7To8Oauth2StrictRedirectUri
} else {
ProtoDomainUpgradeCheckStatus::Fail7To8Oauth2StrictRedirectUri
};
Ok(ProtoDomainUpgradeCheckItem {
status,
from_level: DOMAIN_LEVEL_7,
to_level: DOMAIN_LEVEL_8,
affected_entries,
})
}
}
#[cfg(test)]
mod tests {
use super::{ProtoDomainUpgradeCheckItem, ProtoDomainUpgradeCheckStatus};
use crate::prelude::*;
#[qs_test]
@ -1391,13 +1455,6 @@ mod tests {
.internal_create(vec![ea])
.expect("Unable to create oauth2 client");
// per migration verification.
let domain_entry = write_txn
.internal_search_uuid(UUID_DOMAIN_INFO)
.expect("Unable to access domain entry");
assert!(domain_entry.attribute_pres(Attribute::PrivateCookieKey));
// Set the version to 7.
write_txn
.internal_modify_uuid(
@ -1436,4 +1493,106 @@ mod tests {
write_txn.commit().expect("Unable to commit");
}
#[qs_test(domain_level=DOMAIN_LEVEL_7)]
async fn test_migrations_dl7_dl8(server: &QueryServer) {
// Assert our instance was setup to version 7
let mut write_txn = server.write(duration_from_epoch_now()).await;
let db_domain_version = write_txn
.internal_search_uuid(UUID_DOMAIN_INFO)
.expect("unable to access domain entry")
.get_ava_single_uint32(Attribute::Version)
.expect("Attribute Version not present");
assert_eq!(db_domain_version, DOMAIN_LEVEL_7);
// Create an oauth2 client that doesn't have a landing url set.
let oauth2_client_uuid = Uuid::new_v4();
let ea: Entry<EntryInit, EntryNew> = entry_init!(
(Attribute::Class, EntryClass::Object.to_value()),
(Attribute::Class, EntryClass::Account.to_value()),
(Attribute::Uuid, Value::Uuid(oauth2_client_uuid)),
(
Attribute::Class,
EntryClass::OAuth2ResourceServer.to_value()
),
(
Attribute::Class,
EntryClass::OAuth2ResourceServerPublic.to_value()
),
(Attribute::Name, Value::new_iname("test_resource_server")),
(
Attribute::DisplayName,
Value::new_utf8s("test_resource_server")
),
(
Attribute::OAuth2RsOriginLanding,
Value::new_url_s("https://demo.example.com/oauth2").unwrap()
),
(
Attribute::OAuth2RsOrigin,
Value::new_url_s("https://demo.example.com").unwrap()
)
);
write_txn
.internal_create(vec![ea])
.expect("Unable to create oauth2 client");
write_txn.commit().expect("Unable to commit");
// pre migration verification.
// check we currently would fail a migration.
let mut read_txn = server.read().await;
match read_txn.domain_upgrade_check_7_to_8_oauth2_strict_redirect_uri() {
Ok(ProtoDomainUpgradeCheckItem {
status: ProtoDomainUpgradeCheckStatus::Fail7To8Oauth2StrictRedirectUri,
..
}) => {
trace!("Failed as expected, very good.");
}
other => {
error!(?other);
unreachable!();
}
};
drop(read_txn);
// Okay, fix the problem.
let mut write_txn = server.write(duration_from_epoch_now()).await;
write_txn
.internal_modify_uuid(
oauth2_client_uuid,
&ModifyList::new_purge_and_set(
Attribute::OAuth2StrictRedirectUri,
Value::Bool(true),
),
)
.expect("Unable to enforce strict mode.");
// Set the version to 8.
write_txn
.internal_modify_uuid(
UUID_DOMAIN_INFO,
&ModifyList::new_purge_and_set(
Attribute::Version,
Value::new_uint32(DOMAIN_LEVEL_8),
),
)
.expect("Unable to set domain level to version 8");
// Re-load - this applies the migrations.
write_txn.reload().expect("Unable to reload transaction");
// post migration verification.
write_txn.commit().expect("Unable to commit");
}
}

View file

@ -38,6 +38,8 @@ impl Oauth2Opt {
| Oauth2Opt::DeleteClaimMap { copt, .. }
| Oauth2Opt::EnablePublicLocalhost { copt, .. }
| Oauth2Opt::DisablePublicLocalhost { copt, .. }
| Oauth2Opt::EnableStrictRedirectUri { copt, .. }
| Oauth2Opt::DisableStrictRedirectUri { copt, .. }
| Oauth2Opt::AddOrigin { copt, .. }
| Oauth2Opt::RemoveOrigin { copt, .. } => copt.debug,
}
@ -459,6 +461,27 @@ impl Oauth2Opt {
Err(e) => handle_client_error(e, copt.output_mode),
}
}
Oauth2Opt::EnableStrictRedirectUri { copt, name } => {
let client = copt.to_client(OpType::Write).await;
match client
.idm_oauth2_rs_enable_strict_redirect_uri(name.as_str())
.await
{
Ok(_) => println!("Success"),
Err(e) => handle_client_error(e, copt.output_mode),
}
}
Oauth2Opt::DisableStrictRedirectUri { copt, name } => {
let client = copt.to_client(OpType::Write).await;
match client
.idm_oauth2_rs_disable_strict_redirect_uri(name.as_str())
.await
{
Ok(_) => println!("Success"),
Err(e) => handle_client_error(e, copt.output_mode),
}
}
}
}
}

View file

@ -1062,23 +1062,23 @@ pub enum Oauth2Opt {
#[clap(name="remove-image")]
RemoveImage(Named),
/// Add a supplemental origin as a redirection target. For example a phone app
/// Add a supplemental URL as a redirection target. For example a phone app
/// may use a redirect URL such as `app://my-cool-app` to trigger a native
/// redirection event out of a browser.
#[clap(name = "add-origin")]
#[clap(name = "add-redirect-url")]
AddOrigin {
name: String,
#[clap(name = "origin-url")]
#[clap(name = "url")]
origin: Url,
#[clap(flatten)]
copt: CommonOpt,
},
/// Remove a supplemental origin from the OAuth2 client configuration.
#[clap(name = "remove-origin")]
/// Remove a supplemental redirect URL from the OAuth2 client configuration.
#[clap(name = "remove-redirect-url")]
RemoveOrigin {
name: String,
#[clap(name = "origin-url")]
#[clap(name = "url")]
origin: Url,
#[clap(flatten)]
copt: CommonOpt,
@ -1098,8 +1098,21 @@ pub enum Oauth2Opt {
/// Disable legacy signing crypto on this oauth2 client. This is the default.
#[clap(name = "disable-legacy-crypto")]
DisableLegacyCrypto(Named),
#[clap(name = "prefer-short-username")]
/// Enable strict validation of redirect URLs. Previously redirect URLs only
/// validated the origin of the URL matched. When enabled, redirect URLs must
/// match exactly.
#[clap(name = "enable-strict-redirect-url")]
EnableStrictRedirectUri {
name: String,
#[clap(flatten)]
copt: CommonOpt,
},
#[clap(name = "disable-strict-redirect-url")]
DisableStrictRedirectUri {
name: String,
#[clap(flatten)]
copt: CommonOpt,
},
#[clap(name = "enable-localhost-redirects")]
/// Allow public clients to redirect to localhost.
EnablePublicLocalhost {
@ -1114,11 +1127,11 @@ pub enum Oauth2Opt {
copt: CommonOpt,
name: String,
},
/// Use the 'name' attribute instead of 'spn' for the preferred_username
#[clap(name = "prefer-short-username")]
PreferShortUsername(Named),
#[clap(name = "prefer-spn-username")]
/// Use the 'spn' attribute instead of 'name' for the preferred_username
#[clap(name = "prefer-spn-username")]
PreferSPNUsername(Named),
}