fix(xcecli): restore dangling CA-cert cleanup misses, enable https support
bug-1463: commit 94f50ce6 (bug-1453 CA-cert-plumbing removal) missed
GetJobOutput's dead tls.Config/loadRootCAs block in request.go, plus
peek.go's SetRootCAs call and serverInfo.CACert field access (the
field no longer exists on APIServerInfo) -- broke the xcecli build.
Removed all three dangling references. Also enabled NewAPIClient's
previously-commented-out InsecureSkipVerify TLS config, since no
xcecli method set any TLS config at all -- any https:// apiserver URL
would have failed cert verification, not just GetJobOutput. Matches
bug-1453's documented design (server-side TLS only, no peer
verification, mTLS deferred as P3 hardening).
Documented both the api-server (http+https) and controller
(server-side-cert-only, no client certs) TLS postures in
.kiro/steering/security.md, replacing a stale line claiming TLS is
handled at the network layer, plus a follow-up note for the planned
HAProxy TLS-termination reverse proxy once its cert type and
apiserver's own listener fate are decided.
This commit is contained in:
parent
ff1be48001
commit
4ec401672b
3 changed files with 17 additions and 10 deletions
|
|
@ -58,4 +58,12 @@ Rate limiting (`middleware/rate_limit.go`) and IP whitelist (`middleware/ip_whit
|
||||||
|
|
||||||
## Transport
|
## Transport
|
||||||
|
|
||||||
All daemon-to-daemon communication is over TCP. Production deployments add TLS at the network layer (not handled in application code). The WAL and replication wire format uses Cap'n Proto — schema files in `src/schema/`. Do not add unvalidated deserialization paths for external input.
|
All daemon-to-daemon communication is over TCP. The WAL and replication wire format uses Cap'n Proto — schema files in `src/schema/`. Do not add unvalidated deserialization paths for external input.
|
||||||
|
|
||||||
|
XCE uses **server-side TLS only** — encryption without peer certificate verification. There is no mTLS and no CA-pinning anywhere in the system; peer verification is deferred as optional P3 hardening (see bug-1453, which deleted all CA-cert/client-cert plumbing from both the C tier and the Go tier as dead code). Do not reintroduce CA-cert loading, client certificates, or cert-pinning without a design doc — this posture is intentional, not an oversight.
|
||||||
|
|
||||||
|
**Clients talking to the API server** (xcecli, or any HTTP client of `apiserver`) must support **both `http://` and `https://`** — the scheme is a per-deployment/user choice (`APIServerInfo.URL`), not something the client hardcodes. When the scheme is `https://`, set `InsecureSkipVerify: true` on the TLS client config (see `xcecli/common/request/request.go`'s `NewAPIClient`) rather than attempting cert validation — the API server's cert is typically self-signed/auto-generated and the client has no CA to validate it against.
|
||||||
|
|
||||||
|
**Clients talking to an XCE controller directly** (worker, scheduler, and any other libxce-based client via `cluster.c`'s `do_connect()`) need **only server-side certs — no client-side certs are required or supported**. `xce_cluster_config_t` has no `ca_cert_path`/client-cert fields; do not add any. TLS verification mode is `SSL_VERIFY_NONE` throughout — this matches the API-server posture above, just at the C/libxce layer instead of Go/HTTP.
|
||||||
|
|
||||||
|
**Known follow-up (not yet scoped):** a HAProxy reverse proxy in front of `apiserver` is planned to terminate HTTPS, likely with a CA-trusted cert (Let's Encrypt candidate, unconfirmed), and whether `apiserver`'s own `RunTLS`/`httpsConfig.Enabled` listener stays in the picture is also undecided. Once both are settled, revisit xcecli's blanket `InsecureSkipVerify: true`: if the client-facing endpoint ends up CA-trusted, default to real verification (Go's default `tls.Config{}` validates against the system trust store with no extra plumbing) and make skip-verify an explicit opt-in for self-signed deployments only — matching the automated security review's suggested fix. Don't implement this speculatively; the right shape depends on which of those two decisions lands where.
|
||||||
|
|
|
||||||
|
|
@ -84,7 +84,6 @@ func (q *PeekQuerier) Run(jobID string) error {
|
||||||
}
|
}
|
||||||
|
|
||||||
apiClient := request.NewAPIClient(serverInfo.URL)
|
apiClient := request.NewAPIClient(serverInfo.URL)
|
||||||
apiClient.SetRootCAs(serverInfo.CACert)
|
|
||||||
|
|
||||||
// Determine stream type
|
// Determine stream type
|
||||||
streamType := "stdout"
|
streamType := "stdout"
|
||||||
|
|
@ -119,7 +118,7 @@ func (q *PeekQuerier) Run(jobID string) error {
|
||||||
}
|
}
|
||||||
|
|
||||||
// Fetch output
|
// Fetch output
|
||||||
resp, err := apiClient.GetJobOutput(serverInfo.Token, serverInfo.CACert, apiJobID, params)
|
resp, err := apiClient.GetJobOutput(serverInfo.Token, apiJobID, params)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,7 @@
|
||||||
package request
|
package request
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"crypto/tls"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
|
||||||
|
|
@ -39,7 +40,11 @@ func NewAPIClient(baseURL string) *APIClient {
|
||||||
}
|
}
|
||||||
//c.Client.SetDebug(true)
|
//c.Client.SetDebug(true)
|
||||||
c.Client.SetTimeout(10 * time.Second)
|
c.Client.SetTimeout(10 * time.Second)
|
||||||
//c.client.SetTLSClientConfig(&tls.Config{InsecureSkipVerify: true})
|
// XCE uses server-side TLS only (encryption without peer certificate
|
||||||
|
// verification, see bug-1453) -- the API server's cert is typically
|
||||||
|
// self-signed/auto-generated, so skip verification rather than require
|
||||||
|
// a CA the client has no way to obtain.
|
||||||
|
c.Client.SetTLSClientConfig(&tls.Config{InsecureSkipVerify: true})
|
||||||
return c
|
return c
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -136,14 +141,9 @@ func (c *APIClient) SubmitJob(token string, req *models.JobSubmitRequest) (*mode
|
||||||
}
|
}
|
||||||
|
|
||||||
// GetJobOutput retrieves job output from the API server.
|
// GetJobOutput retrieves job output from the API server.
|
||||||
func (c *APIClient) GetJobOutput(token, certPath, jobID string, params map[string]string) (*models.JobOutputResponse, error) {
|
func (c *APIClient) GetJobOutput(token, jobID string, params map[string]string) (*models.JobOutputResponse, error) {
|
||||||
var resp models.JobOutputResponse
|
var resp models.JobOutputResponse
|
||||||
baseURL := c.BaseURL
|
baseURL := c.BaseURL
|
||||||
if strings.HasPrefix(baseURL, "https://") {
|
|
||||||
c.Client.SetTLSClientConfig(&tls.Config{
|
|
||||||
RootCAs: loadRootCAs(certPath),
|
|
||||||
})
|
|
||||||
}
|
|
||||||
if idx := strings.Index(baseURL, "/xce/v1"); idx != -1 {
|
if idx := strings.Index(baseURL, "/xce/v1"); idx != -1 {
|
||||||
baseURL = baseURL[:idx]
|
baseURL = baseURL[:idx]
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue