feat: enable CIRA connection in console #133
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #133 +/- ##
==========================================
- Coverage 36.51% 35.57% -0.95%
==========================================
Files 55 56 +1
Lines 4017 4124 +107
==========================================
Hits 1467 1467
- Misses 2458 2565 +107
Partials 92 92 ☔ View full report in Codecov by Sentry. |
rsdmike
force-pushed
the
main
branch
5 times, most recently
from
e0ea0e5 to
62fb1d5
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| func (s *Server) ListenAndServe() error { | ||
| config := &tls.Config{ | ||
| Certificates: []tls.Certificate{s.certificates}, | ||
| InsecureSkipVerify: true, |
Check failure
Code scanning / CodeQL
Disabled TLS certificate check High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 15 days ago
The best way to fix this problem is to remove the InsecureSkipVerify: true setting from the TLS configuration on line 73 of ListenAndServe, ensuring that client certificates are properly validated. If certificate verification must be skipped in test environments, introduce a configuration/parameter/environment variable that determines whether to set InsecureSkipVerify: true, and default to verification in all production scenarios.
For the shown code, simply remove the line setting InsecureSkipVerify: true in the TLS config. If the application requires custom certificate validation for compatibility with Intel AMT or similar, consider handling validation using a custom verification function via the VerifyPeerCertificate field, but do not skip verification globally.
Change needed:
- Edit file
internal/controller/tcp/cira/tunnel.go, remove or comment out the lineInsecureSkipVerify: true,in the TLS configuration inside theListenAndServefunction.
No new methods, definitions, or imports are needed.
| @@ -70,7 +70,7 @@ | ||
| config := &tls.Config{ | ||
| Certificates: []tls.Certificate{s.certificates}, | ||
| // ClientAuth: tls.NoClientCert, | ||
| InsecureSkipVerify: true, | ||
| // InsecureSkipVerify: true, // Removed for security. Do not skip certificate verification. | ||
| MinVersion: tls.VersionTLS12, | ||
| } | ||
|
|
| // Parse the authentication details from the original data | ||
| username, password, parseErr := s.parseAuthRequest(data) | ||
| if parseErr != nil { | ||
| log.Printf("Failed to parse authentication request: %v\n", parseErr) |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 15 days ago
To fix the problem, we should ensure that sensitive data (such as the user-supplied password length) is not written in clear text to application logs, even as part of an error message. Instead, we should replace explicit values with a generic placeholder when constructing error strings related to parsing password authentication. Specifically, inside parseAuthRequest, all error messages that mention the value of the password length should be modified to avoid exposing this number; we can simply state "password too long" without including the actual length or, optionally, log only whether the value appears malformed.
The change should be made only to error construction inside parseAuthRequest — particularly the error case for password length on line 330 and any similar fields involving sensitive data. Only the relevant error string needs to be modified; the rest of the parsing logic remains unchanged.
No new imports or external libraries are needed for this fix.
| @@ -327,7 +327,7 @@ | ||
| } | ||
|
|
||
| if passwordLen > 256 { | ||
| return "", "", fmt.Errorf("password too long: %d", passwordLen) | ||
| return "", "", fmt.Errorf("password too long") | ||
| } | ||
|
|
||
| passwordBytes := make([]byte, passwordLen) |
addresses Add Support for CIRA Fixes #665
1 participant
No description provided.