Commit b77f8ec
Add TLS support for Registrar communication (#1139)
* Add TLS support for Registrar communication
This change enables TLS-based communication with the Registrar service
in the push model agent, providing secure registration and activation.
Key changes:
- Added registrar_tls_enabled, registrar_tls_ca_cert,
registrar_tls_client_cert, and registrar_tls_client_key configuration
options with empty defaults for backwards compatibility
- Updated RegistrarClientBuilder to accept TLS configuration parameters
(ca_certificate, certificate, key, insecure, timeout)
- Modified RegistrarClient to use HTTPS client when TLS is configured,
falling back to plain HTTP when TLS parameters are not provided
- Refactored to use single ResilientClient for all HTTP/HTTPS requests
instead of maintaining separate client instances
- Added RegistrarTlsConfig struct in push model agent to manage TLS
configuration from config file
- Updated StateMachine to accept and pass registrar_tls_config to
registration functions
Backwards compatibility:
- Defaults to plain HTTP when registrar_tls_enabled is false (default)
- Defaults to plain HTTP when TLS certificate paths are empty (default)
- TLS only enabled when all three certificate paths are provided AND
registrar_tls_enabled is true
- Pull model agent unchanged - maintains existing behavior with None
values for all new TLS fields
The implementation separates Registrar TLS configuration from Verifier
TLS configuration, allowing each service to be secured independently
based on deployment requirements.
Co-Authored-By: Claude <[email protected]>
Signed-off-by: Sergio Arroutbi <[email protected]>
* Add comprehensive unit tests for Registrar TLS communication
This commit adds extensive unit tests to increase coverage for the
Registrar TLS communication feature, focusing on actual execution of
production code paths rather than just configuration validation.
Tests added in https_client.rs (10 tests, 330 lines):
- test_get_https_client_with_valid_certs: Validates successful HTTPS
client creation with real generated TLS certificates
- test_get_https_client_insecure_mode: Tests insecure mode bypassing
certificate validation
- test_get_https_client_missing_ca_cert: Tests error handling when
CA certificate file is missing
- test_get_https_client_missing_client_cert: Tests error handling when
client certificate file is missing
- test_get_https_client_missing_client_key: Tests error handling when
client key file is missing
- test_get_https_client_invalid_ca_cert: Tests error handling with
malformed CA certificate content
- test_get_https_client_invalid_client_identity: Tests error handling
with invalid client certificate/key pair
- test_get_https_client_with_different_timeouts: Validates timeout
configuration with various values (0, 1000, 5000, 30000, 300000ms)
- test_get_https_client_insecure_default: Tests default behavior when
insecure flag is None
- test_get_https_client_empty_ca_cert_path: Tests error handling with
empty certificate path strings
Tests added in registration.rs (17 tests, 581 lines):
- test_get_retry_config_all_none: Tests retry config when all values
are None (should return None)
- test_get_retry_config_with_max_retries: Tests retry config with only
max_retries set
- test_get_retry_config_with_initial_delay: Tests retry config with
only initial_delay set
- test_get_retry_config_with_max_delay: Tests retry config with only
max_delay set
- test_get_retry_config_with_all_values: Tests retry config with all
values configured
- test_check_registration_with_none_context: Tests registration check
with no context (early return path)
- test_register_agent_creates_agent_registration_config: Tests full
registration flow with TLS config and retry config
- Plus 10 additional tests for TLS config validation, partial configs,
insecure mode, and integration with real certificates
Tests added in registrar_client.rs (21 tests, 421 lines):
- Builder pattern tests for TLS configuration fields (ca_certificate,
certificate, key, insecure, timeout)
- Tests for partial TLS configurations (missing CA, cert, or key)
- Tests for empty string paths and various timeout values
- test_builder_with_real_tls_certificates: Validates builder with
actual generated certificates written to temp files
- test_builder_build_with_invalid_tls_cert_files: Tests build failure
with non-existent certificate files
- test_tls_enabled_when_all_certs_provided: Tests TLS activation when
all certificate paths are provided
- test_tls_disabled_when_insecure_true: Tests TLS bypass with insecure
flag
- test_http_fallback_when_partial_tls_config: Tests HTTP fallback when
only some TLS parameters are provided
Test infrastructure:
- Added generate_test_certificates() helper in registrar_client.rs that
creates real CA, client, and server certificates using
crypto::x509::CertificateBuilder
- Added generate_test_tls_certificates() helper in registration.rs for
creating TLS test certificates
- Added generate_test_certificates() helper in https_client.rs for
certificate generation
- All certificate generation uses crypto::testing::rsa_generate(2048)
with proper PKCS8 PEM encoding
- Certificates written to temporary directories that are automatically
cleaned up after tests
Coverage improvements:
- https_client.rs: Executes production TLS certificate loading code
(lines 16-67), including CA cert parsing, client identity creation,
and error handling paths
- registration.rs: Covers get_retry_config() function logic (lines
33-52) and TLS config extraction (lines 60-71)
- registrar_client.rs: Tests builder configuration but not build()
execution paths (requires running server)
All tests require --features testing flag as they depend on
crypto::testing::rsa_generate() which is only available with the
testing feature enabled.
Tests validated with: cargo test --features testing --lib
All 305+ tests passing.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Signed-off-by: Sergio Arroutbi <[email protected]>
* Add Mockoon-based registration tests
This commit introduces a suite of integration tests for the registrar
client using Mockoon to simulate a registrar server. These tests provide
coverage for HTTP and HTTPS interactions, including agent registration,
activation, and API version retrieval.
Key changes include:
* New GitHub Actions Workflow: A dedicated workflow,
mockoon-registrar-tests, has been added to mockoon.yaml to run the new
registrar tests in CI. [cite_start]This workflow installs and runs the
Mockoon CLI with a new data file.
* Mockoon Registrar Configuration: A new JSON file, registrar.json
[cite_start], defines the mock registrar's API endpoints, including
/version
/v1.2/agents/{uuid} for registration and activation, and
their TLS-secured counterparts.
* Rust Integration Tests: New tests have been added to registrar_client.rs
to cover:
** HTTP agent registration and activation.
** API version endpoint retrieval.
** Client behavior with retry configurations.
** Verification of the TLS code path during client build.
* Test Execution Script: The mockoon_registrar_tests.sh script manages the
setup and teardown of the testing environment. It starts the
Mockoon server on port 3001, handles certificate generation
and executes the Cargo tests. It also includes
cleanup logic to stop the Mockoon server after the test run.
Assisted-by: Gemini <[email protected]>
Signed-off-by: Sergio Arroutbi <[email protected]>
* Fix Mockoon port conflict in CI registrar tests
The CI job for mockoon-registrar-tests was failing because both the GitHub
Actions mockoon/cli-action and the test script were trying to start Mockoon
on port 3001, causing a "Port 3001 is already in use" error.
Changes:
- Enhanced port detection logic in tests/mockoon_registrar_tests.sh to use
multiple methods (lsof, netstat, ss, and curl) for detecting if port 3001
is already in use
- Improved cleanup logic to properly terminate Mockoon processes using
multiple methods (kill by PID, lsof, pkill by process name)
- Made the script more robust in CI environments where lsof might not
be available or behave differently
The test script now properly detects when Mockoon is already running
(started by the GitHub Actions workflow) and skips starting a duplicate
instance, avoiding the port conflict.
Co-Authored-By: Claude <[email protected]>
Signed-off-by: Sergio Arroutbi <[email protected]>
* Fix issues related to linter (cargo fmt)
Signed-off-by: Sergio Arroutbi <[email protected]>
* Add comprehensive port 3001 debugging and logging
Enhanced the Mockoon CI testing infrastructure with extensive debugging
capabilities to diagnose port conflicts and process issues.
Changes to tests/mockoon_registrar_tests.sh:
- Added log_port_3001_info() function that captures detailed information:
* Process IDs, commands, users, parent PIDs, start times
* Working directories and environment variables
* Network socket information (lsof, netstat, ss)
* Docker container status and systemd services
* HTTP connectivity tests and response headers
- Enhanced cleanup logic with before/after logging
- Made the script resilient to missing system tools
Changes to .github/workflows/mockoon.yaml:
- Added pre-Mockoon debugging step to capture baseline system state
- Added post-Mockoon debugging step to verify Mockoon startup
- Logs available system tools, network state, processes, and environment
- Provides comprehensive visibility into CI environment
This will help diagnose any future port conflicts or process management
issues in the CI environment, making debugging much more efficient.
Co-Authored-By: Claude <[email protected]>
Signed-off-by: Sergio Arroutbi <[email protected]>
* Apply shellcheck and yamllint fixes to Mockoon CI
Fixed all shellcheck and yamllint issues in the Mockoon testing infrastructure:
Shellcheck fixes in tests/mockoon_registrar_tests.sh:
- Added proper quoting for /proc/$pid paths to prevent word splitting
- Replaced 'cat file | cmd' with 'cmd < file' to avoid useless cat
- Replaced 'ps aux | grep' with 'pgrep' for better process detection
- All variables now properly quoted to prevent globbing
Yamllint fixes in .github/workflows/mockoon.yaml:
- Split long lines (>80 chars) using YAML line continuation
- Improved readability while maintaining functionality
- All debugging commands now properly formatted
The code now passes both shellcheck (only expected SC1091 source
warnings remain) and yamllint with no errors, improving code quality
and maintainability.
Co-Authored-By: Claude <[email protected]>
Signed-off-by: Sergio Arroutbi <[email protected]>
* Improve TLS testing infrastructure and reliability
This change consolidates duplicate certificate generation code and improves
test infrastructure reliability based on code review feedback.
Key improvements:
1. Consolidate Certificate Generation Utilities:
- Removed three separate and nearly identical certificate generation
helper functions from https_client.rs, registration.rs, and
registrar_client.rs (261 lines of duplicate code eliminated)
- Introduced a single, comprehensive utility function
crypto::testing::generate_tls_certs_for_test that creates CA,
server, and client certificate sets for testing purposes
- All relevant tests refactored to use this new shared utility,
reducing code duplication and improving maintainability
2. Strengthen Mockoon TLS Test Assertion:
- Fixed weak assertion assert!(result.is_err() || result.is_ok()) in
test_mockoon_registrar_https_registration test
- Changed to assert!(result.is_err()) to specifically verify that
connection fails as expected when mock server lacks TLS configuration
3. Replace Brittle Sleep with Polling Mechanism:
- Replaced sleep 3 command in tests/mockoon_registrar_tests.sh with
robust polling loop using curl to check server responsiveness
- Added 15-second timeout with proper error handling
- Makes tests less prone to failures on slower or busier CI runners
Co-Authored-By: Gemini <[email protected]>
Co-Authored-By: Claude <[email protected]>
Signed-off-by: Sergio Arroutbi <[email protected]>
* Add default ECC P-256 support for mTLS keys
This change updates the default key generation algorithm for mTLS keys
from RSA 2048 to ECC P-256 (secp256r1) for improved security and
performance characteristics.
Key changes:
- Updated keylime-agent/src/main.rs to use Ecc256 instead of Rsa2048
for mTLS key generation via load_or_generate_key()
- Updated keylime/src/cert.rs cert_from_server_key() function to
generate ECC P-256 keys instead of RSA 2048 when creating new keys
Benefits:
- Smaller key sizes (256-bit vs 2048-bit) with equivalent security
- Faster key generation and cryptographic operations
- Lower memory and storage footprint
- Better performance in embedded and resource-constrained environments
Backward compatibility:
- Existing RSA keys will continue to work due to load_or_generate_key
logic that loads existing keys regardless of algorithm
- Algorithm validation is disabled for mTLS keys to maintain compatibility
- Only affects new key generation when no existing key file is found
The ECC P-256 curve (X9_62_PRIME256V1/secp256r1) is widely supported,
FIPS 186-4 approved, and provides 128-bit security level equivalent
to RSA 3072.
Co-Authored-By: Claude <[email protected]>
Signed-off-by: Sergio Arroutbi <[email protected]>
* Include additional logs for troubleshooting
Signed-off-by: Sergio Arroutbi <[email protected]>
* Fix registrar client issue after rebasing
Signed-off-by: Sergio Arroutbi <[email protected]>
* Refactor and warn missing configuration paths
Co-Authored-By: Claude <[email protected]>
Signed-off-by: Sergio Arroutbi <[email protected]>
---------
Signed-off-by: Sergio Arroutbi <[email protected]>
Co-authored-by: Claude <[email protected]>
Co-authored-by: Gemini <[email protected]>1 parent ec3e412 commit b77f8ec
File tree
14 files changed
+2807
-94
lines changed- .github/workflows
- keylime-agent/src
- keylime-push-model-agent
- src
- test-data
- keylime/src
- config
- tests
14 files changed
+2807
-94
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
| 11 | + | |
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| |||
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
27 | | - | |
| 27 | + | |
28 | 28 | | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
515 | 515 | | |
516 | 516 | | |
517 | 517 | | |
| 518 | + | |
518 | 519 | | |
519 | 520 | | |
520 | 521 | | |
521 | 522 | | |
522 | | - | |
| 523 | + | |
523 | 524 | | |
524 | 525 | | |
525 | 526 | | |
| |||
608 | 609 | | |
609 | 610 | | |
610 | 611 | | |
| 612 | + | |
| 613 | + | |
| 614 | + | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
611 | 618 | | |
612 | 619 | | |
613 | 620 | | |
| |||
0 commit comments