feat(jans-lock): Acquire config from JS, Improved codebase structure
Prepare
- [x] Read PR guidelines
- [x] Read license information
Test and Document the changes
- [x] Static code analysis has been run locally and issues have been fixed
- [ ] Relevant unit and integration tests have been added/updated
- [ ] Relevant documentation has been updated if any (i.e. user guides, installation and configuration guides, technical design docs etc)
\nPlease check the below before submitting your PR. The PR will not be merged if there are no commits that start with docs: to indicate documentation changes or if the below checklist is not selected.\n- [x] I confirm that there is no impact on the docs due to the code changes in this PR.
Hi there :wave:, @dryrunsecurity here, below is a summary of our analysis and findings.
| DryRun Security | Status | Findings |
|---|---|---|
| Server-Side Request Forgery Analyzer | :white_check_mark: | 0 findings |
| Configured Codepaths Analyzer | :white_check_mark: | 0 findings |
| Secrets Analyzer | :white_check_mark: | 0 findings |
| Authn/Authz Analyzer | :white_check_mark: | 0 findings |
| SQL Injection Analyzer | :white_check_mark: | 0 findings |
| Sensitive Files Analyzer | :white_check_mark: | 0 findings |
| IDOR Analyzer | :white_check_mark: | 0 findings |
[!Note] :green_circle: Risk threshold not exceeded.
Change Summary (click to expand)
The following is a summary of changes in this pull request made by me, your security buddy :robot:. Note that this summary is auto-generated and not meant to be a definitive list of security issues but rather a helpful summary from a security perspective.
Summary:
The code changes in this pull request span across multiple files in the
jans-lock/cedarlingproject, primarily focusing on updates to the application's security-related functionality, such as JSON Web Token (JWT) validation, policy store management, and dynamic client registration for OAuth 2.0.The key security-relevant changes include:
JWT Validation: The code in the
crypto/decode.rsandcrypto/types.rsfiles has been updated to improve the handling and validation of JWTs, including the use of a trust store to look up the appropriate JSON Web Keys (JWKs) for verifying the JWT signatures.Policy Store Management: The changes in the
startup/mod.rsandstartup/types.rsfiles introduce new configuration options for the policy store, including support for local, remote, and "lock-master" configurations. The security of the policy store is crucial, as it contains the application's authorization policies.Dynamic Client Registration: The code in the
lock_master/mod.rsfile demonstrates the implementation of dynamic client registration for OAuth 2.0, which is a security-conscious approach to avoid hardcoding client credentials.Token Handling: The application's handling of access tokens, ID tokens, and other sensitive tokens has been reviewed and updated to ensure proper validation, storage, and usage.
Input Validation: The code changes highlight the importance of properly validating and sanitizing user input, especially in the context of deserialization of untrusted data, to prevent potential security vulnerabilities.
Overall, the changes in this pull request appear to be focused on improving the security posture of the Cedarling application, particularly in the areas of JWT validation, policy store management, and OAuth 2.0 client registration. As an application security engineer, I would recommend thoroughly reviewing the implementation of these security-critical components to ensure that they are designed and implemented securely.
Files Changed:
jans-lock/cedarling/Cargo.toml: Added a new dependency on thejsonwebtokencrate, which is likely used for handling JSON Web Tokens (JWT) in the application.jans-lock/cedarling/.gitignore: Added a new line to exclude the/outdirectory from being tracked by Git.jans-lock/cedarling/Cargo.lock: Updated the dependencies, including the addition of thejsonwebtokencrate.jans-lock/cedarling/README.md: Updated the documentation to provide information about the policy store strategies and other security-relevant features of the Cedarling application.jans-lock/cedarling/src/authz/token2entity.rs: Added a comment describing the purpose of this file, which is to map tokens to entities on the fly for the authorization module.jans-lock/cedarling/src/authz/mod.rs: Removed theTokensstruct and introduced new modules (token2entityandtypes) related to authorization functionality.jans-lock/cedarling/src/authz/types.rs: Defined new structs for deserialization of various token types, such as access tokens, transaction tokens, and ID tokens.jans-lock/cedarling/src/crypto/decode.rs: Implemented the logic for validating JSON Web Tokens (JWT) using a trust store.jans-lock/cedarling/src/crypto/mod.rs: Initialized the trust store for verifying JWT signatures from trusted issuers.jans-lock/cedarling/src/crypto/types.rs: Defined a newTrustedIssuerstruct to represent a trusted identity provider.jans-lock/cedarling/src/http.rs: Made minor changes to thepostfunction, which is responsible for making HTTP POST requests.jans-lock/cedarling/src/lib.rs: Refactored the startup sequence, including the initialization of the authorization module.jans-lock/cedarling/src/lock_master/sse.rs: Implemented the handling of Server-Sent Events (SSE) updates from the lock master service.jans-lock/cedarling/src/lock_master/mod.rs: Initialized the lock master component, including the dynamic client registration and access token acquisition.- `jans-lock
Powered by DryRun Security
@kdhttps while I do understand the importance of Clean Architecture and writing sufficient tests, can we get something that works first then we can start the polish work? I'll definitely do it, I just don't want to get bogged down in structural work right now
@SafinWasi https://lock.master.gluu.cloud doesn't seem to point to anything, so I can't get the relevant OpenId config, and thus can't get the supported scopes. I might resolve to reading through the source code, but I'm not sure if that's a forward-looking solution.
On the matter of writing the README, I'm restoring my Typora license then I'll get to it.
@kdhttps while I do understand the importance of Clean Architecture and writing sufficient tests, can we get something that works first then we can start the polish work? I'll definitely do it, I just don't want to get bogged down in structural work right now
I'm sorry, but I must respectfully disagree with you. At least we should write unit test cases for each function.
@kdhttps, can we agree then that the tests are not final and are written to fail.
@kdhttps, can we agree then that the tests are not final and are written to fail.
agreed! it is ok if tests fail but we will get an idea of the module what it's accepting, and rejecting, and what the final result should look like.
@sokorototo you can check out the cedar-wasm code, it is a good example of TDD. It has small functions, accepting everything in parameters and respecting true/false test cases. The goal is code should look and execute health(bug free) and it is only possible by adding test cases.
Looks good to me. @kdhttps can you check functionality and JS bindings?
@sokorototo I am getting this on my HTML page.
My JS Code looks like:
<script type="module">
import init from "./out/cedarling.js";
const config = {
policyStore: {
// can be "local", "remote" or "lock-master",
// each strategy requires different parameters, see below
strategy: "remote",
url: "https://raw.githubusercontent.com/JanssenProject/jans/main/jans-lock/cedarling/policy-store/default.json",
},
// if policy-store.json is compressed using deflate-zlib
decompressPolicyStore: false,
// [OPTIONAL] How often, in milliseconds, will the cedarling refresh it's TrustStore. The cedarling won't refresh it's trust store if not omitted
trustStoreRefreshRate: 2000,
// Set of jwt algorithms that the cedarling will allow, empty if omitted
supportedAlgorithms: ["HS256", "HS384", "RS256"],
};
async function run() {
await init(config);
}
run();
</script>
@kdhttps to build for the Web, you use wasm-bindgen --out-dir out ./target/wasm32-unknown-unknown/release/cedarling.wasm --target web. I was testing with node, so I forgot to include this, let me patch it in. Building for the Web is somewhat complicated, let me add further instructions.
ok, thanks but now getting this @sokorototo .
@kdhttps can you check if your dev server is setup correctly? Is the wasm binary being loaded in the Network Section of the dev tools? Since everything seems to load on my side
I investigate the cedarling.js. It is not invoking the wasm binary load function. so I manually added it to my HTML page. Not sure if this way is right or wrong. My HTML JS code is:
<script type="module">
import __wbg_init, { init } from "./out/cedarling.js";
const config = {
policyStore: {
strategy: "remote",
url: "https://raw.githubusercontent.com/kdhttps/cedar-policies/main/example-policy-store.json",
},
decompressPolicyStore: false,
trustStoreRefreshRate: 2000,
supportedAlgorithms: ["HS256", "HS384", "RS256"],
policyStoreId: "gluu_store",
};
async function run() {
await __wbg_init();
await init(config);
}
run();
</script>
await __wbg_init(); after manually invoking the wasm binary I can now see it in the network tab. and now looks like it is actually calling the wasm init function but getting an error:
DryRun Security Summary
The pull request includes various code changes to the Jans Cedarling application, focusing on JWT handling, policy store management, authorization, and HTTP request handling, with a focus on ensuring secure implementation and mitigating potential security vulnerabilities.
Expand for full summary
Summary:
The code changes in this pull request are focused on various aspects of the Jans Cedarling application, including updates to the .gitignore file, the Cargo.toml file, the README.md, and several source code files. The changes cover a wide range of functionality, such as JWT handling, policy store management, authorization, and HTTP request handling.
From an application security perspective, the key areas to focus on are:
-
JWT Handling: The addition of the
jsonwebtokendependency and the changes in thecryptomodule suggest that the application is handling JSON Web Tokens (JWTs) for authentication and authorization purposes. It is crucial to ensure that the JWT handling is implemented securely, with proper validation, verification, and protection against common JWT-related vulnerabilities. -
Policy Store Management: The changes in the
startupandlock_mastermodules indicate that the application is using different policy store configurations (local, remote, and lock-master). Proper validation and secure handling of the policy store data are essential to prevent unauthorized access or tampering. -
Authorization and Access Control: The changes in the
authzmodule suggest that the application has some form of authorization and access control mechanisms. These mechanisms should be thoroughly reviewed to ensure that they are implemented securely and follow the principles of least privilege and defense-in-depth. -
Input Validation and Sanitization: The application is deserializing various data structures, including those containing sensitive information (e.g., tokens, configuration parameters). Proper input validation and sanitization should be implemented to mitigate potential vulnerabilities, such as injection attacks or deserialization issues.
-
Secure Configuration Management: The application's configuration, including sensitive values like URIs, client credentials, and supported cryptographic algorithms, should be managed securely to prevent unauthorized access or modifications.
Overall, the changes in this pull request introduce several security-relevant components and functionality. As an application security engineer, it is crucial to review the implementation details, ensure that security best practices are followed, and verify that the application is not introducing any new vulnerabilities or security risks.
Files Changed:
.gitignore: The changes add a new line to ignore the "/out" directory, which is a common practice to avoid accidentally committing generated artifacts.Cargo.toml: The changes update dependencies, including the addition of thejsonwebtokencrate, which is used for handling JSON Web Tokens (JWTs).README.md: The changes provide more detailed information about the Cedarling application, including configuration options, trust store management, and integration with the Jans Lock Master.src/authz/mod.rs: The changes refactor theauthzmodule, including the removal of theTokensstruct and the introduction of theAuthzInputstruct for handling authorization-related input data.src/authz/types.rs: The changes introduce new data structures for handling various types of tokens and input data related to authorization, which should be reviewed for proper input validation and secure handling of sensitive information.src/crypto/decode.rs: The changes focus on the implementation of JWT validation, including the extraction of the JSON Web Key (JWK) and the creation of theDecodingKeyandValidationobjects.src/crypto/mod.rs: The changes introduce theSUPPORTED_ALGORITHMSstatic variable and theTrustedIssuerdata structure, which are used for verifying the authenticity of JWT tokens.src/crypto/types.rs: The changes add theTrustedIssuerdata structure, which is used to represent trusted OpenID Connect (OIDC) identity providers.src/http.rs: The changes update the HTTP request method and provide options for handling different types of request bodies, which should be reviewed for proper input validation and sanitization.src/lock_master/mod.rs: The changes focus on the implementation of the dynamic client registration and token acquisition functionality, which should be reviewed for secure handling of sensitive information, such as client credentials.src/lib.rs: The changes are related to the initialization and startup of the Cedarling application, including the handling of unexpected panics and the initialization of theauthzmodule.src/startup/mod.rs: The changes simplify the policy store initialization process and remove the handling of trusted issuers, which may have security implications that should be reviewed.src/lock_master/sse.rs: The changes introduce the
Code Analysis
We ran 7 analyzers against 22 files and 0 analyzers had findings. 7 analyzers had no findings.
Riskiness
:green_circle: Risk threshold not exceeded.