Skip to content

Commit

Permalink
Merge pull request #324 from yetanalytics/sql-208
Browse files Browse the repository at this point in the history
[SQL-208] Proxy JWT Validation Bypass
  • Loading branch information
cliffcaseyyet authored Aug 28, 2023
2 parents 240e52b + d64d6d7 commit 5779c2e
Show file tree
Hide file tree
Showing 22 changed files with 299 additions and 70 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

# Version of LRS Admin UI to use

LRS_ADMIN_UI_VERSION ?= v0.1.9
LRS_ADMIN_UI_VERSION ?= v0.1.10
LRS_ADMIN_UI_LOCATION ?= https://github.com/yetanalytics/lrs-admin-ui/releases/download/${LRS_ADMIN_UI_VERSION}/lrs-admin-ui.zip
LRS_ADMIN_ZIPFILE ?= lrs-admin-ui-${LRS_ADMIN_UI_VERSION}.zip

Expand Down
1 change: 1 addition & 0 deletions doc/endpoints.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ The following examples use `http://example.org` as the URL body. All methods ret
- `POST http://example.org/admin/account/create`: Create a new admin account. The request body must be a JSON object that contains `username` and `password` strings. The endpoint returns a JSON object with the ID (UUID) of the newly created user on success, and returns a `409 CONFLICT` if the account already exists.
- `DELETE http://example.org/admin/account`: Delete an existing account. The JSON request body must contain a UUID `account-id` value. The endpoint returns a JSON object with the ID of the deleted account on success and returns a `404 NOT FOUND` error if the account does not exist.
- `GET http://example.org/admin/account`: Return an array of all admin accounts in the system on success.
- `GET http://example.org/admin/me`: Returns the currently authenticated admin accounts on success.

#### Admin Credential Routes

Expand Down
5 changes: 5 additions & 0 deletions doc/env_vars.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,11 @@ _NOTE:_ `LRSQL_STMT_RETRY_LIMIT` and `LRSQL_STMT_RETRY_BUDGET` are used to mitig
| `LRSQL_KEY_ENABLE_SELFIE` | `keyEnableSelfie` | Boolean, whether or not to enable self-signed cert generation. | `true` |
| `LRSQL_JWT_EXP_TIME` | `jwtExpTime` | The amount of time, in seconds, after a JWT is created when it expires. Since JWTs are not revocable, **this this time should be short** (i.e. one hour or less). | `3600` (one hour) |
| `LRSQL_JWT_EXP_LEEWAY` | `jwtExpLeeway` | The amount of time, in seconds, before or after the expiration instant when a JWT should still count as un-expired. Used to compensate for clock desync. Applied to both LRS and OIDC tokens. | `1` (one second) |
| `LRSQL_JWT_NO_VAL` | `jwtNoVal` | (**DANGEROUS!**) This flag removes JWT Token Validation and simply accepts token claims as configured by the associated variables below. It is extemely unlikely that you need this as it is for very specific proxy-overwrite authentication scenarios, and it poses a serious threat to system security if enabled. | `false` |
| `LRSQL_JWT_NO_VAL_UNAME` | `jwtNoValUname` | (**DANGEROUS!** See `LRSQL_JWT_NO_VAL`) This variable configures which claim key to use for the username when token validation is turned off. | Not Set |
| `LRSQL_JWT_NO_VAL_ISSUER` | `jwtNoValIssuer` | (**DANGEROUS!** See `LRSQL_JWT_NO_VAL`) This variable configures which claim key to use for the issuer when token validation is turned off. | Not Set |
| `LRSQL_JWT_NO_VAL_ROLE_KEY` | `jwtNoValRoleKey` | (**DANGEROUS!** See `LRSQL_JWT_NO_VAL`) This variable configures which claim key to look in for the role when token validation is turned off. | Not Set |
| `LRSQL_JWT_NO_VAL_ROLE` | `jwtNoValRole` | (**DANGEROUS!** See `LRSQL_JWT_NO_VAL`) This variable configures what role must be present in the key above when token validation is turned off. | Not Set |
| `LRSQL_ENABLE_HTTP` | `enableHttp` | Whether HTTP is enabled or not (as opposed to HTTPS, which is always enabled). | `true` |
| `LRSQL_ENABLE_HTTP2` | `enableHttp2` | Whether HTTP/2 is supported or not. | `true` |
| `LRSQL_HTTP_HOST` | `httpHost` | The host that the webserver will run on. | `0.0.0.0` |
Expand Down
5 changes: 5 additions & 0 deletions resources/lrsql/config/prod/default/webserver.edn
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
:key-enable-selfie #boolean #or [#env LRSQL_KEY_ENABLE_SELFIE true]
:jwt-exp-time #long #or [#env LRSQL_JWT_EXP_TIME 3600]
:jwt-exp-leeway #long #or [#env LRSQL_JWT_EXP_LEEWAY 1]
:jwt-no-val #boolean #or [#env LRSQL_JWT_NO_VAL false]
:jwt-no-val-uname #or [#env LRSQL_JWT_NO_VAL_UNAME nil]
:jwt-no-val-issuer #or [#env LRSQL_JWT_NO_VAL_ISSUER nil]
:jwt-no-val-role-key #or [#env LRSQL_JWT_NO_VAL_ROLE_KEY nil]
:jwt-no-val-role #or [#env LRSQL_JWT_NO_VAL_ROLE nil]
:enable-http #boolean #or [#env LRSQL_ENABLE_HTTP true]
:enable-http2 #boolean #or [#env LRSQL_ENABLE_HTTP2 true]
:http-host #or [#env LRSQL_HTTP_HOST "0.0.0.0"]
Expand Down
5 changes: 5 additions & 0 deletions resources/lrsql/config/test/default/webserver.edn
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
:key-enable-selfie true
:jwt-exp-time 3600
:jwt-exp-leeway 1
:jwt-no-val false
:jwt-no-val-uname nil
:jwt-no-val-issuer nil
:jwt-no-val-role-key nil
:jwt-no-val-role nil
:enable-http true
:enable-http2 true
:ssl-port 8443
Expand Down
2 changes: 1 addition & 1 deletion src/db/postgres/lrsql/postgres/sql/query.sql
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ WHERE activity_iri = :activity-iri
-- :command :query
-- :result :one
-- :doc Given an account `username` or `account-id`, return the ID and the hashed password, which can be used to verify the account.
SELECT id, passhash FROM admin_account
SELECT id, passhash, username FROM admin_account
--~ (when (:username params) "WHERE username = :username")
--~ (when (:account-id params) "WHERE id = :account-id")
;
Expand Down
2 changes: 1 addition & 1 deletion src/db/sqlite/lrsql/sqlite/sql/query.sql
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ WHERE activity_iri = :activity-iri
-- :command :query
-- :result :one
-- :doc Given an account `username` or `account-id`, return the ID and the hashed password, which can be used to verify the account.
SELECT id, passhash FROM admin_account
SELECT id, passhash, username FROM admin_account
--~ (when (:username params) "WHERE username = :username")
--~ (when (:account-id params) "WHERE id = :account-id")

