Skip to content

Okta mtls implementation#384

Open
Aaditesh2307 wants to merge 5 commits into
wootzapp:chromiumfrom
Aaditesh2307:okta-mtls-implementation
Open

Okta mtls implementation#384
Aaditesh2307 wants to merge 5 commits into
wootzapp:chromiumfrom
Aaditesh2307:okta-mtls-implementation

Conversation

@Aaditesh2307
Copy link
Copy Markdown
Contributor

@Aaditesh2307 Aaditesh2307 commented Oct 17, 2025

User description

🔐 Feature: mTLS Integration for Okta Login via Network Layer

🧩 Summary

This PR introduces mutual TLS (mTLS) support for Okta login flows on the WootzApp backend.
It enhances authentication security by validating client certificates before allowing access to Okta SSO endpoints, ensuring that only authorized WootzApp browsers can initiate or complete authentication requests.


🏗️ Implementation Details

1. Network Layer Integration

  • Integrated mTLS enforcement directly at the Chromium network layer to handle okta.com/app login URLs.
  • Added logic to intercept all outgoing network requests and:
    • Detect if the request URL matches an Okta login pattern.
    • Trigger certificate retrieval flow before the request proceeds.

PR Type

Enhancement


Description

  • Implements mTLS certificate gating for Okta app access via network throttle

  • Adds extension API endpoint to receive and validate mTLS certificates

  • Integrates certificate manager with time-based validation (10-minute window)

  • Updates device enrollment URLs and adds certificate validity parameter

  • Changes host validation from eb.wootzapp.com to trust.wootzapp.com


Diagram Walkthrough

flowchart LR
  A["Extension API<br/>chrome.wootz.mtlsCert"] -->|Certificate String| B["WootzMtlsCertFunction"]
  B -->|Store & Validate| C["OktaCertificateManager"]
  C -->|Static Storage| D["Stored Certificate<br/>10-min window"]
  E["Network Request<br/>to Okta"] -->|Intercept| F["OktaAppGateThrottle"]
  F -->|Check Certificate| D
  D -->|Valid| G["Allow Access"]
  D -->|Invalid| H["Block & Show Error"]
  I["eb.wootzapp.com Visit"] -->|Trigger| A
Loading

File Walkthrough

Relevant files
Enhancement
12 files
chrome_content_browser_client.cc
Add Okta throttle and update host validation                         
+6/-1     
wootz_api.cc
Implement mtlsCert extension API function                               
+49/-0   
okta_app_gate_throttle.cc
Create URL loader throttle for Okta access gating               
+151/-0 
okta_certificate_manager.cc
Implement certificate storage and validation logic             
+232/-0 
okta_gate_util.cc
Add URL matching utilities for Okta and eb.wootzapp.com   
+18/-0   
WootzEnrollmentUtils.java
Add certificate validity parameter to enrollment request 
+3/-2     
wootz_api.h
Add WootzMtlsCertFunction class declaration                           
+9/-0     
okta_app_gate_throttle.h
Define OktaAppGateThrottle throttle interface                       
+66/-0   
okta_certificate_manager.h
Define certificate manager interface and storage                 
+61/-0   
okta_gate_util.h
Define URL matching utility functions                                       
+17/-0   
extension_function_histogram_value.h
Add histogram value for mtlsCert function                               
+1/-0     
wootz.json
Add mtlsCert API definition to extension manifest               
+26/-0   
Configuration changes
2 files
WootzDeviceEnrollment.java
Update enrollment URLs and add bearer token placeholder   
+3/-2     
BUILD.gn
Add new Okta mTLS source files to build                                   
+6/-0     

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented Oct 17, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🔴
Sensitive information exposure

Description: Certificate contents are logged including a preview of up to 100 characters, potentially
exposing sensitive certificate data in logs.
wootz_api.cc [1934-1941]

Referred Code
// Log the certificate string in Android browser
LOG(INFO) << "Aaditesh_mtls -> MTLS_CERT API: Received certificate string "
             "(length: "
          << cert_string.length() << ")";
LOG(INFO) << "Aaditesh_mtls -> MTLS_CERT API: Certificate preview (first "
             "100 chars): "
          << cert_string.substr(0, std::min<size_t>(100, cert_string.length()));
Insufficient certificate validation

Description: Certificate validation intentionally omits chain/root verification and relies only on time
validity, allowing untrusted/self-signed certificates to be accepted if time-valid.
okta_certificate_manager.cc [42-47]

Referred Code
// Root CA validation removed - certificate time validity is sufficient

