Skip to content

Commit 8ee2020

Browse files
authored
fix(ldap): [OCISDEV-1031] restore pinned-CA-only trust for LDAP TLS (#674)
tlsConfigFromLDAPConn's CA-cert branch built its RootCAs pool from x509.SystemCertPool() plus the configured CA, so configuring a CA cert also trusted every system-level CA instead of just the pinned one — a trust-scope widening versus the pre-pool graph service behavior (x509.NewCertPool()). It also ignored AppendCertsFromPEM's return value, silently falling back to an unrestricted trust store on a malformed/empty CA file instead of failing initialization. Use x509.NewCertPool() and fail when the PEM contains no valid certificates, matching the behavior callers relied on before this helper was introduced in #658. Flagged during review of owncloud/ocis#12660. Signed-off-by: Lukas Hirt <info@hirt.cz>
1 parent c38fd3b commit 8ee2020

2 files changed

Lines changed: 16 additions & 2 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
Bugfix: Restore pinned-CA-only trust and PEM validation for LDAP TLS
2+
3+
`tlsConfigFromLDAPConn`'s CA-cert branch built its `RootCAs` pool from
4+
`x509.SystemCertPool()` plus the configured CA, so callers configuring a CA cert
5+
ended up trusting every system-level CA as well, not just the pinned one — a
6+
trust-scope widening versus the pre-pool behavior. It also ignored
7+
`AppendCertsFromPEM`'s return value, so a malformed or empty CA file would
8+
silently fall back to an unrestricted trust store instead of failing
9+
initialization. Use `x509.NewCertPool()` (pinned CA only) and fail
10+
`tlsConfigFromLDAPConn` when the PEM contains no valid certificates.
11+
12+
https://github.com/owncloud/reva/pull/658

pkg/utils/ldap.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,10 @@ func tlsConfigFromLDAPConn(c *LDAPConn) (*tls.Config, error) {
6464
if err != nil {
6565
return nil, errors.Wrapf(err, "Error reading LDAP CA Cert '%s.'", c.CACert)
6666
}
67-
rpool, _ := x509.SystemCertPool()
68-
rpool.AppendCertsFromPEM(pemBytes)
67+
rpool := x509.NewCertPool()
68+
if !rpool.AppendCertsFromPEM(pemBytes) {
69+
return nil, errors.Errorf("Error adding LDAP CA Cert '%s': no valid certificates found", c.CACert)
70+
}
6971
return &tls.Config{
7072
MinVersion: tls.VersionTLS12,
7173
RootCAs: rpool,

0 commit comments

Comments
 (0)