hyperledger / hyperledger/fabric-x
fxconfig: TLSConfig.Validate silently accepts a half-configured mTLS setup and config validation has no unit tests
- Dominant language
- Go
- Stars
- 64
- Forks
- 80
- Avg merge
- 1d 22h
- Merged PRs (30d)
- 15
Description
While exploring the fxconfig codebase I noticed two related gaps in `tools/fxconfig/internal/config/validate.go` that I think are worth fixing together.
## The mTLS partial-config problem
`TLSConfig.Validate` has this guard for mTLS:
```go
// mTLS
if c.ClientCertPath == "" && c.ClientKeyPath == "" {
return nil
}
```
This only skips mTLS validation if **both** fields are empty. If a user accidentally sets only `clientCertPath` (forgetting `clientKeyPath`), the code falls through and eventually hits:
```go
if _, err := tls.LoadX509KeyPair(c.ClientCertPath, c.ClientKeyPath); err != nil {
return fmt.Errorf("invalid cert/key pair: %w", err)
}
```
With an empty key path that becomes something like `open : no such file or directory` — which gives the user no hint that they need to set *both* fields. An explicit XOR check early in the mTLS block would catch this and give a clear message like `clientCertPath and clientKeyPath must both be set or both be empty`.
## The test coverage gap
`validate_test.go` currently only has `TestErrorIfEmpty`. None of the actual Validate methods — `MSPConfig.Validate`, `OrdererConfig.Validate`, `EndpointServiceConfig.Validate`, `TLSConfig.Validate`, or `validateEndpoint` — have any direct unit tests. These functions have real branching logic (disabled vs. server TLS vs. mTLS, bad endpoint format, zero timeout, etc.) that deserves coverage.
## What I'd like to do
- Add the XOR guard for the mTLS pair in `TLSConfig.Validate`
- Add unit tests for all the Validate methods and `validateEndpoint`
Happy to put up a PR if this looks good to the maintainers.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.