OktaCertificateManager::OktaCertificateManager(content::BrowserContext* context)
    : context_(context) {
  LOG(INFO) << "Aaditesh_mtls -> OktaCertificateManager initialized (using "
               "extension API for certificate)";
Hardcoded credentials

Description: Hardcoded production bearer token placeholder suggests potential risk of committing
secrets; if real tokens are used this becomes credential exposure.
WootzDeviceEnrollment.java [33-37]

Referred Code
private static final String NONCE_URL = "https://app.wootzapp.com/api/csr/nonce"; // e.g.
private static final String ENROLLMENT_URL = "https://app.wootzapp.com/api/csr/sign"; // e.g.
// "https://app.woozapp.com/api/v1/device/enroll"
private static final String BEARER_TOKEN = "PROD_BEARER_TOKEN";
Insecure certificate storage

Description: Stored certificate and timestamp are kept in static process-wide variables without
persistence controls or per-profile isolation, enabling cross-profile leakage or misuse.
okta_certificate_manager.cc [37-41]

Referred Code
// Static storage for certificate persistence across instances
scoped_refptr<net::X509Certificate>
    OktaCertificateManager::stored_certificate_ = nullptr;
base::Time OktaCertificateManager::certificate_stored_time_ = base::Time();
Incomplete URL gating criteria

Description: Okta URL detection is limited to DomainIs("okta.com") with path starting "/app/", missing
regional domains (e.g., okta-emea.com) and other Okta SSO paths, enabling bypass.
okta_gate_util.cc [10-14]

Referred Code
bool IsOktaAppUrl(const GURL& url) {
  return url.SchemeIsCryptographic() &&
         url.DomainIs("okta.com") &&  // matches *.okta.com
         base::StartsWith(url.path_piece(), "/app/");
}
Access control bypass risk

Description: The gating logic unconditionally allows access to eb.wootzapp.com and only checks for cert
presence on Okta domains, which may be bypassed if Okta auth flows use alternate domains
or non-/app paths.
okta_app_gate_throttle.cc [36-51]

Referred Code
if (IsFacebookUrl(request->url)) {
  LOG(INFO) << "Aaditesh_mtls -> eb.wootzapp.com URL detected: "
            << request->url;
  LOG(INFO) << "Aaditesh_mtls -> Certificate provisioning via extension API (chrome.wootz.mtlsCert)";

  // Check if we already have a valid certificate
  if (certificate_manager_->HasValidStoredCertificate()) {
    LOG(INFO) << "Aaditesh_mtls -> Valid certificate already exists, "
                 "allowing eb.wootzapp.com access";
  } else {
    LOG(INFO) << "Aaditesh_mtls -> No certificate yet - extension should call chrome.wootz.mtlsCert";
  }

  // Always allow eb.wootzapp.com access (certificate comes from extension API)
  return;
}
Ticket Compliance
🎫 No ticket provided
- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->

</details></td></tr>
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented Oct 17, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Security
Isolate certificate storage per profile

Refactor OktaCertificateManager to store certificates on a per-profile basis
instead of using static variables. This can be achieved by converting it to a
BrowserContextKeyedService to prevent certificate data from being shared between
different user profiles.

src/chrome/browser/net/okta_certificate_manager.cc [37-40]

-// Static storage for certificate persistence across instances
-scoped_refptr<net::X509Certificate>
-    OktaCertificateManager::stored_certificate_ = nullptr;
-base::Time OktaCertificateManager::certificate_stored_time_ = base::Time();
+// As a full BrowserContextKeyedService is a larger change, here's an
+// alternative using a map to store data per profile.
+// Note: This requires careful memory management to avoid leaks.
 
+namespace {
+
+struct CertificateData {
+  scoped_refptr<net::X509Certificate> certificate;
+  base::Time stored_time;
+};
+
+// Per-profile storage for the certificate data.
+// WARNING: This map can grow indefinitely if not cleaned up when
+// BrowserContexts are destroyed. A BrowserContextKeyedService is the
+// robust solution.
+std::map<content::BrowserContext*, CertificateData>& GetCertificateStorage() {
+  static base::NoDestructor<std::map<content::BrowserContext*, CertificateData>>
+      storage;
+  return *storage;
+}
+
+} // namespace
+
+// Then, in your class methods, access data via the context:
+// Example for StoreCertificate:
+// bool OktaCertificateManager::StoreCertificate(const std::string& cert_pem) {
+//   ...
+//   auto& storage = GetCertificateStorage();
+//   storage[context_].certificate = certs[0];
+//   storage[context_].stored_time = base::Time::Now();
+//   ...
+// }
+

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 10

__

Why: The suggestion correctly identifies a critical security vulnerability where a static certificate is shared across all browser profiles and proposes the standard Chromium pattern to fix it.

High
Possible issue
Use a persistent per-profile service

Refactor the code to retrieve a persistent, per-profile instance of
OktaCertificateManager (e.g., as a BrowserContextKeyedService) instead of
creating a new local instance in WootzMtlsCertFunction::Run.

src/chrome/browser/extensions/api/wootz/wootz_api.cc [1952-1955]

-// Create certificate manager instance and store certificate
-auto certificate_manager =
-    std::make_unique<OktaCertificateManager>(context);
+// Assuming OktaCertificateManager is converted to a BrowserContextKeyedService
+// with a corresponding OktaCertificateManagerFactory.
+
+// Get the per-profile instance of the certificate manager
+OktaCertificateManager* certificate_manager =
+    OktaCertificateManagerFactory::GetForBrowserContext(context);
+
+// Store certificate
 bool success = certificate_manager->StoreAndValidateCertificate(cert_string);
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that instantiating OktaCertificateManager as a local variable is fragile and will break once the underlying static storage (a security flaw) is fixed. It correctly proposes using a persistent, per-profile service.

High
Fix inconsistent URL and misleading name

Rename the misleading function IsFacebookUrl to IsWootzAppCertUrl and update its
implementation to check for trust.wootzapp.com to ensure consistency with other
URL checks in the PR.

src/chrome/browser/net/okta_gate_util.cc [16-18]

-bool IsFacebookUrl(const GURL& url) {
-  return url.SchemeIsCryptographic() && url.DomainIs("eb.wootzapp.com");
+bool IsWootzAppCertUrl(const GURL& url) {
+  return url.SchemeIsCryptographic() && url.DomainIs("trust.wootzapp.com");
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a misleading function name and an inconsistency in the URLs used across the PR, which could lead to incorrect behavior. Fixing this improves both correctness and maintainability.

Medium
  • Update

Removed detailed logging and custom error message for access denied events in OktaAppGateThrottle.
Removed logging statements related to certificate storage and validation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant