Skip to content

Conversation

@inureyes
Copy link
Member

Summary

Implements a Logstash exporter for audit events, enabling integration with ELK stack (Elasticsearch, Logstash, Kibana) for log analysis.

Changes

  • ✅ LogstashExporter implementation in src/server/audit/logstash.rs
  • ✅ TCP connection with JSON Lines protocol
  • ✅ Auto-reconnection on failure
  • ✅ Batch support for efficient event transmission
  • ✅ Connection timeout handling (10 seconds)
  • ✅ Comprehensive test coverage (9 unit tests)
  • ✅ Wire up in AuditManager

Implementation Details

LogstashExporter

  • Maintains TCP connection with configurable host and port
  • Implements AuditExporter trait with all required methods
  • Automatically reconnects when connection drops
  • Uses JSON Lines protocol (each event as JSON followed by newline)
  • Supports both single event and batch operations

Tests

All tests passing (9/9):

  • test_logstash_exporter_creation
  • test_logstash_exporter_invalid_host
  • test_format_event
  • test_export_single_event
  • test_export_batch
  • test_connection_timeout
  • test_flush
  • test_close
  • test_json_lines_format

Configuration Example

audit:
  enabled: true
  exporters:
    - type: logstash
      host: logstash.example.com
      port: 5044

Testing

cargo build --lib
cargo clippy --lib -- -D warnings
cargo test --lib server::audit::logstash
cargo fmt

Resolves #137

Add LogstashExporter implementation that sends audit events to Logstash via TCP using JSON Lines protocol.

Key features:
- TCP connection with automatic reconnection on failure
- JSON Lines protocol (newline-delimited JSON)
- Batch support for efficient event transmission
- Connection timeout handling (10 seconds)
- Comprehensive test coverage

Implementation details:
- Create src/server/audit/logstash.rs with LogstashExporter struct
- Wire up LogstashExporter in AuditManager
- Implement AuditExporter trait methods: export, export_batch, flush, close
- Add 9 unit tests covering all functionality including edge cases

Resolves #137
@inureyes inureyes added priority:medium Medium priority issue status:review Under review type:enhancement New feature or request labels Jan 24, 2026
@inureyes
Copy link
Member Author

Security & Performance Review

Analysis Summary

  • PR Branch: feature/issue-137-logstash-audit-exporter
  • Scope: changed-files
  • Languages: Rust
  • Total issues: 6
  • Critical: 1 | High: 2 | Medium: 2 | Low: 1

CRITICAL

1. Unencrypted Transmission of Sensitive Audit Data

File: src/server/audit/logstash.rs (lines 130-145)

The LogstashExporter uses plain TCP without TLS encryption to transmit audit events. Audit logs often contain sensitive security information including:

  • Usernames and session IDs
  • Client IP addresses
  • File paths and operations
  • Authentication events

Impact: An attacker with network access could intercept audit data in transit (man-in-the-middle attack), compromising security monitoring and potentially exposing sensitive information.

Comparison: The OtelExporter in otel.rs (lines 92-99) warns when HTTPS is not used:

if !endpoint.starts_with("https://") {
    tracing::warn!(
        endpoint = %endpoint,
        "OpenTelemetry audit exporter is not using HTTPS. ..."
    );
}

Recommendation:

  1. Add TLS support using tokio-rustls or tokio-native-tls
  2. At minimum, add a warning log when connecting without TLS (similar to OtelExporter)
  3. Document the security implications in the module documentation

HIGH

2. Missing Host Validation - Potential SSRF Vector

File: src/server/audit/logstash.rs (lines 91-103)

The new() function only validates that the host is non-empty, but does not validate the host format. A misconfigured or malicious host value could be used to:

  • Connect to internal services (SSRF)
  • Resolve to unexpected addresses
