-
Notifications
You must be signed in to change notification settings - Fork 78
Adjust Tenable integration by requiring auth and validating host URLs #8510
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 16 commits
53cfe2d
f2d6a86
6dd3ada
a1322ae
e68e3e5
afcda97
c452f75
6c40c83
13ca3c3
1569459
3d3c39b
7d4eebf
c685679
0e0ab24
694ab1d
71bbddd
d0fbc3b
77c178f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we'll also probably need to update the heimdall helm chart
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we'll also probably need to update the tenable integration part of the integrations wiki page |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,11 +27,11 @@ | |
| } | ||
|
|
||
| set(key: string, value: string | undefined): void { | ||
| this.envConfig[key] = value; | ||
| } | ||
|
|
||
| get(key: string): string | undefined { | ||
| return process.env[key] || this.envConfig[key]; | ||
|
Check warning on line 34 in apps/backend/config/app_config.ts
|
||
| } | ||
|
|
||
| getExternalUrl(): string { | ||
|
|
@@ -52,13 +52,44 @@ | |
| } | ||
| } | ||
|
|
||
| getTenableHostUrl(): string { | ||
| // Newline-separated allowlist of Tenable.SC hosts for the login/proxy endpoints | ||
| // Entries must include a protocol and may include a port; malformed entries throw at startup. | ||
| getTenableHostUrl(): string[] { | ||
| const tenable_host_url = this.get('TENABLE_HOST_URL'); | ||
| if (tenable_host_url !== undefined) { | ||
| return tenable_host_url; | ||
| } else { | ||
| return ''; | ||
| if (tenable_host_url === undefined) { | ||
| return []; | ||
| } | ||
| return tenable_host_url | ||
| .split('\n') | ||
| .map((url) => url.trim()) | ||
| .filter((url) => url.length > 0) | ||
| .map((url) => { | ||
| let parsed: URL; | ||
| try { | ||
| parsed = new URL(url); | ||
| } catch { | ||
| throw new Error( | ||
| `Invalid TENABLE_HOST_URL entry "${url}": must be a complete URL including protocol (e.g. https://example.com)` | ||
| ); | ||
| } | ||
| if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { | ||
| throw new Error( | ||
| `Invalid TENABLE_HOST_URL entry "${url}": protocol must be http or https` | ||
| ); | ||
| } | ||
| // Only protocol + hostname + port are allowed; ports are permitted since | ||
| // Tenable.SC may run on a non-default port. | ||
| const hasExtra = | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. both of the hasExtra processing areas need to exclude everything aside from hostname + protocol + optionally port considering that this processing code is the same, maybe we can extract out to some common utility function and use it in both places. |
||
| (parsed.pathname !== '' && parsed.pathname !== '/') || | ||
| parsed.search !== '' || | ||
| parsed.hash !== ''; | ||
| if (hasExtra) { | ||
| throw new Error( | ||
| `Invalid TENABLE_HOST_URL entry "${url}": must contain only a protocol, hostname, and optional port (no path, query, or fragment)` | ||
| ); | ||
| } | ||
| return parsed.origin; | ||
|
DMedina6 marked this conversation as resolved.
|
||
| }); | ||
| } | ||
|
|
||
| getDatabaseName(): string { | ||
|
|
@@ -91,8 +122,8 @@ | |
| sslKey = this.get('DATABASE_SSL_KEY'); | ||
| } else { | ||
| // Verify file exists | ||
| if (fs.statSync(this.get('DATABASE_SSL_KEY')!).isFile()) { | ||
| sslKey = fs.readFileSync(this.get('DATABASE_SSL_KEY')!); | ||
| } else { | ||
| throw new Error('SSL Key file does not exist'); | ||
| } | ||
|
|
@@ -104,8 +135,8 @@ | |
| sslCert = this.get('DATABASE_SSL_CERT'); | ||
| } else { | ||
| // Verify file exists | ||
| if (fs.statSync(this.get('DATABASE_SSL_CERT')!).isFile()) { | ||
| sslCert = fs.readFileSync(this.get('DATABASE_SSL_CERT')!); | ||
| } else { | ||
| throw new Error('SSL Cert file does not exist'); | ||
| } | ||
|
|
@@ -117,8 +148,8 @@ | |
| sslCA = this.get('DATABASE_SSL_CA'); | ||
| } else { | ||
| // Verify file exists | ||
| if (fs.statSync(this.get('DATABASE_SSL_CA')!).isFile()) { | ||
| sslCA = fs.readFileSync(this.get('DATABASE_SSL_CA')!); | ||
| } else { | ||
| throw new Error('SSL CA file does not exist'); | ||
| } | ||
|
|
@@ -162,7 +193,7 @@ | |
| return false; | ||
| } else { | ||
| const pattern = | ||
| /^(?:([^:\/?#\s]+):\/{2})?(?:([^@\/?#\s]+)@)?([^\/?#\s]+)?(?:\/([^?#\s]*))?(?:[?]([^#\s]+))?\S*$/; | ||
| const matches = url.match(pattern); | ||
|
|
||
| if (matches === null) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we'll also need to update the envvar wiki page