Expand Down
14 changes: 14 additions & 0 deletions src/main/lrsql/admin/interceptors/account.clj
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
[lrsql.admin.protocol :as adp]
[lrsql.spec.admin :as ads]
[lrsql.util.admin :as admin-u]
[lrsql.admin.interceptors.jwt :as jwt]
[lrsql.util :as u]))

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
Expand Down Expand Up @@ -230,6 +231,19 @@
:response
{:status 200 :body result})))}))

(def me
"Get the currently authenticated account."
(interceptor
{:name ::get-me
:enter
(fn get-accounts [ctx]
(let [{lrs :com.yetanalytics/lrs
{:keys [account-id]} ::jwt/data} ctx
result (adp/-get-account lrs account-id)]
(assoc ctx
:response
{:status 200 :body result})))}))

(defn generate-jwt
"Upon account login, generate a new JSON web token."
[secret exp]
Expand Down
29 changes: 24 additions & 5 deletions src/main/lrsql/admin/interceptors/jwt.clj
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,42 @@
;; For JWT generation see `account/generate-jwt`.

(defn validate-jwt
"Validate that the header JWT is valid (e.g. not expired)."
[secret leeway]
"Validate that the header JWT is valid (e.g. not expired and signed properly).
If no-val? is true run an entirely separate decoding that gets the username
and issuer claims, verifies a role and ensures the account if necessary."
[secret leeway {:keys [no-val? no-val-uname no-val-issuer no-val-role-key
no-val-role]}]
(interceptor
{:name ::validate-jwt
:enter
(fn validate-jwt [ctx]
(let [token (-> ctx
(let [{lrs :com.yetanalytics/lrs} ctx
token (-> ctx
(get-in [:request :headers "authorization"])
admin-u/header->jwt)
result (admin-u/jwt->account-id token secret leeway)]
result (if no-val?
;; decode jwt w/o validation and ensure account
(let [{:keys [issuer username] :as result}
(admin-u/proxy-jwt->username-and-issuer
token no-val-uname no-val-issuer no-val-role-key
no-val-role)]
(if (some? username)
(:result (adp/-ensure-account-oidc lrs username issuer))
result))
;; normal jwt, check signature etc
(admin-u/jwt->account-id token secret leeway))]
(cond
;; Success - assoc the account ID as an intermediate value
(uuid? result)
(-> ctx
(assoc-in [::data :account-id] result)
(assoc-in [:request :session ::data :account-id] result))

;; Problem with the non-validated account ensure
(= :lrsql.admin/oidc-issuer-mismatch-error result)
(assoc (chain/terminate ctx)
:response
{:status 401
:body {:error "Token Issuer Mismatch!"}})
;; The token is bad (expired, malformed, etc.) - Unauthorized
(= :lrsql.admin/unauthorized-token-error result)
(assoc (chain/terminate ctx)
Expand Down
8 changes: 5 additions & 3 deletions src/main/lrsql/admin/interceptors/ui.clj
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@
config to inject:
:enable-admin-status - boolean, determines if the admin status endpoint is
enabled."
[{:keys [enable-admin-status]
:or {enable-admin-status false}}]
[{:keys [enable-admin-status no-val?]
:or {enable-admin-status false
no-val? false}}]
(interceptor
{:name ::get-env
:enter
Expand All @@ -30,5 +31,6 @@
(merge
{:url-prefix url-prefix
:enable-stmt-html (some? enable-stmt-html)
:enable-admin-status enable-admin-status}
:enable-admin-status enable-admin-status
:no-val? no-val?}
oidc-env)})))}))
2 changes: 2 additions & 0 deletions src/main/lrsql/admin/protocol.clj
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
"Authenticate by looking up if the account exists in the account table.")
(-existing-account? [this account-id]
"Check that the account with the given ID exists in the account table. Returns a boolean.")
(-get-account [this account-id]
"Get the account with the given ID exists in the account table. Returns a boolean.")
(-delete-account [this account-id oidc-enabled?]
"Delete the account and all associated creds. Assumes the account has already been authenticated. Requires OIDC status to prevent sole account deletion.")
(-ensure-account-oidc [this username oidc-issuer]
Expand Down
61 changes: 44 additions & 17 deletions src/main/lrsql/admin/routes.clj
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
(i/lrs-interceptor lrs)])

(defn admin-account-routes
[common-interceptors jwt-secret jwt-exp jwt-leeway]
[common-interceptors jwt-secret jwt-exp jwt-leeway no-val-opts]
#{;; Log into an existing account
["/admin/account/login" :post (conj common-interceptors
ai/validate-params
Expand All @@ -30,68 +30,83 @@
;; Create new account
["/admin/account/create" :post (conj common-interceptors
ai/validate-params
(ji/validate-jwt jwt-secret jwt-leeway)
(ji/validate-jwt
jwt-secret jwt-leeway no-val-opts)
ji/validate-jwt-account
ai/create-admin)
:route-name :lrsql.admin.account/create]
;; Update account password
["/admin/account/password"
:put (conj common-interceptors
ai/validate-update-password-params
(ji/validate-jwt jwt-secret jwt-leeway)
(ji/validate-jwt jwt-secret jwt-leeway no-val-opts)
ji/validate-jwt-account
ai/update-admin-password)]
;; Get all accounts
["/admin/account" :get (conj common-interceptors
(ji/validate-jwt jwt-secret jwt-leeway)
(ji/validate-jwt
jwt-secret jwt-leeway no-val-opts)
ji/validate-jwt-account
ai/get-accounts)
:route-name :lrsql.admin.account/get]
;; Get my accounts
["/admin/me" :get (conj common-interceptors
(ji/validate-jwt
jwt-secret jwt-leeway no-val-opts)
ji/validate-jwt-account
ai/me)
:route-name :lrsql.admin.me/get]
;; Delete account (and associated credentials)
["/admin/account" :delete (conj common-interceptors
ai/validate-delete-params
(ji/validate-jwt jwt-secret jwt-leeway)
(ji/validate-jwt
jwt-secret jwt-leeway no-val-opts)
ji/validate-jwt-account
ai/delete-admin)
:route-name :lrsql.admin.account/delete]})

(defn admin-cred-routes
[common-interceptors jwt-secret jwt-leeway]
[common-interceptors jwt-secret jwt-leeway no-val-opts]
#{;; Create new API key pair w/ scope set
["/admin/creds" :post (conj common-interceptors
(ci/validate-params {:scopes? true})
(ji/validate-jwt jwt-secret jwt-leeway)
(ji/validate-jwt
jwt-secret jwt-leeway no-val-opts)
ji/validate-jwt-account
ci/create-api-keys)
:route-name :lrsql.admin.creds/put]
;; Create or update new keys w/ scope set
["/admin/creds" :put (conj common-interceptors
(ci/validate-params {:key-pair? true
:scopes? true})
(ji/validate-jwt jwt-secret jwt-leeway)
(ji/validate-jwt
jwt-secret jwt-leeway no-val-opts)
ji/validate-jwt-account
ci/update-api-keys)
:route-name :lrsql.admin.creds/post]
;; Get current keys + scopes associated w/ account
["/admin/creds" :get (conj common-interceptors
(ji/validate-jwt jwt-secret jwt-leeway)
(ji/validate-jwt
jwt-secret jwt-leeway no-val-opts)
ji/validate-jwt-account
ci/read-api-keys)
:route-name :lrsql.admin.creds/get]
;; Delete API key pair and associated scopes
["/admin/creds" :delete (conj common-interceptors
(ci/validate-params {:key-pair? true})
(ji/validate-jwt jwt-secret jwt-leeway)
(ji/validate-jwt
jwt-secret jwt-leeway no-val-opts)
ji/validate-jwt-account
ci/delete-api-keys)
:route-name :lrsql.admin.creds/delete]})

(defn admin-status-routes
[common-interceptors jwt-secret jwt-leeway]
[common-interceptors jwt-secret jwt-leeway no-val-opts]
#{;; Return LRS Status information
["/admin/status" :get (conj common-interceptors
si/validate-params
(ji/validate-jwt jwt-secret jwt-leeway)
(ji/validate-jwt
jwt-secret jwt-leeway no-val-opts)
ji/validate-jwt-account
si/get-status)
:route-name :lrsql.admin.status/get]})
Expand Down Expand Up @@ -120,6 +135,11 @@
exp
leeway
secret
no-val?
no-val-issuer
no-val-uname
no-val-role-key
no-val-role
enable-admin-ui
enable-admin-status
enable-account-routes
Expand All @@ -130,17 +150,24 @@
enable-account-routes true}}
routes]
(let [common-interceptors (make-common-interceptors lrs)
common-interceptors-oidc (into common-interceptors oidc-interceptors)]
common-interceptors-oidc (into common-interceptors oidc-interceptors)
no-val-opts {:no-val? no-val?
:no-val-uname no-val-uname
:no-val-issuer no-val-issuer
:no-val-role-key no-val-role-key
:no-val-role no-val-role}]
(cset/union routes
(when enable-account-routes
(admin-account-routes
common-interceptors-oidc secret exp leeway))
(admin-cred-routes common-interceptors-oidc secret leeway)
common-interceptors-oidc secret exp leeway no-val-opts))
(admin-cred-routes
common-interceptors-oidc secret leeway no-val-opts)
(when enable-admin-ui
(admin-ui-routes
(into common-interceptors
oidc-ui-interceptors)
{:enable-admin-status enable-admin-status}))
{:enable-admin-status enable-admin-status
:no-val? no-val?}))
(when enable-admin-status
(admin-status-routes
common-interceptors-oidc secret leeway)))))
common-interceptors-oidc secret leeway no-val-opts)))))
9 changes: 9 additions & 0 deletions src/main/lrsql/input/admin.clj
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,15 @@
[ensure-input]
(u/add-primary-key ensure-input))

(s/fdef get-account-input
:args (s/cat :account-id ::ads/account-id)
:ret ads/admin-id-input-spec)

(defn query-account-input
"Given `account-id`, construct the input param map for `get-account`."
[account-id]
{:account-id account-id})

(s/fdef delete-admin-input
:args (s/cat :account-id ::ads/account-id
:oidc-enabled? :lrsql.spec.admin.input/oidc-enabled?)
Expand Down
10 changes: 6 additions & 4 deletions src/main/lrsql/ops/query/admin.clj
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,16 @@

(defn query-admin
"Query an admin account with the given username (or account-id) and password.
Returns a map containing `:account-id` and `:passhash` on success, or nil on
failure."
Returns a map containing `:account-id`, `:passhash` and `username` on
success, or nil on failure."
[bk tx input]
(when-some [{account-id :id
passhash :passhash}
passhash :passhash
username :username}
(bp/-query-account bk tx input)]
{:account-id account-id
:passhash passhash}))
:passhash passhash
:username username}))

(defn query-admin-exists
"Query whether an admin account with the given ID exists. Returns true
Expand Down
Loading

0 comments on commit 5779c2e

Please sign in to comment.