Skip to content

Conversation

markphelps
Copy link
Contributor

@markphelps markphelps commented Jun 27, 2025

Summary

Implements Phase 1 of self-signed certificate support by adding comprehensive TLS configuration to the FFI engine. This enables FFI-based client SDKs (Python, Java, C#, Ruby, Dart) to connect to Flipt servers using self-signed certificates.

Changes

Core Implementation

  • New TlsConfig struct in EngineOpts with support for:
    • ca_cert_file / ca_cert_data - Custom CA certificates (file path or base64 encoded)
    • insecure_skip_verify - Skip certificate verification (development only)
    • client_cert_file/data + client_key_file/data - Client certificates for mutual TLS

HTTP Client Integration

  • Added tls_config() method to HTTPFetcherBuilder
  • Implemented configure_tls() function that configures reqwest::Client with custom TLS settings
  • Proper error handling with descriptive error messages

Testing

  • 13 comprehensive unit tests covering:
    • All TLS configuration combinations
    • Error handling for invalid certificates/files
    • Both file-based and data-based certificate loading
    • EngineOpts JSON serialization/deserialization

Usage Example

FFI clients can now pass TLS configuration via JSON:

{
  "url": "https://localhost:8443",
  "tls_config": {
    "ca_cert_file": "/path/to/ca.crt",
    "insecure_skip_verify": false
  }
}

Or with base64-encoded certificate data:

{
  "url": "https://localhost:8443", 
  "tls_config": {
    "ca_cert_data": "LS0tLS1CRUdJTi...",
    "client_cert_data": "LS0tLS1CRUdJTi...",
    "client_key_data": "LS0tLS1CRUdJTi..."
  }
}

Security Considerations

  • Secure by default: All TLS options are optional, existing behavior unchanged
  • Development safety: insecure_skip_verify requires explicit opt-in
  • Clear error messages: Detailed feedback for certificate configuration issues
  • Proper certificate validation: Uses reqwest's built-in certificate handling

Testing

All tests pass including new TLS-specific tests:

  • ✅ Custom CA certificate loading (file and data)
  • ✅ Client certificate authentication
  • ✅ Insecure mode for development
  • ✅ Error handling for invalid configurations
  • ✅ JSON serialization compatibility

Next Steps

This implements the foundation for self-signed certificate support. Future phases will:

  1. Phase 2: Update FFI client SDKs (Python, Java, C#, Ruby, Dart) to expose TLS options
  2. Phase 3: Update WASM clients (Go, JavaScript) with equivalent functionality

Related

Re: #1132
Related to discussion: https://github.com/orgs/flipt-io/discussions/4366

Breaking Changes

None - all changes are backward compatible.

@codecov-commenter
Copy link

codecov-commenter commented Jun 27, 2025

Codecov Report

Attention: Patch coverage is 91.51786% with 19 lines in your changes missing coverage. Please review.

Project coverage is 75.48%. Comparing base (f701007) to head (efe80f4).
Report is 430 commits behind head on main.

Files with missing lines Patch % Lines
flipt-engine-ffi/src/lib.rs 78.84% 11 Missing ⚠️
flipt-engine-ffi/src/http.rs 97.04% 5 Missing ⚠️
flipt-engine-wasm/src/lib.rs 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1133      +/-   ##
==========================================
- Coverage   80.36%   75.48%   -4.88%     
==========================================
  Files           8        9       +1     
  Lines        4165     4467     +302     
==========================================
+ Hits         3347     3372      +25     
- Misses        818     1095     +277     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Add TlsConfig struct with support for custom CA certificates,
insecure skip verify, and client certificates for mutual TLS.

Fixes #1132

Signed-off-by: Mark Phelps <[email protected]>
@markphelps markphelps force-pushed the feat/issue-1132-self-signed-cert-support branch from 2214bd5 to e30f7bd Compare June 27, 2025 14:45
Signed-off-by: Mark Phelps <[email protected]>
Update error! macro calls to use inline format arguments
instead of positional arguments for better readability.

Signed-off-by: Mark Phelps <[email protected]>
@markphelps markphelps marked this pull request as ready for review June 27, 2025 15:00
@markphelps markphelps requested a review from a team as a code owner June 27, 2025 15:00
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Jun 27, 2025
@markphelps markphelps requested review from erka and Copilot June 27, 2025 15:02
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Implements initial TLS configuration support in the FFI engine, allowing clients to supply CA and client certificates (file or base64 data) and skip verification for development.

  • Introduced TlsConfig in EngineOpts with optional fields for CA and client certificates.
  • Integrated TLS settings into HTTPFetcherBuilder via configure_tls().
  • Added unit tests covering all TLS configuration permutations and error cases.

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

File Description
release/README.md Removed a stray list marker in the release script instructions
flipt-engine-wasm/src/lib.rs Switched panic logging to Rust’s {e} interpolation in eprintln! calls
flipt-engine-ffi/src/lib.rs Added TlsConfig type, wired into EngineOpts default and FFI init paths
flipt-engine-ffi/src/http.rs Added configure_tls() to apply TLS options on reqwest::ClientBuilder
Comments suppressed due to low confidence (3)

flipt-engine-ffi/src/http.rs:495

  • [nitpick] Consider documenting the precedence rules (e.g., ca_cert_data takes priority over ca_cert_file, and likewise for client certs) so users know which option is used when both are set.
fn configure_tls(

flipt-engine-ffi/src/lib.rs:106

  • [nitpick] Add #[serde(rename_all = "snake_case")] to TlsConfig to explicitly enforce and document the JSON key naming convention.
pub struct TlsConfig {

flipt-engine-ffi/src/http.rs:183

  • [nitpick] Add an integration test for HTTPFetcherBuilder::build() with a non-None tls_config to verify that the resulting client applies the expected TLS settings.
    pub fn tls_config(mut self, tls_config: TlsConfig) -> Self {

Signed-off-by: Mark Phelps <[email protected]>
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 28, 2025
@markphelps markphelps merged commit 352156e into main Jun 29, 2025
34 of 35 checks passed
@markphelps markphelps deleted the feat/issue-1132-self-signed-cert-support branch June 29, 2025 00:47
markphelps added a commit that referenced this pull request Jun 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants