diff --git a/design/authenticators/authn_jwt/authn-jwt-fetch-more-keys.md b/design/authenticators/authn_jwt/authn-jwt-fetch-more-keys.md
index c094b6cd32..5e662d023a 100644
--- a/design/authenticators/authn_jwt/authn-jwt-fetch-more-keys.md
+++ b/design/authenticators/authn_jwt/authn-jwt-fetch-more-keys.md
@@ -1,78 +1,71 @@
-# Solution Design - Enhance public key delivery mechanisms
-[//]: # "Change the title above from 'Template' to your design's title"
-
-[//]: # "General notes:"
-[//]: # "1. Design should be graphical-based and table-based - avoid long text explanations"
-[//]: # "2. Design documents should not be updated after implementation"
-[//]: # "3. Design decisions should be made before writing this document, and as such this document should not include options / choices"
-
+# Solution Design - Enhance public key delivery mechanisms
## Table of Contents
-[//]: # "You can use this tool to generate a TOC - https://ecotrust-canada.github.io/markdown-toc/"
-
-## Glossary
-[//]: # "Describe terms that will be used throughout the design"
-[//]: # "You can use this tool to generate a table - https://www.tablesgenerator.com/markdown_tables#"
-
-| **Term** | **Description** |
-|----------|-----------------|
-| | |
-| | |
-
-## Useful Links
-[//]: # "Add links that may be useful for the reader"
-| **Name** | **Link** |
-|-------------|----------|
-| Feature Doc | |
-| Issue | |
+* [Background](#background)
+* [Issue Description](#issue-description)
+* [Solution](#solution)
+ * [User Interface](#user-interface)
+ * [`public-keys` variable](#-public-keys--variable)
+ * [`ca-cert` variable](#-ca-cert--variable)
+* [Design](#design)
+ * [Expected validation errors](#expected-validation-errors)
+ * [Expected validation errors](#expected-validation-errors-1)
+* [Performance](#performance)
+* [Backwards Compatibility](#backwards-compatibility)
+* [Work in Parallel](#work-in-parallel)
+ * [Effort estimation](#effort-estimation)
+* [Test Plan](#test-plan)
+* [Definition of Done](#definition-of-done)
## Background
-[//]: # "Give relevant background for the designed feature. What is the motivation for this solution?"
JWT Authenticator has two ways of fetching public keys in order to verify JWT tokens:
-- `jwks-uri` - is a URI that refers to a resource for a set of JSON-encoded public keys, one of which
- corresponds to the key used to digitally sign the JWS
-- `provider-uri` - is a URI that refers to a server supports OpenID Connect standard
-Current state requires connectivity between Conjur and a host of jwks/provider-url. If Conjur is unable to connect
-the host authentication will fail.
+* `jwks-uri` - is a URI that refers to a resource for a set of JSON-encoded
+ public keys, one of which corresponds to the key used to digitally sign the JWS
+* `provider-uri` - is a URI that refers to a server supports OpenID Connect standard
-In addition fetching public key is possible if and only if a target https uri presents certificate is signed by
-well-known certificate authority.
+Current state requires connectivity between Conjur and a host of jwks/provider-url.
+If Conjur is unable to connect the host authentication will fail.
+
+In addition fetching public key is possible if and only if a target https uri presents
+certificate is signed by well-known certificate authority.
## Issue Description
-[//]: # "Elaborate on the issue you are writing a solution for"
-There're use-cases where jwks/provider-uri is unreachable by Conjur or presents certificate signed by 3rd party
-certificate authority/self-signed.
+There're use-cases where jwks/provider-uri is unreachable by Conjur or presents certificate
+signed by 3rd party certificate authority/self-signed.
At the moment both use-cases are unsupported and required code changes in JWT authenticator.
## Solution
-[//]: # "Elaborate on the solution you are suggesting in this page. Address the functional requirements and the non functional requirements that this solution is addressing. If there are a few options considered for the solution, mention them and explain why the actual solution was chosen over them. Add an execution plan when relevant. It doesn't have to be a full breakdown of the feature, but just a recommendation to how the solution should be approached."
Add two new variables to the authenticator policy:
-- `public-keys` - a variable contains static JWKS value for the case where Conjur is unable to reach
- server hosts jwks/provider-uri.
+* `public-keys` - a variable contains static JWKS value for the case where Conjur
+ is unable to reach server hosts jwks/provider-uri.
The variable is optional.
- The variable should be defined without other "key delivering" variables. If `public-keys` variable is defined
- alongside with `jwks-uri` and/or `provider-uri` and/or `ca-cert` variable authentication will fail.
-
- When `public-keys` variable is defined `issuer` variable became mandatory (in case of jwks/provider-uri variables
- Conjur is trying to figure out issuer from variables values).
+ The variable should be defined without other "key delivering" variables. If `public-keys`
+ variable is defined alongside with `jwks-uri` and/or `provider-uri` and/or `ca-cert`
+ variable authentication will fail.
+
+ When `public-keys` variable is defined `issuer` variable became mandatory
+ (in case of jwks/provider-uri variables Conjur is trying to figure out issuer
+ from variables values).
`public-keys` variable format is JSON with two fields:
- - `type` - the type of public keys. At the moment only one type `jwks` will be supported. More types can be
- supported in future.
- - `value` - the value of JWKS as is it's returned from unreachable jwks/provider-uri endpoint.
+ * `type` - the type of public keys. At the moment only one type `jwks` will be
+ supported. More types can be supported in future.
+ * `value` - the value of JWKS as is it's returned from unreachable jwks/provider-uri
+ endpoint.
JSON example
For example (based on https://www.googleapis.com/oauth2/v3/certs):
-
+
```json
+
{
"type": "jwks",
"value":
@@ -98,22 +91,25 @@ Add two new variables to the authenticator policy:
]
}
}
+
```
-
-- `ca-cert` - a variable contains certificates bundle with additional CA certificates conjur will trust when it
+
+
+* `ca-cert` - a variable contains certificates bundle with additional CA certificates
+ conjur will trust when it
approaches jwks/provider uri.
The variable is optional.
- The variable can be defined with one of "key delivering" variables: `jwks-uri` or `provider-uri`.
-
- The variable cannot be define with `public-keys` variable. If `ca-cert` variable is defined with `public-keys`
- variable authentication will fail.
+ The variable can be defined with one of "key delivering" variables: `jwks-uri`
+ or `provider-uri`.
- The `ca-cert` variable format is a certificate bundle contains one or more certificates in pem
- ([rfc7468](https://datatracker.ietf.org/doc/html/rfc7468)) format.
+ The variable cannot be define with `public-keys` variable. If `ca-cert` variable
+ is defined with `public-keys` variable authentication will fail.
+
+ The `ca-cert` variable format is a certificate bundle contains one or more certificates
+ in pem ([rfc7468](https://datatracker.ietf.org/doc/html/rfc7468)) format.
Bundle example
```
@@ -257,15 +253,16 @@ Add two new variables to the authenticator policy:
d0lIKO2d1xozclOzgjXPYovJJIultzkMu34qQb9Sz/yilrbCgj8=
-----END CERTIFICATE-----
```
+
### User Interface
-[//]: # "Describe user interface (including command structure, inputs/outputs, etc where relevant)"
#### `public-keys` variable
JWT Authenticator policy contains new `public-keys` variable example:
+
```yaml
- !policy
id: conjur/authn-jwt/myUnreachableVendor
@@ -278,6 +275,7 @@ JWT Authenticator policy contains new `public-keys` variable example:
Populating relevant variables using [Python CLI](https://github.com/cyberark/conjur-api-python3)
(based on google's [openid configuration](https://accounts.google.com/.well-known/openid-configuration)):
+
```bash
conjur variable set -i conjur/authn-jwt/myUnreachableVendor/public-keys \
"{ \"type\":\"jwks\", \"value\": $(curl https://www.googleapis.com/oauth2/v3/certs) }"
@@ -288,6 +286,7 @@ conjur variable set -i conjur/authn-jwt/myUnreachableVendor/issuer \
#### `ca-cert` variable
JWT Authenticator policy contains new `ca-cert` variable example:
+
```yaml
- !policy
id: conjur/authn-jwt/myUntrustedVendor
@@ -298,328 +297,281 @@ JWT Authenticator policy contains new `ca-cert` variable example:
...
```
-Populating relevant variables using [Python CLI](https://github.com/cyberark/conjur-api-python3) and
-[OpenSSL command line tool](https://www.openssl.org/docs/manmaster/man1/openssl.html) for fetching certificate bundle:
+Populating relevant variables using [Python CLI](https://github.com/cyberark/conjur-api-python3)
+and [OpenSSL command line tool](https://www.openssl.org/docs/manmaster/man1/openssl.html)
+for fetching certificate bundle:
+
```bash
conjur variable set -i conjur/authn-jwt/myUntrustedVendor/provider-uri \
"https://provider.host.name"
conjur variable set -i conjur/authn-jwt/myUntrustedVendor/ca-cert \
- "$(openssl s_client -showcerts -connect provider.host.name:443 -servername provider.host.name < /dev/null 2>/dev/null \
- | awk '/-----BEGIN/,/-----END/')"
+ "$(openssl s_client \
+ -showcerts \
+ -connect provider.host.name:443 \
+ -servername provider.host.name \
+ < /dev/null 2>/dev/null \
+ | awk '/-----BEGIN/,/-----END/')"
```
## Design
-[//]: # "Add any diagrams, charts and explanations about the design aspect of the solution. Elaborate also about the expected user experience for the feature"
-Today authenticator's code is tightly coupled to the way the authenticator is configured - conjur variables.
+Today authenticator's code is tightly coupled to the way the authenticator is
+configured - conjur variables.
-The next design is taking a step forward in direction of decoupling variables from the main authenticator's business logic.
+The next design is taking a step forward in direction of decoupling variables from
+the main authenticator's business logic.
-The design is considering around and changing code area presented on the next class diagram:
+The design is considering around and changing code area presented on the next class
+diagram:
-Next sections are describing code changes step by step towards functional - supporting public-keys configuration, and
-non-functional - decoupling variables from main BL requirements.
+Next sections are describing code changes step by step towards functional - supporting
+public-keys configuration, and non-functional - decoupling variables from main BL
+requirements.
1. Create abstraction layer for relevant Conjur variables from authenticator's BL
- New class `SigningKeySettings` the class is [POJO](https://en.wikipedia.org/wiki/Plain_old_Java_object) like
- class for JWKS fetching related settings of the authenticator.
+ New class `SigningKeySettings` the class is [POJO](https://en.wikipedia.org/wiki/Plain_old_Java_object)
+ like class for JWKS fetching related settings of the authenticator.
+
The class will expose next properties:
- - `signing_key_uri`: `String` - returns `jwks-uri` or `provider-uri` variable value
- - `signing_key_type`: one of [`JWKS`, `OIDC`] - returns provider type base on existing variables
-
+
+ * `signing_key_uri`: `String` - returns `jwks-uri` or `provider-uri` variable
+ value
+ * `signing_key_type`: one of [`JWKS`, `OIDC`] - returns provider type base on
+ existing variables
+
New class `FetchSigningKeySettingsFromVariables` the command class:
- - fetches settings from variables
- - validates settings
- - populates and returns `SigningKeySettings` instance
-
- The responsibility for variables permutation validation is moving from `CreateSigningKeyProvider` class
- to the new `FetchSigningKeySettingsFromVariables` class.
+
+ * fetches settings from variables
+ * validates settings
+ * populates and returns `SigningKeySettings` instance
+
+ The responsibility for variables permutation validation is moving from `CreateSigningKeyProvider`
+ class to the new `FetchSigningKeySettingsFromVariables` class.
+
The class also figures out and sets `signing_key_type` which will be used by `CreateSigningKeyProvider`
to create needed signing key provider.
Integrate both classes into existing flow:
- - `FetchSigningKeySettingsFromVariables` is `CreateSigningKeyProvider` dependency
- - Change `FetchJwksUriSigningKey` and `FetchProviderUriSigningKey` input parameters
+
+ * `FetchSigningKeySettingsFromVariables` is `CreateSigningKeyProvider` dependency
+ * Change `FetchJwksUriSigningKey` and `FetchProviderUriSigningKey` input parameters
+
Add new properties to the `SigningKeySettings` class:
- - `signing_key`: String - returns `public-keys` variable value
- - `ca_cert`: `OpenSSL::X509::Store` - see [authn-k8s](https://github.com/cyberark/conjur/blob/5335422839f11c8bcbef4d3d0c5e01f2878cf6a4/app/domain/authentication/authn_k8s/k8s_object_lookup.rb#L25)
+
+ * `signing_key`: String - returns `public-keys` variable value
+ * `ca_cert`: `OpenSSL::X509::Store` - see [authn-k8s](https://github.com/cyberark/conjur/blob/5335422839f11c8bcbef4d3d0c5e01f2878cf6a4/app/domain/authentication/authn_k8s/k8s_object_lookup.rb#L25)
example how to transform `ca-cert` variable value into an `OpenSSL::X509::Store`
-
- Add new `signing_key_type` - `STATIC`. The provider type static when `public-keys` variable is set and non empty.
+
+ Add new `signing_key_type` - `STATIC`. The provider type static when `public-keys`
+ variable is set and non empty.
- Modify `FetchSigningKeySettingsFromVariables` class accordingly.
+ Modify `FetchSigningKeySettingsFromVariables` class accordingly.
Implement subsequent input validation:
- - Only one of `jwks-uri`, `provider-uri`, `public-keys` variables can be define
- - `ca-cert` variable can be defined only with `jwks-uri` or `provider-uri` variables
- - If `public-keys` variable is defined `issuer` variable should be defined too
- - Each defined variable should be non empty
-
+
+ * Only one of `jwks-uri`, `provider-uri`, `public-keys` variables can be define
+ * `ca-cert` variable can be defined only with `jwks-uri` or `provider-uri` variables
+ * If `public-keys` variable is defined `issuer` variable should be defined too
+ * Each defined variable should be non empty
+
### Expected validation errors
+
| **Error message** | **Description** |
|-------------------|-----------------|
| Signing key configuration is invalid: only one of `jwks-uri`, `provider-uri`, `public-keys` variables can be define simultaneously | When more than one variable is defined |
| Signing key configuration is invalid: `ca-cert` variable can be defined only with `jwks-uri` or `provider-uri` variables | When `ca-cert` variable defined with `public-keys` one |
- | Signing key configuration is invalid: missing `issuer` variable | When `public-keys` variable is defined but `issuer` is not |
+ | Signing key configuration is invalid: missing `issuer` variable | When `public-keys` variable is defined but `issuer` is not |
| Signing key configuration is invalid: {variable-name} variable is defined but empty | When variables permutation is valid but one of it is empty |
- Pass `ca_cert`:`OpenSSL::X509::Store` parameter to `FetchJwksUriSigningKey`, `FetchProviderUriSigningKey` and
- `OAuth::DiscoverIdentityProvider` classes.
+ Pass `ca_cert`:`OpenSSL::X509::Store` parameter to `FetchJwksUriSigningKey`, `FetchProviderUriSigningKey`
+ and `OAuth::DiscoverIdentityProvider` classes.
`ca_cert` parameter is optional, default value is `nil`.
- **Modify the way `FetchJwksUriSigningKey` invokes `Net::HTTP` `GET` request to `jwks_url` in a way allows to trust
- certificates from `ca_cert`:`OpenSSL::X509::Store`.**
-
- See [Net::HTTP](https://ruby-doc.org/stdlib-2.5.8/libdoc/net/http/rdoc/Net/HTTP.html) reference.
+ **Modify the way `FetchJwksUriSigningKey` invokes `Net::HTTP` `GET` request
+ to `jwks_url` in a way allows to trust certificates from `ca_cert`:`OpenSSL::X509::Store`.**
+
+ See [Net::HTTP](https://ruby-doc.org/stdlib-2.5.8/libdoc/net/http/rdoc/Net/HTTP.html)
+ reference.
Suggested implementation:
+
```ruby
uri = URI(jwks_uri)
- http = Net::HTTP.start(uri.host, uri.port, :use_ssl => true, :cert_store => ca_cert )
+ http = Net::HTTP.start(
+ uri.host,
+ uri.port,
+ :use_ssl => true,
+ :cert_store => ca_cert)
request = Net::HTTP::Get.new(uri)
response = http.request(request)
```
- **Modify the way `OAuth::DiscoverIdentityProvider` invokes `OpenIDConnect::Discovery::Provider::Config` `discover!`
- request in a way allows to trust certificates from `ca_cert`:`OpenSSL::X509::Store`.**
+ **Modify the way `OAuth::DiscoverIdentityProvider` invokes `OpenIDConnect::Discovery::Provider::Config`
+ `discover!` request in a way allows to trust certificates from `ca_cert`:`OpenSSL::X509::Store`.**
Suggested implementation:
+
```ruby
OpenIDConnect.http_config do |config|
config.ssl_config.cert_store = ca_cert
end
openid_config = OpenIDConnect::Discovery::Provider::Config.discover!(provider_uri)
```
- **Risk:** state-full `OpenIDConnect` configuration - need to check with multiple authenticators are configured
- with `provider-uri` and different `ca-cert`.
- Suggested testing approach: do not run `ci/keycloak/fetch_certificate` script as a part of env deployment.
- Replace the functionality with `ca-cert` variable.
+ **Risk:** state-full `OpenIDConnect` configuration - need to check with multiple
+ authenticators are configured with `provider-uri` and different `ca-cert`.
+
+ Suggested testing approach: do not run `ci/keycloak/fetch_certificate` script
+ as a part of env deployment. Replace the functionality with `ca-cert` variable.
- Fallback scenario: to support `ca-cert` variable only for `jwks-uri` variable.
+ Fallback scenario: to support `ca-cert` variable only for `jwks-uri` variable.
Today cache (`ConcurrencyLimitedCache`):
- - combines two responsibilities caching and throttling mechanism around fetching signing keys.
- - has two instances one under `ValidateAndDecodeToken` and one under `ValidateStatus`. Each instance has its own
- state independent from each other.
- - caches responses from `Fetch*SigningKey` classes. The cache does not distinct types of signing key fetchers.
-
- Caching behaviour is completely unnecessary in case when signing key is in DB. Changing signing key in variable
- should take an immediate effect on the authenticator.
-
- In order to prevent from DB based signing key to be cached the decision to cache or not to cache should be taken
- inside concrete signing key provider. In order to achieve it the following changes need to be implemented
- (see [cyberark/conjur#2447](https://github.com/cyberark/conjur/pull/2447) for POC implementation):
- - Make cache instance to the `CreateSigningKeyProvider` class dependencies. Remove cache instance from
- `ValidateAndDecodeToken` and `ValidateStatus` classes dependencies. Additional side effect of this change is
- that there will be only one instance of cache that will prevent additional approaches to signing key provider
- from authenticate or status flows once it's cached.
- - Make cache a ctor parameter of `Fetch*SigningKey` classes. `CreateSigningKeyProvider` will pass cache dependency
- into the classes when create them.
- - Adjust calling sequence:
- - `ValidateAndDecodeToken` and `ValidateStatus` are calling `call` method of `Fetch*SigningKey` classes with
- optional `force_read` flag instead of `call` method of the `ConcurrencyLimitedCache` class. _`call` is a new
- method in ephemeral `FetchSigningKey` interface._
- - `Fetch*SigningKey` classes are calling `call` method of the `ConcurrencyLimitedCache` class with appropriate
- parameters. _`signing_key_url` is not logger a part of ephemeral `FetchSigningKey` interface `cache_key` is
- defined internally in concrete `Fetch*SigningKey` class._
+
+ * combines two responsibilities caching and throttling mechanism around fetching
+ signing keys.
+ * has two instances one under `ValidateAndDecodeToken` and one under `ValidateStatus`.
+ Each instance has its own state independent from each other.
+ * caches responses from `Fetch*SigningKey` classes. The cache does not distinct
+ types of signing key fetchers.
+
+ Caching behaviour is completely unnecessary in case when signing key is in DB.
+ Changing signing key in variable should take an immediate effect on the authenticator.
+
+ In order to prevent from DB based signing key to be cached the decision to
+ cache or not to cache should be taken inside concrete signing key provider.
+ In order to achieve it the following changes need to be implemented (see
+ [cyberark/conjur#2447](https://github.com/cyberark/conjur/pull/2447) for POC
+ implementation):
+
+ * Make cache instance to the `CreateSigningKeyProvider` class dependencies.
+ Remove cache instance from `ValidateAndDecodeToken` and `ValidateStatus`
+ classes dependencies. Additional side effect of this change is that there
+ will be only one instance of cache that will prevent additional approaches to
+ signing key provider from authenticate or status flows once it's cached.
+ * Make cache a ctor parameter of `Fetch*SigningKey` classes. `CreateSigningKeyProvider`
+ will pass cache dependency into the classes when create them.
+ * Adjust calling sequence:
+ * `ValidateAndDecodeToken` and `ValidateStatus` are calling `call` method of
+ `Fetch*SigningKey` classes with optional `force_read` flag instead of `call`
+ method of the `ConcurrencyLimitedCache` class. _`call` is a new method in
+ ephemeral `FetchSigningKey` interface._
+ * `Fetch*SigningKey` classes are calling `call` method of the `ConcurrencyLimitedCache`
+ class with appropriate parameters. _`signing_key_url` is not logger a part
+ of ephemeral `FetchSigningKey` interface `cache_key` is defined internally
+ in concrete `Fetch*SigningKey` class._
FetchStaticSigningKey` class:
- - partially implements ephemeral `FetchSigningKey` interface:
- - `call` method receives `force_read` parameter, but ignores it
- - `fetch_signing_key` method does not require implementation
- - receives `signing_key` value in ctor
- - parses and validates `signing_key` value in `call`
- For reference see expected validation errors below and `CreateJwksFromHttpResponse.parse_jwks_response`
+
+ * partially implements ephemeral `FetchSigningKey` interface:
+ * `call` method receives `force_read` parameter, but ignores it
+ * `fetch_signing_key` method does not require implementation
+ * receives `signing_key` value in ctor
+ * parses and validates `signing_key` value in `call`
+ For reference see expected validation errors below and `CreateJwksFromHttpResponse.parse_jwks_response`
`CreateSigningKeyProvider` class:
- - creates and returns `FetchStaticSigningKey` class when `signing_key_type` is `STATIC`
- - passes `signing_key` from `SigningKeySettings`
- 
+ * creates and returns `FetchStaticSigningKey` class when `signing_key_type`
+ is `STATIC`
+ * passes `signing_key` from `SigningKeySettings`
+
+ 
### Expected validation errors
+
| **Error message** | **Description** |
|-------------------|-----------------|
- | Signing key configuration is invalid: JSON parsing error from JSON gem | When the variable value is not a valid JSON |
- | Signing key configuration is invalid: `public-keys` `type` field value is missing or empty | When type field absent or has empty value |
+ | Signing key configuration is invalid: JSON parsing error from JSON gem | When the variable value is not a valid JSON |
+ | Signing key configuration is invalid: `public-keys` `type` field value is missing or empty | When type field absent or has empty value |
| Signing key configuration is invalid: `public-keys` `type` field value {} is wrong | When type field value is no `jwks` |
| Signing key configuration is invalid: `public-keys` `value` field is missing or empty | When value field absent or has empty value |