BREAKING CHANGE(certs): accept a second eventComms argument in certProvisionFunction, add cert provisioning event types, and emit certificate lifecycle events
This commit is contained in:
10
changelog.md
10
changelog.md
@@ -1,5 +1,15 @@
|
||||
# Changelog
|
||||
|
||||
## 2026-02-13 - 25.0.0 - BREAKING CHANGE(certs)
|
||||
accept a second eventComms argument in certProvisionFunction, add cert provisioning event types, and emit certificate lifecycle events
|
||||
|
||||
- Breaking API change: certProvisionFunction signature changed from (domain: string) => Promise<TSmartProxyCertProvisionObject> to (domain: string, eventComms: ICertProvisionEventComms) => Promise<TSmartProxyCertProvisionObject>. Custom provisioners must accept (or safely ignore) the new second argument.
|
||||
- New types added and exported: ICertProvisionEventComms, ICertificateIssuedEvent, ICertificateFailedEvent.
|
||||
- smart-proxy now constructs an eventComms channel that allows provisioners to log/warn/error and set expiry date and source for the issued event.
|
||||
- Emits 'certificate-issued' (domain, expiryDate, source, isRenewal?) on successful provisioning and 'certificate-failed' (domain, error, source) on failures.
|
||||
- Updated public exports to include the new types so they are available to consumers.
|
||||
- Removed readme.byte-counting-audit.md (documentation file deleted).
|
||||
|
||||
## 2026-02-13 - 24.0.1 - fix(proxy)
|
||||
improve proxy robustness: add connect timeouts, graceful shutdown, WebSocket watchdog, and metrics guard
|
||||
|
||||
|
||||
@@ -1,169 +0,0 @@
|
||||
# SmartProxy Byte Counting Audit Report
|
||||
|
||||
## Executive Summary
|
||||
|
||||
After a comprehensive audit of the SmartProxy codebase, I can confirm that **byte counting is implemented correctly** with no instances of double counting. Each byte transferred through the proxy is counted exactly once in each direction.
|
||||
|
||||
## Byte Counting Implementation
|
||||
|
||||
### 1. Core Tracking Mechanisms
|
||||
|
||||
SmartProxy uses two complementary tracking systems:
|
||||
|
||||
1. **Connection Records** (`IConnectionRecord`):
|
||||
- `bytesReceived`: Total bytes received from client
|
||||
- `bytesSent`: Total bytes sent to client
|
||||
|
||||
2. **MetricsCollector**:
|
||||
- Global throughput tracking via `ThroughputTracker`
|
||||
- Per-connection byte tracking for route/IP metrics
|
||||
- Called via `recordBytes(connectionId, bytesIn, bytesOut)`
|
||||
|
||||
### 2. Where Bytes Are Counted
|
||||
|
||||
Bytes are counted in only two files:
|
||||
|
||||
#### a) `route-connection-handler.ts`
|
||||
- **Line 351**: TLS alert bytes when no SNI is provided
|
||||
- **Lines 1286-1301**: Data forwarding callbacks in `setupBidirectionalForwarding()`
|
||||
|
||||
#### b) `http-proxy-bridge.ts`
|
||||
- **Line 127**: Initial TLS chunk for HttpProxy connections
|
||||
- **Lines 142-154**: Data forwarding callbacks in `setupBidirectionalForwarding()`
|
||||
|
||||
## Connection Flow Analysis
|
||||
|
||||
### 1. Direct TCP Connection (No TLS)
|
||||
|
||||
```
|
||||
Client → SmartProxy → Target Server
|
||||
```
|
||||
|
||||
1. Connection arrives at `RouteConnectionHandler.handleConnection()`
|
||||
2. For non-TLS ports, immediately routes via `routeConnection()`
|
||||
3. `setupDirectConnection()` creates target connection
|
||||
4. `setupBidirectionalForwarding()` handles all data transfer:
|
||||
- `onClientData`: `bytesReceived += chunk.length` + `recordBytes(chunk.length, 0)`
|
||||
- `onServerData`: `bytesSent += chunk.length` + `recordBytes(0, chunk.length)`
|
||||
|
||||
**Result**: ✅ Each byte counted exactly once
|
||||
|
||||
### 2. TLS Passthrough Connection
|
||||
|
||||
```
|
||||
Client (TLS) → SmartProxy → Target Server (TLS)
|
||||
```
|
||||
|
||||
1. Connection waits for initial data to detect TLS
|
||||
2. TLS handshake detected, SNI extracted
|
||||
3. Route matched, `setupDirectConnection()` called
|
||||
4. Initial chunk stored in `pendingData` (NOT counted yet)
|
||||
5. On target connect, `pendingData` written to target (still not counted)
|
||||
6. `setupBidirectionalForwarding()` counts ALL bytes including initial chunk
|
||||
|
||||
**Result**: ✅ Each byte counted exactly once
|
||||
|
||||
### 3. TLS Termination via HttpProxy
|
||||
|
||||
```
|
||||
Client (TLS) → SmartProxy → HttpProxy (localhost) → Target Server
|
||||
```
|
||||
|
||||
1. TLS connection detected with `tls.mode = "terminate"`
|
||||
2. `forwardToHttpProxy()` called:
|
||||
- Initial chunk: `bytesReceived += chunk.length` + `recordBytes(chunk.length, 0)`
|
||||
3. Proxy connection created to HttpProxy on localhost
|
||||
4. `setupBidirectionalForwarding()` handles subsequent data
|
||||
|
||||
**Result**: ✅ Each byte counted exactly once
|
||||
|
||||
### 4. HTTP Connection via HttpProxy
|
||||
|
||||
```
|
||||
Client (HTTP) → SmartProxy → HttpProxy (localhost) → Target Server
|
||||
```
|
||||
|
||||
1. Connection on configured HTTP port (`useHttpProxy` ports)
|
||||
2. Same flow as TLS termination
|
||||
3. All byte counting identical to TLS termination
|
||||
|
||||
**Result**: ✅ Each byte counted exactly once
|
||||
|
||||
### 5. NFTables Forwarding
|
||||
|
||||
```
|
||||
Client → [Kernel NFTables] → Target Server
|
||||
```
|
||||
|
||||
1. Connection detected, route matched with `forwardingEngine: 'nftables'`
|
||||
2. Connection marked as `usingNetworkProxy = true`
|
||||
3. NO application-level forwarding (kernel handles packet routing)
|
||||
4. NO byte counting in application layer
|
||||
|
||||
**Result**: ✅ No counting (correct - kernel handles everything)
|
||||
|
||||
## Special Cases
|
||||
|
||||
### PROXY Protocol
|
||||
- PROXY protocol headers sent to backend servers are NOT counted in client metrics
|
||||
- Only actual client data is counted
|
||||
- **Correct behavior**: Protocol overhead is not client data
|
||||
|
||||
### TLS Alerts
|
||||
- TLS alerts (e.g., for missing SNI) are counted as sent bytes
|
||||
- **Correct behavior**: Alerts are actual data sent to the client
|
||||
|
||||
### Initial Chunks
|
||||
- **Direct connections**: Stored in `pendingData`, counted when forwarded
|
||||
- **HttpProxy connections**: Counted immediately upon receipt
|
||||
- **Both approaches**: Count each byte exactly once
|
||||
|
||||
## Verification Methodology
|
||||
|
||||
1. **Code Analysis**: Searched for all instances of:
|
||||
- `bytesReceived +=` and `bytesSent +=`
|
||||
- `recordBytes()` calls
|
||||
- Data forwarding implementations
|
||||
|
||||
2. **Flow Tracing**: Followed data path for each connection type from entry to exit
|
||||
|
||||
3. **Handler Review**: Examined all forwarding handlers to ensure no additional counting
|
||||
|
||||
## Findings
|
||||
|
||||
### ✅ No Double Counting Detected
|
||||
|
||||
- Each byte is counted exactly once in the direction it flows
|
||||
- Connection records and metrics are updated consistently
|
||||
- No overlapping or duplicate counting logic found
|
||||
|
||||
### Areas of Excellence
|
||||
|
||||
1. **Centralized Counting**: All byte counting happens in just two files
|
||||
2. **Consistent Pattern**: Uses `setupBidirectionalForwarding()` with callbacks
|
||||
3. **Clear Separation**: Forwarding handlers don't interfere with proxy metrics
|
||||
|
||||
## Recommendations
|
||||
|
||||
1. **Debug Logging**: Add optional debug logging to verify byte counts in production:
|
||||
```typescript
|
||||
if (settings.debugByteCount) {
|
||||
logger.log('debug', `Bytes counted: ${connectionId} +${bytes} (total: ${record.bytesReceived})`);
|
||||
}
|
||||
```
|
||||
|
||||
2. **Unit Tests**: Create specific tests to ensure byte counting accuracy:
|
||||
- Test initial chunk handling
|
||||
- Test PROXY protocol overhead exclusion
|
||||
- Test HttpProxy forwarding accuracy
|
||||
|
||||
3. **Protocol Overhead Tracking**: Consider separately tracking:
|
||||
- PROXY protocol headers
|
||||
- TLS handshake bytes
|
||||
- HTTP headers vs body
|
||||
|
||||
4. **NFTables Documentation**: Clearly document that NFTables-forwarded connections are not included in application metrics
|
||||
|
||||
## Conclusion
|
||||
|
||||
SmartProxy's byte counting implementation is **robust and accurate**. The design ensures that each byte is counted exactly once, with clear separation between connection tracking and metrics collection. No remediation is required.
|
||||
@@ -3,6 +3,6 @@
|
||||
*/
|
||||
export const commitinfo = {
|
||||
name: '@push.rocks/smartproxy',
|
||||
version: '24.0.1',
|
||||
version: '25.0.0',
|
||||
description: 'A powerful proxy package with unified route-based configuration for high traffic management. Features include SSL/TLS support, flexible routing patterns, WebSocket handling, advanced security options, and automatic ACME certificate management.'
|
||||
}
|
||||
|
||||
@@ -8,7 +8,7 @@ export { SharedRouteManager as RouteManager } from './core/routing/route-manager
|
||||
|
||||
// Export smart-proxy models
|
||||
export type { ISmartProxyOptions, IConnectionRecord, IRouteConfig, IRouteMatch, IRouteAction, IRouteTls, IRouteContext } from './proxies/smart-proxy/models/index.js';
|
||||
export type { TSmartProxyCertProvisionObject } from './proxies/smart-proxy/models/interfaces.js';
|
||||
export type { TSmartProxyCertProvisionObject, ICertProvisionEventComms, ICertificateIssuedEvent, ICertificateFailedEvent } from './proxies/smart-proxy/models/interfaces.js';
|
||||
export * from './proxies/smart-proxy/utils/index.js';
|
||||
|
||||
// Original: export * from './smartproxy/classes.pp.snihandler.js'
|
||||
|
||||
@@ -2,6 +2,6 @@
|
||||
* SmartProxy models
|
||||
*/
|
||||
// Export everything except IAcmeOptions from interfaces
|
||||
export type { ISmartProxyOptions, ISmartProxyCertStore, IConnectionRecord, TSmartProxyCertProvisionObject } from './interfaces.js';
|
||||
export type { ISmartProxyOptions, ISmartProxyCertStore, IConnectionRecord, TSmartProxyCertProvisionObject, ICertProvisionEventComms, ICertificateIssuedEvent, ICertificateFailedEvent } from './interfaces.js';
|
||||
export * from './route-types.js';
|
||||
export * from './metrics-types.js';
|
||||
|
||||
@@ -34,6 +34,38 @@ import type { IRouteConfig } from './route-types.js';
|
||||
*/
|
||||
export type TSmartProxyCertProvisionObject = plugins.tsclass.network.ICert | 'http01';
|
||||
|
||||
/**
|
||||
* Communication channel passed as second argument to certProvisionFunction.
|
||||
* Allows the callback to report metadata back to SmartProxy for event emission.
|
||||
*/
|
||||
export interface ICertProvisionEventComms {
|
||||
/** Informational log */
|
||||
log: (message: string) => void;
|
||||
/** Warning (non-fatal) */
|
||||
warn: (message: string) => void;
|
||||
/** Error */
|
||||
error: (message: string) => void;
|
||||
/** Set the certificate expiry date (for the issued event) */
|
||||
setExpiryDate: (date: Date) => void;
|
||||
/** Set the source/method used for provisioning (e.g. 'smartacme-dns-01') */
|
||||
setSource: (source: string) => void;
|
||||
}
|
||||
|
||||
/** Payload for 'certificate-issued' and 'certificate-renewed' events */
|
||||
export interface ICertificateIssuedEvent {
|
||||
domain: string;
|
||||
expiryDate?: string; // ISO 8601
|
||||
source: string; // e.g. 'certProvisionFunction', 'smartacme-dns-01'
|
||||
isRenewal?: boolean;
|
||||
}
|
||||
|
||||
/** Payload for 'certificate-failed' event */
|
||||
export interface ICertificateFailedEvent {
|
||||
domain: string;
|
||||
error: string;
|
||||
source: string;
|
||||
}
|
||||
|
||||
// Legacy options and type checking functions have been removed
|
||||
|
||||
/**
|
||||
@@ -140,7 +172,7 @@ export interface ISmartProxyOptions {
|
||||
* Optional certificate provider callback. Return 'http01' to use HTTP-01 challenges,
|
||||
* or a static certificate object for immediate provisioning.
|
||||
*/
|
||||
certProvisionFunction?: (domain: string) => Promise<TSmartProxyCertProvisionObject>;
|
||||
certProvisionFunction?: (domain: string, eventComms: ICertProvisionEventComms) => Promise<TSmartProxyCertProvisionObject>;
|
||||
|
||||
/**
|
||||
* Whether to fallback to ACME if custom certificate provision fails.
|
||||
|
||||
@@ -14,7 +14,7 @@ import { generateDefaultCertificate } from './utils/default-cert-generator.js';
|
||||
import { Mutex } from './utils/mutex.js';
|
||||
|
||||
// Types
|
||||
import type { ISmartProxyOptions, TSmartProxyCertProvisionObject, IAcmeOptions } from './models/interfaces.js';
|
||||
import type { ISmartProxyOptions, TSmartProxyCertProvisionObject, IAcmeOptions, ICertProvisionEventComms, ICertificateIssuedEvent, ICertificateFailedEvent } from './models/interfaces.js';
|
||||
import type { IRouteConfig } from './models/route-types.js';
|
||||
import type { IMetrics } from './models/metrics-types.js';
|
||||
|
||||
@@ -420,8 +420,21 @@ export class SmartProxy extends plugins.EventEmitter {
|
||||
for (const domain of certDomains) {
|
||||
if (provisionedDomains.has(domain)) continue;
|
||||
provisionedDomains.add(domain);
|
||||
|
||||
// Build eventComms channel for this domain
|
||||
let expiryDate: string | undefined;
|
||||
let source = 'certProvisionFunction';
|
||||
|
||||
const eventComms: ICertProvisionEventComms = {
|
||||
log: (msg) => logger.log('info', `[certProvision ${domain}] ${msg}`, { component: 'smart-proxy' }),
|
||||
warn: (msg) => logger.log('warn', `[certProvision ${domain}] ${msg}`, { component: 'smart-proxy' }),
|
||||
error: (msg) => logger.log('error', `[certProvision ${domain}] ${msg}`, { component: 'smart-proxy' }),
|
||||
setExpiryDate: (date) => { expiryDate = date.toISOString(); },
|
||||
setSource: (s) => { source = s; },
|
||||
};
|
||||
|
||||
try {
|
||||
const result: TSmartProxyCertProvisionObject = await provisionFn(domain);
|
||||
const result: TSmartProxyCertProvisionObject = await provisionFn(domain, eventComms);
|
||||
|
||||
if (result === 'http01') {
|
||||
// Callback wants HTTP-01 for this domain — trigger Rust ACME explicitly
|
||||
@@ -455,10 +468,24 @@ export class SmartProxy extends plugins.EventEmitter {
|
||||
logger.log('warn', `certStore.save() failed for ${domain}: ${storeErr.message}`, { component: 'smart-proxy' });
|
||||
}
|
||||
}
|
||||
|
||||
// Emit certificate-issued event
|
||||
this.emit('certificate-issued', {
|
||||
domain,
|
||||
expiryDate: expiryDate || (certObj.validUntil ? new Date(certObj.validUntil).toISOString() : undefined),
|
||||
source,
|
||||
} satisfies ICertificateIssuedEvent);
|
||||
}
|
||||
} catch (err: any) {
|
||||
logger.log('warn', `certProvisionFunction failed for ${domain}: ${err.message}`, { component: 'smart-proxy' });
|
||||
|
||||
// Emit certificate-failed event
|
||||
this.emit('certificate-failed', {
|
||||
domain,
|
||||
error: err.message,
|
||||
source,
|
||||
} satisfies ICertificateFailedEvent);
|
||||
|
||||
// Fallback to ACME if enabled and route has a name
|
||||
if (this.settings.certProvisionFallbackToAcme !== false && route.name) {
|
||||
try {
|
||||
|
||||
Reference in New Issue
Block a user