pub fn new(host: &str, port: u16) -> Result<Self> {
    if host.is_empty() {
        anyhow::bail!("Logstash host cannot be empty");
    }
    // No further validation of host format

Recommendation: Add validation similar to OtelExporter's URL validation:

  • Validate the host is a valid hostname or IP address
  • Consider blocking loopback addresses in production
  • Log a warning if connecting to non-standard addresses

3. Reconnect Delay Blocks Event Export

File: src/server/audit/logstash.rs (line 173)

When a connection fails during send(), the code sleeps for reconnect_delay (5 seconds) while holding the mutex lock:

// Connection lost or didn't exist, reconnect and retry
tokio::time::sleep(self.reconnect_delay).await;  // Blocks with lock held
let mut stream = self.connect().await?;

Impact:

  • All concurrent export operations are blocked during reconnection
  • Under high load with connection issues, audit events could be significantly delayed or dropped
  • The 5-second delay compounds with connection timeout (10 seconds), causing up to 15 seconds of blocking

Recommendation:

  • Drop the lock before sleeping and re-acquire after
  • Consider using a connection retry backoff mechanism with exponential increase
  • Consider adding a "fast-fail" mode for high-availability scenarios

MEDIUM

4. Unbounded Batch Buffer Growth

File: src/server/audit/logstash.rs (lines 208-217)

The export_batch() function builds a complete string buffer in memory before sending:

async fn export_batch(&self, events: &[AuditEvent]) -> Result<()> {
    self.ensure_connected().await?;
    let mut batch = String::new();
    for event in events {
        batch.push_str(&self.format_event(event)?);
    }
    self.send(batch.as_bytes()).await
}

Impact: For large batches, this could cause memory pressure or OOM in constrained environments.

Recommendation:

  • Add a maximum batch size constant
  • Consider streaming the events to the socket instead of buffering
  • Pre-allocate the string buffer with estimated capacity

5. Audit Event Loss on Partial Send Failure

File: src/server/audit/logstash.rs (lines 158-182)

If a write partially succeeds or the connection drops mid-batch, there's no mechanism to track which events were successfully sent:

async fn send(&self, data: &[u8]) -> Result<()> {
    // ... if write fails, reconnect and retry from beginning
    stream.write_all(data).await  // Retries entire buffer

Impact: During network instability, events could be:

  • Sent twice (if partial success before failure)
  • Lost (if retry also fails)

Recommendation:

  • Consider implementing idempotent event IDs for deduplication at Logstash
  • Add event send confirmation in documentation
  • Consider implementing a local buffer/queue for reliability

LOW

6. Missing Debug Trait Implementation

File: src/server/audit/logstash.rs

The LogstashExporter struct does not implement Debug, unlike OtelExporter which has a custom Debug implementation:

impl std::fmt::Debug for OtelExporter {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { ... }
}

Impact: Makes debugging and logging the exporter state more difficult.

Recommendation: Add #[derive(Debug)] or implement Debug manually (to avoid exposing connection internals).


Positive Observations

  1. Good test coverage: 9 comprehensive unit tests covering creation, export, batch, timeout, and edge cases
  2. Proper async handling: Uses tokio::sync::Mutex correctly for async context
  3. Consistent error handling: Uses anyhow::Context for proper error context propagation
  4. Idempotent close(): The close operation can be called multiple times safely
  5. Good documentation: Module-level docs with example code

Summary

The implementation is functionally correct and follows Rust best practices for async code. The main concerns are:

  1. Security: The lack of TLS encryption is a significant security gap for transmitting sensitive audit data
  2. Reliability: The reconnection handling could block operations and lose events during network issues

These issues should be addressed before using this exporter in production environments where audit log integrity and confidentiality are important.

Mark LogstashExporter as implemented (no longer "planned") and add
documentation with usage example.
@inureyes
Copy link
Member Author

PR Finalization Report

Project Structure Discovered

  • Project Type: Rust
  • Test Framework: cargo test
  • Documentation System: Plain markdown (docs/architecture/)
  • Lint Tools: cargo fmt, cargo clippy

Checklist

Tests

  • Analyzed existing test structure
  • Verified test coverage (11 tests in logstash module)
  • All tests passing

Documentation

  • Updated ARCHITECTURE.md to reflect LogstashExporter is now implemented (previously marked as "planned")
  • Added LogstashExporter usage example with TLS configuration
  • Server configuration documentation already includes Logstash exporter configuration

Code Quality

  • cargo fmt --check: Passed
  • cargo clippy --lib -- -D warnings: Passed
  • All warnings resolved

Changes Made

  • Updated /home/inureyes/Development/backend.ai/bssh/ARCHITECTURE.md:
    • Added LogstashExporter description with features
    • Added Rust usage example with TLS support
    • Removed "Future Exporters (planned)" section for Logstash

Test Summary

All 11 logstash tests pass:

  • test_logstash_exporter_creation
  • test_logstash_exporter_invalid_host
  • test_host_validation
  • test_with_tls
  • test_format_event
  • test_export_single_event
  • test_export_batch
  • test_connection_timeout
  • test_flush
  • test_close
  • test_json_lines_format

Notes

  • The LogstashExporter implementation is complete with TLS support
  • The module is properly wired up in AuditManager
  • Documentation has been updated to reflect implementation status

@inureyes inureyes merged commit c2dbb31 into main Jan 24, 2026
1 of 2 checks passed
@inureyes inureyes deleted the feature/issue-137-logstash-audit-exporter branch January 24, 2026 05:37
@inureyes inureyes self-assigned this Jan 24, 2026
@inureyes inureyes added status:done Completed and removed status:review Under review labels Jan 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority:medium Medium priority issue status:done Completed type:enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement Logstash audit exporter

2 participants