You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
🔐 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.
The implementation should be refactored to remove hardcoded secrets, such as the bearer token, test certificate, and user identity. These values must be provisioned securely at runtime instead of being embedded in the source code.
if (!X509_NAME_add_entry_by_txt(
name.get(), "CN", MBSTRING_ASC,
reinterpret_cast<constunsignedchar*>("adnan-user-123"), -1, -1,
0) ||
Solution Walkthrough:
Before:
// src/chrome/browser/net/okta_certificate_manager.cc
std::string OktaCertificateManager::GenerateCSR() {
// ...// Hardcoded user identity for CSRX509_NAME_add_entry_by_txt(name.get(), "CN", ..., "adnan-user-123", ...);
// ...
}
voidOktaCertificateManager::MakeCSRSigningRequest(...) {
// ...// Hardcoded bearer token for API request
resource_request->headers.SetHeader("Authorization", "Bearer Aoi3dkgpE905nvSiec");
// ...
}
voidOktaCertificateManager::OnCSRSigningResponse(...) {
if (url_loader_->NetError() == net::ERR_SSL_CLIENT_AUTH_CERT_NEEDED) {
// Fallback to hardcoded test certificatestd::move(callback).Run(true, kTestCertificatePEM);
return;
}
}
After:
// src/chrome/browser/net/okta_certificate_manager.cc
std::string OktaCertificateManager::GenerateCSR() {
// ...// User identity should be retrieved dynamically (e.g., from profile)
std::string user_id = GetCurrentUserId(context_);
X509_NAME_add_entry_by_txt(name.get(), "CN", ..., user_id.c_str(), ...);
// ...
}
voidOktaCertificateManager::MakeCSRSigningRequest(...) {
// ...// Token should be fetched from a secure storage or configuration service
std::string token = GetApiAuthToken();
resource_request->headers.SetHeader("Authorization", "Bearer " + token);
// ...
}
voidOktaCertificateManager::OnCSRSigningResponse(...) {
if (url_loader_->NetError() == net::ERR_SSL_CLIENT_AUTH_CERT_NEEDED) {
// Handle bootstrap failure properly without using hardcoded certsLOG(ERROR) << "Bootstrap failed: server requires a client cert.";
std::move(callback).Run(false, "");
return;
}
}
Suggestion importance[1-10]: 10
__
Why: The suggestion correctly identifies critical security vulnerabilities, including a hardcoded bearer token (Bearer Aoi3dkgpE905nvSiec) and user identity (adnan-user-123), which renders the feature insecure and unscalable.
Convert the static certificate storage variables to non-static members to prevent a critical security flaw where certificate data is shared across different user profiles.
-// Static storage for certificate persistence across instances-scoped_refptr<net::X509Certificate>- OktaCertificateManager::stored_certificate_ = nullptr;-base::Time OktaCertificateManager::certificate_stored_time_ = base::Time();+// Certificate storage per instance+scoped_refptr<net::X509Certificate> stored_certificate_ = nullptr;+base::Time certificate_stored_time_ = base::Time();
Apply / Chat
Suggestion importance[1-10]: 10
__
Why: The suggestion correctly identifies a critical security and privacy vulnerability where static variables cause certificate data to be shared across different user profiles, and the proposed fix is appropriate.
High
Implement proper certificate chain validation
Replace the insecure, hardcoded string check in ValidateAgainstRootCA with proper cryptographic certificate chain validation to fix a critical security vulnerability.
bool OktaCertificateManager::ValidateAgainstRootCA(
scoped_refptr<net::X509Certificate> cert) {
- LOG(INFO)- << "Aaditesh_mtls -> Validating certificate against Root CA (TEST MODE)";+ LOG(INFO) << "Aaditesh_mtls -> Validating certificate against Root CA";- // For testing purposes, we'll accept the hardcoded certificate as valid- // Check if this is our expected test certificate by comparing subject- std::string cert_subject = cert->subject().GetDisplayName();- LOG(INFO) << "Aaditesh_mtls -> Certificate subject: " << cert_subject;+ scoped_refptr<net::X509Certificate> root_ca_cert = LoadRootCACertificate();+ if (!root_ca_cert) {+ LOG(ERROR) << "Aaditesh_mtls -> Could not load Root CA for validation.";+ return false;+ }- // Accept certificates that contain our expected test subject components- bool is_test_cert =- (cert_subject.find("adnan-user-123") != std::string::npos);+ // Create a temporary trust store with only our root CA.+ scoped_refptr<net::CertVerifyProc> verify_proc =+ net::CertVerifyProc::CreateSystemVerifyProc(nullptr, nullptr);+ net::CertVerifyResult result;+ int flags = net::CertVerifier::VERIFY_DISABLE_NETWORK_FETCHES;- LOG(INFO) << "Aaditesh_mtls -> Is test certificate: "- << (is_test_cert ? "YES" : "NO");- LOG(INFO) << "Aaditesh_mtls -> Root CA validation: "- << (is_test_cert ? "VALID" : "INVALID");+ // The hostname does not matter here as we are only checking the trust anchor.+ int error = verify_proc->Verify(+ cert.get(), "eb.wootzapp.com", /*ocsp_response=*/std::string(),+ /*sct_list=*/std::string(), flags, &result, net::NetLogWithSource());- return is_test_cert;+ if (error != net::OK) {+ LOG(ERROR) << "Aaditesh_mtls -> Certificate validation failed with error: "+ << net::ErrorToString(error);+ return false;+ }++ // Check if the certificate chains up to our trusted root.+ if (!result.verified_cert ||+ !result.verified_cert->IsIssuedBy(root_ca_cert.get())) {+ LOG(ERROR) << "Aaditesh_mtls -> Certificate was not issued by the expected Root CA.";+ return false;+ }++ LOG(INFO) << "Aaditesh_mtls -> Certificate successfully validated against Root CA.";+ return true;
}
Apply / Chat
Suggestion importance[1-10]: 10
__
Why: The suggestion correctly identifies a critical security vulnerability where certificate validation is bypassed by a simple string check, and it proposes a correct implementation using proper cryptographic verification.
High
✅ Remove hardcoded authentication tokenSuggestion Impact:The commit removed the code path that made the CSR signing request which included the hardcoded Authorization: Bearer token, effectively eliminating the hardcoded token usage from the file.
code diff:
-void OktaCertificateManager::MakeCSRSigningRequest(- const std::string& csr,- CertificateCallback callback) {- LOG(INFO) << "Aaditesh_mtls -> Making CSR signing API request (New API "- "Documentation Format)";-- // Create resource request- auto resource_request = std::make_unique<network::ResourceRequest>();- resource_request->url = GURL("https://app.wootzapp.com/api/csr/sign");- resource_request->method = "POST";- resource_request->load_flags = net::LOAD_DISABLE_CACHE |- net::LOAD_DISABLE_CERT_NETWORK_FETCHES |- net::LOAD_SHOULD_BYPASS_HSTS;-- // Skip certificate validation for this bootstrap request- resource_request->credentials_mode = network::mojom::CredentialsMode::kOmit;-- // Configure trusted params for this request\n- // resource_request->trusted_params =- // network::ResourceRequest::TrustedParams();-- // Additional SSL bypass settings- resource_request->request_initiator = url::Origin();- resource_request->site_for_cookies = net::SiteForCookies();-- // Add headers- resource_request->headers.SetHeader("Content-Type", "application/json");- resource_request->headers.SetHeader("Authorization",- "Bearer Aoi3dkgpE905nvSiec");-- LOG(INFO) << "Aaditesh_mtls -> Headers set:";- LOG(INFO) << "Aaditesh_mtls -> Content-Type: application/json";- LOG(INFO) << "Aaditesh_mtls -> Authorization: Bearer Aoi3dkgpE905nvSiec";-- // Create JSON request body- base::Value::Dict request_dict;-- // Clean up CSR (remove extra whitespace/newlines if needed)- std::string clean_csr = csr;- base::ReplaceChars(clean_csr, "\r", "", &clean_csr);-- request_dict.Set("csr", clean_csr);-- // Verify CSR format matches working curl command- LOG(INFO) << "Aaditesh_mtls -> CSR for API call (first 100 chars): "- << clean_csr.substr(0, 100);- LOG(INFO) << "Aaditesh_mtls -> CSR ends with (last 50 chars): "- << clean_csr.substr(std::max(0, (int)clean_csr.length() - 50));- LOG(INFO) << "Aaditesh_mtls -> Total CSR length: " << clean_csr.length();-- // Verify it starts and ends correctly- bool starts_correct =- clean_csr.find("-----BEGIN CERTIFICATE REQUEST-----") == 0;- bool ends_correct =- clean_csr.find("-----END CERTIFICATE REQUEST-----") != std::string::npos;- LOG(INFO) << "Aaditesh_mtls -> CSR format check - Starts correctly: "- << (starts_correct ? "YES" : "NO");- LOG(INFO) << "Aaditesh_mtls -> CSR format check - Ends correctly: "- << (ends_correct ? "YES" : "NO");-- std::string request_body;- if (!base::JSONWriter::Write(request_dict, &request_body)) {- LOG(ERROR) << "Aaditesh_mtls -> Failed to create JSON request body";- std::move(callback).Run(false, "");- return;- }-- LOG(INFO) << "Aaditesh_mtls -> Request body length: "- << request_body.length();- LOG(INFO) << "Aaditesh_mtls -> Request body: " << request_body;- LOG(INFO) << "Aaditesh_mtls -> Request URL: " << resource_request->url.spec();- LOG(INFO) << "Aaditesh_mtls -> Request method: " << resource_request->method;- LOG(INFO) << "Aaditesh_mtls -> UPDATED: Using app.wootzapp.com domain (not "- "eb.wootzapp.com)";-- // Create traffic annotation- net::NetworkTrafficAnnotationTag traffic_annotation =- net::DefineNetworkTrafficAnnotation("okta_csr_signing", R"(- semantics {- sender: "Okta Certificate Manager"- description: "Request to sign a Certificate Signing Request (CSR) for Okta authentication"- trigger: "User navigates to Okta app URLs"- data: "Certificate Signing Request in PEM format"- destination: WEBSITE- }- policy {- cookies_allowed: NO- setting: "This feature cannot be disabled"- })");-- // Create URL loader with SSL bypass configuration- url_loader_ = network::SimpleURLLoader::Create(std::move(resource_request),- traffic_annotation);-- // Attach the JSON body containing the CSR to the POST request- url_loader_->AttachStringForUpload(request_body, "application/json");-- LOG(INFO) << "Aaditesh_mtls -> CSR data attached to POST request body";-- // Configure URL loader to allow HTTP errors and disable retries- url_loader_->SetAllowHttpErrorResults(true);- url_loader_->SetRetryOptions(0, network::SimpleURLLoader::RETRY_NEVER);-- // Get URL loader factory- auto* storage_partition = context_->GetDefaultStoragePartition();- auto* url_loader_factory =- storage_partition->GetURLLoaderFactoryForBrowserProcess().get();-- // Start the request- LOG(INFO) << "Aaditesh_mtls -> Starting URL loader request with SSL bypass "- "configured";- LOG(INFO) << "Aaditesh_mtls -> Note: This is a bootstrap request to GET our "- "first certificate";- url_loader_->DownloadToStringOfUnboundedSizeUntilCrashAndDie(- url_loader_factory,- base::BindOnce(&OktaCertificateManager::OnCSRSigningResponse,- base::Unretained(this), std::move(callback)));-}--void OktaCertificateManager::OnCSRSigningResponse(- CertificateCallback callback,- std::unique_ptr<std::string> response_body) {- LOG(INFO) << "Aaditesh_mtls -> Received CSR signing response";-- // Check for network errors first- if (url_loader_->NetError() != net::OK) {- LOG(ERROR) << "Aaditesh_mtls -> Network error: " << url_loader_->NetError();- LOG(ERROR) << "Aaditesh_mtls -> Network error string: "- << net::ErrorToString(url_loader_->NetError());-- // Special handling for SSL client certificate errors\n if- // (url_loader_->NetError() == net::ERR_SSL_CLIENT_AUTH_CERT_NEEDED) {\n- // LOG(ERROR) << \"Aaditesh_mtls -> CRITICAL: Server requires client- // certificate but we don't have one yet!\";\n LOG(ERROR) <<- // \"Aaditesh_mtls -> This is a bootstrap certificate request - server- // should not require client certs\";\n LOG(ERROR) << \"Aaditesh_mtls- // -> API endpoint: https://eb.wootzapp.com/api/csr/sign\";\n LOG(ERROR) <<- // \"Aaditesh_mtls -> Bearer token: Aoi3dkgpE905nvSiec\";\n LOG(ERROR)- // << \"Aaditesh_mtls -> FALLBACK: Using hardcoded test certificate for- // initial access\";\n \n // As a fallback, use the hardcoded test- // certificate since we have a chicken-and-egg problem\n- // std::move(callback).Run(true, kTestCertificatePEM);\n return;\n }-- // Special handling for SSL client certificate errors- if (url_loader_->NetError() == net::ERR_SSL_CLIENT_AUTH_CERT_NEEDED) {- LOG(ERROR) << "Aaditesh_mtls -> CRITICAL: Server requires client "- "certificate but we don't have one yet!";- LOG(ERROR) << "Aaditesh_mtls -> This is a bootstrap certificate request "- "- server should not require client certs";- LOG(ERROR) << "Aaditesh_mtls -> API endpoint: "- "https://app.wootzapp.com/api/csr/sign";- LOG(ERROR) << "Aaditesh_mtls -> Bearer token: Aoi3dkgpE905nvSiec";- LOG(ERROR) << "Aaditesh_mtls -> FALLBACK: Using hardcoded test "- "certificate for initial access";-- // As a fallback, use the hardcoded test certificate since we have a- // chicken-and-egg problem- std::move(callback).Run(true, kTestCertificatePEM);- return;- }-- std::move(callback).Run(false, "");- return;- }-- // First check HTTP status code regardless of response body- int response_code =- url_loader_->ResponseInfo()- ? url_loader_->ResponseInfo()->headers->response_code()- : 0;- LOG(INFO) << "Aaditesh_mtls -> HTTP response code: " << response_code;-- // Log response headers for debugging- if (url_loader_->ResponseInfo() && url_loader_->ResponseInfo()->headers) {- scoped_refptr<net::HttpResponseHeaders> headers =- url_loader_->ResponseInfo()->headers;- LOG(INFO) << "Aaditesh_mtls -> Response headers:";- size_t iter = 0;- std::string name, value;- while (headers->EnumerateHeaderLines(&iter, &name, &value)) {- LOG(INFO) << "Aaditesh_mtls -> " << name << ": " << value;- }- }-- if (!response_body) {- LOG(ERROR) << "Aaditesh_mtls -> Empty response body (nullptr)";- LOG(ERROR) << "Aaditesh_mtls -> This suggests the network request failed "- "completely";- std::move(callback).Run(false, "");- return;- }-- if (response_body->empty()) {- LOG(ERROR) << "Aaditesh_mtls -> Empty response body (zero length)";- LOG(ERROR) << "Aaditesh_mtls -> Server returned no content but request "- "succeeded at network level";- std::move(callback).Run(false, "");- return;- }-- LOG(INFO) << "Aaditesh_mtls -> Response body length: "- << response_body->length();- LOG(INFO) << "Aaditesh_mtls -> Full API Response body: " << *response_body;-- if (response_code != net::HTTP_OK) {- LOG(ERROR) << "Aaditesh_mtls -> API request failed with status: "- << response_code;- LOG(ERROR) << "Aaditesh_mtls -> Response body: " << *response_body;- std::move(callback).Run(false, "");- return;- }-- LOG(INFO) << "Aaditesh_mtls -> Response length: " << response_body->length();-- // Parse JSON response- auto parsed_json =- base::JSONReader::ReadAndReturnValueWithError(*response_body);- if (!parsed_json.has_value()) {- LOG(ERROR) << "Aaditesh_mtls -> Failed to parse JSON response: "- << parsed_json.error().message;- std::move(callback).Run(false, "");- return;- }-- const base::Value::Dict* response_dict = parsed_json->GetIfDict();- if (!response_dict) {- LOG(ERROR) << "Aaditesh_mtls -> Response is not a JSON object";- std::move(callback).Run(false, "");- return;- }-- // According to new API documentation, certificate is nested in certificate- // object- LOG(INFO) << "Aaditesh_mtls -> Parsing response using NEW API format: "- "certificate.certificatePem";- const base::Value::Dict* certificate_obj =- response_dict->FindDict("certificate");- if (!certificate_obj) {- LOG(ERROR) << "Aaditesh_mtls -> Certificate object not found in response";- LOG(ERROR) << "Aaditesh_mtls -> Expected format: {\"certificate\": "- "{\"certificatePem\": \"...\"}}";- std::move(callback).Run(false, "");- return;- }-- const std::string* certificate_pem =- certificate_obj->FindString("certificatePem");- if (!certificate_pem) {- LOG(ERROR) << "Aaditesh_mtls -> certificatePem field not found in "- "certificate object";- LOG(ERROR) << "Aaditesh_mtls -> Expected format: {\"certificate\": "- "{\"certificatePem\": \"...\"}}";- std::move(callback).Run(false, "");- return;- }-- LOG(INFO)- << "Aaditesh_mtls -> Extracted certificate from API response, length: "- << certificate_pem->length();-- // Store and validate the certificate- bool success = StoreCertificate(*certificate_pem);- if (success) {- LOG(INFO)- << "Aaditesh_mtls -> Certificate stored and validated successfully";- } else {- LOG(ERROR) << "Aaditesh_mtls -> Certificate validation failed";- }-- std::move(callback).Run(success, *certificate_pem);
}
Remove the hardcoded bearer token from the source code to fix a critical security vulnerability, and instead, fetch it from a secure location at runtime.
[To ensure code accuracy, apply this suggestion manually]
Suggestion importance[1-10]: 10
__
Why: The suggestion correctly identifies a critical security vulnerability due to a hardcoded bearer token and recommends the standard security practice of fetching credentials from a secure storage at runtime.
High
Possible issue
Rename function for clarity and correctness
Rename the function IsFacebookUrl to better reflect its purpose of checking for eb.wootzapp.com for certificate provisioning, as indicated by the accompanying comment.
// Determines if the given URL is eb.wootzapp.com (triggers certificate
// provisioning)
-bool IsFacebookUrl(const GURL& url);+bool IsCertProvisioningUrl(const GURL& url);
Apply / Chat
Suggestion importance[1-10]: 7
__
Why: The suggestion correctly identifies a misleading function name IsFacebookUrl that contradicts its documented purpose, which significantly improves code clarity and prevents future bugs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
okta.com/applogin URLs.