Skip to content

Commit 4289db0

Browse files
jackivanovdguidoclaude
authored
Refactor StrongSwan PKI tasks to use Ansible crypto modules and remove legacy OpenSSL scripts (#14809)
* Refactor StrongSwan PKI automation with Ansible crypto modules - Replace shell-based OpenSSL commands with community.crypto modules - Remove custom OpenSSL config template and manual file management - Upgrade Ansible to 11.8.0 in requirements.txt - Improve idempotency, maintainability, and security of certificate and CRL handling * Enhance nameConstraints with comprehensive exclusions - Add email domain exclusions (.com, .org, .net, .gov, .edu, .mil, .int) - Include private IPv4 network exclusions - Add IPv6 null route exclusion - Preserve all security constraints from original openssl.cnf.j2 - Note: Complex IPv6 conditional logic simplified for Ansible compatibility Security: Maintains defense-in-depth certificate scope restrictions * Refactor StrongSwan PKI with comprehensive security enhancements and hybrid testing ## StrongSwan PKI Modernization - Migrated from shell-based OpenSSL commands to Ansible community.crypto modules - Simplified complex Jinja2 templates while preserving all security properties - Added clear, concise comments explaining security rationale and Apple compatibility ## Enhanced Security Implementation (Issues #75, #153) - **Name constraints**: CA certificates restricted to specific IP/email domains - **EKU role separation**: Server certs (serverAuth only) vs client certs (clientAuth only) - **Domain exclusions**: Blocks public domains (.com, .org, etc.) and private IP ranges - **Apple compatibility**: SAN extensions and PKCS#12 compatibility2022 encryption - **Certificate revocation**: Automated CRL generation for removed users ## Comprehensive Test Suite - **Hybrid testing**: Validates real certificates when available, config validation for CI - **Security validation**: Verifies name constraints, EKU restrictions, role separation - **Apple compatibility**: Tests SAN extensions and PKCS#12 format compliance - **Certificate chain**: Validates CA signing and certificate validity periods - **CI-compatible**: No deployment required, tests Ansible configuration directly ## Configuration Updates - Updated CLAUDE.md: Ansible version rationale (stay current for security/performance) - Streamlined comments: Removed duplicative explanations while preserving technical context - Maintained all Issue #75/#153 security enhancements with modern Ansible approach 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * Fix linting issues across the codebase ## Python Code Quality (ruff) - Fixed import organization and removed unused imports in test files - Replaced `== True` comparisons with direct boolean checks - Added noqa comments for intentional imports in test modules ## YAML Formatting (yamllint) - Removed trailing spaces in openssl.yml comments - All YAML files now pass yamllint validation (except one pre-existing long regex line) ## Code Consistency - Maintained proper import ordering in test files - Ensured all code follows project linting standards - Ready for CI pipeline validation 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * Replace magic number with configurable certificate validity period ## Maintainability Improvement - Replaced hardcoded `+3650d` (10 years) with configurable variable - Added `certificate_validity_days: 3650` in vars section with clear documentation - Applied consistently to both server and client certificate signing ## Benefits - Single location to modify certificate validity period - Supports compliance requirements for shorter certificate lifespans - Improves code readability and maintainability - Eliminates magic number duplication ## Backwards Compatibility - Default remains 10 years (3650 days) - no behavior change - Organizations can now easily customize certificate validity as needed 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * Update test to validate configurable certificate validity period ## Test Update - Fixed test failure after replacing magic number with configurable variable - Now validates both variable definition and usage patterns: - `certificate_validity_days: 3650` (configurable parameter) - `ownca_not_after: "+{{ certificate_validity_days }}d"` (variable usage) ## Improved Test Coverage - Better validation: checks that validity is configurable, not hardcoded - Maintains backwards compatibility verification (10-year default) - Ensures proper Ansible variable templating is used ## Verified - Config validation mode: All 6 tests pass ✓ - Validates the maintainability improvement from previous commit 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * Update to Python 3.11 minimum and fix IPv6 constraint format - Update Python requirement from 3.10 to 3.11 to align with Ansible 11 - Pin Ansible collections in requirements.yml for stability - Fix invalid IPv6 constraint format causing deployment failure - Update ruff target-version to py311 for consistency 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * Fix x509_crl mode parameter and auto-fix Python linting - Remove deprecated 'mode' parameter from x509_crl task - Add separate file task to set CRL permissions (0644) - Auto-fix Python datetime import (use datetime.UTC alias) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * Fix final IPv6 constraint format in defaults template - Update nameConstraints template in defaults/main.yml - Change malformed IP:0:0:0:0:0:0:0:0/0:0:0:0:0:0:0:0 to correct IP:::/0 - This ensures both Ansible crypto modules and OpenSSL template use consistent IPv6 format * Fix critical certificate generation issues for macOS/iOS VPN compatibility This commit addresses multiple certificate generation bugs in the Ansible crypto module implementation that were causing VPN authentication failures on Apple devices. Fixes implemented: 1. **Basic Constraints Extension**: Added missing `CA:FALSE` constraints to both server and client certificate CSRs. This was causing certificate chain validation errors on macOS/iOS devices. 2. **Subject Key Identifier**: Added `create_subject_key_identifier: true` to CA certificate generation to enable proper Authority Key Identifier creation in signed certificates. 3. **Complete Name Constraints**: Fixed missing DNS and IPv6 constraints in CA certificate that were causing size differences compared to legacy shell-based generation. Now includes: - DNS constraints for the deployment-specific domain - IPv6 permitted addresses when IPv6 support is enabled - Complete IPv6 exclusion ranges (fc00::/7, fe80::/10, 2001:db8::/32) These changes bring the certificate format much closer to the working shell-based implementation and should resolve most macOS/iOS VPN connectivity issues. **Outstanding Issue**: Authority Key Identifier still incomplete - missing DirName and serial components. The community.crypto module limitation may require additional investigation or alternative approaches. Certificate size improvements: Server certificates increased from ~750 to ~775 bytes, CA certificates from ~1070 to ~1250 bytes, bringing them closer to the expected ~3000 byte target size. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * Fix certificate generation and improve version parsing This commit addresses multiple issues found during macOS certificate validation: Certificate Generation Fixes: - Add Basic Constraints (CA:FALSE) to server and client certificates - Generate Subject Key Identifier for proper AKI creation - Improve Name Constraints implementation for security - Update community.crypto to version 3.0.3 for latest fixes Code Quality Improvements: - Clean up certificate comments and remove obsolete references - Fix server certificate identification in tests - Update datetime comparisons for cryptography library compatibility - Fix Ansible version parsing in main.yml with proper regex handling Testing: - All certificate validation tests pass - Ansible syntax checks pass - Python linting (ruff) clean - YAML linting (yamllint) clean These changes restore macOS/iOS certificate compatibility while maintaining security best practices and improving code maintainability. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * Enhance security documentation with comprehensive inline comments Add detailed technical explanations for critical PKI security features: - Name Constraints: Defense-in-depth rationale and attack prevention - Public domain/network exclusions: Impersonation attack prevention - RFC 1918 private IP blocking: Lateral movement prevention - IPv6 constraint strategy: ULA/link-local/documentation range handling - Role separation enforcement: Server vs client EKU restrictions - CA delegation prevention: pathlen:0 security implications - Cross-deployment isolation: UUID-based certificate scope limiting These comments provide essential context for maintainers to understand the security importance of each configuration without referencing external issue numbers, ensuring long-term maintainability. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * Fix CI test failures in PKI certificate validation Resolve Smart Test Selection workflow failures by fixing test validation logic: **Certificate Configuration Fixes:** - Remove unnecessary serverAuth/clientAuth EKUs from CA certificate - CA now only has IPsec End Entity EKU for VPN-specific certificate issuance - Maintains proper role separation between server and client certificates **Test Validation Improvements:** - Fix domain exclusion detection to handle both single and double quotes in YAML - Improve EKU validation to check actual configuration lines, not comments - Server/client certificate tests now correctly parse YAML structure - Tests pass in both CI mode (config validation) and local mode (real certificates) **Root Cause:** The CI failures were caused by overly broad test assertions that: 1. Expected double-quoted strings but found single-quoted YAML 2. Detected EKU keywords in comments rather than actual configuration 3. Failed to properly parse YAML list structures All security constraints remain intact - no actual security issues were present. The certificate generation produces properly constrained certificates for VPN use. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * Fix trailing space in openssl.yml for yamllint compliance --------- Co-authored-by: Dan Guido <[email protected]> Co-authored-by: Claude <[email protected]>
1 parent 0aaca43 commit 4289db0

File tree

11 files changed

+770
-546
lines changed

11 files changed

+770
-546
lines changed

CLAUDE.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ algo/
5151

5252
### Current Versions (MUST maintain compatibility)
5353
```
54-
ansible==9.2.0 # Stay within 9.x for stability
54+
ansible==11.8.0 # Stay current to get latest security, performance and bugfixes
5555
jinja2~=3.1.6 # Security fix for CVE-2025-27516
5656
netaddr==1.3.0 # Network address manipulation
5757
```
@@ -76,7 +76,7 @@ Currently unpinned in `requirements.yml`, but key ones include:
7676
```toml
7777
# pyproject.toml configuration
7878
[tool.ruff]
79-
target-version = "py310"
79+
target-version = "py311"
8080
line-length = 120
8181

8282
[tool.ruff.lint]

main.yml

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,23 +22,29 @@
2222
no_log: true
2323
register: ipaddr
2424

25-
- name: Set required ansible version as a fact
25+
- name: Extract ansible version from requirements
2626
set_fact:
27-
required_ansible_version: "{{ item | regex_replace('^ansible[\\s+]?(?P<op>[=,>,<]+)[\\s+]?(?P<ver>\\d.\\d+(.\\d+)?)$', '{\"op\": \"\\g<op>\",\"ver\"\
28-
: \"\\g<ver>\" }') }}"
27+
ansible_requirement: "{{ item }}"
2928
when: '"ansible" in item'
3029
with_items: "{{ lookup('file', 'requirements.txt').splitlines() }}"
3130

31+
- name: Parse ansible version requirement
32+
set_fact:
33+
required_ansible_version:
34+
op: "{{ ansible_requirement | regex_replace('^ansible\\s*([~>=<]+)\\s*.*$', '\\1') }}"
35+
ver: "{{ ansible_requirement | regex_replace('^ansible\\s*[~>=<]+\\s*(\\d+\\.\\d+(?:\\.\\d+)?).*$', '\\1') }}"
36+
when: ansible_requirement is defined
37+
3238
- name: Just get the list from default pip
3339
community.general.pip_package_info:
3440
register: pip_package_info
3541

3642
- name: Verify Python meets Algo VPN requirements
3743
assert:
38-
that: (ansible_python.version.major|string + '.' + ansible_python.version.minor|string) is version('3.8', '>=')
44+
that: (ansible_python.version.major|string + '.' + ansible_python.version.minor|string) is version('3.11', '>=')
3945
msg: >
4046
Python version is not supported.
41-
You must upgrade to at least Python 3.8 to use this version of Algo.
47+
You must upgrade to at least Python 3.11 to use this version of Algo.
4248
See for more details - https://trailofbits.github.io/algo/troubleshooting.html#python-version-is-not-supported
4349
4450
- name: Verify Ansible meets Algo VPN requirements

pyproject.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@
22
name = "algo"
33
description = "Set up a personal IPSEC VPN in the cloud"
44
version = "0.1.0"
5-
requires-python = ">=3.10"
5+
requires-python = ">=3.11"
66

77
[tool.ruff]
88
# Ruff configuration
9-
target-version = "py310"
9+
target-version = "py311"
1010
line-length = 120
1111

1212
[tool.ruff.lint]

requirements.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
ansible==9.13.0
1+
ansible==11.8.0
22
jinja2~=3.1.6
33
netaddr==1.3.0

requirements.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
---
22
collections:
33
- name: ansible.posix
4+
version: ">=2.1.0"
45
- name: community.general
6+
version: ">=11.1.0"
57
- name: community.crypto
8+
version: ">=3.0.3"
69
- name: openstack.cloud
10+
version: ">=2.4.1"

roles/strongswan/defaults/main.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ nameConstraints: >-
3737
,permitted;IP:{{ ansible_default_ipv6['address'] }}/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff
3838
,excluded;IP:fc00:0:0:0:0:0:0:0/fe00:0:0:0:0:0:0:0,excluded;IP:fe80:0:0:0:0:0:0:0/ffc0:0:0:0:0:0:0:0,excluded;IP:2001:db8:0:0:0:0:0:0/ffff:fff8:0:0:0:0:0:0
3939
{%- else -%}
40-
,excluded;IP:0:0:0:0:0:0:0:0/0:0:0:0:0:0:0:0
40+
,excluded;IP:::/0
4141
{%- endif -%}
4242
openssl_bin: openssl
4343
strongswan_enabled_plugins:

0 commit comments

Comments
 (0)