Skip to content

Commit aefb47f

Browse files
shjalaeriknordmark
authored andcommitted
Load TLS root CA directly from /config instead of /persist/certs
This change fixes the security issue where TLS root certificates were copied from the integrity-protected /config partition to the mutable /persist/certs directory and referenced via SHA256 indirection. Changes: - Remove certificate copy logic from InitializeCertDir() - now only creates the certs directory without copying files - Update GetTLSConfig() to read certificates directly from types.V2TLSBaseFile (/config/v2tlsbaseroot-certificates.pem) - Update UpdateTLSProxyCerts() to read from V2TLSBaseFile instead of V2TLSCertShaFilename - Remove V2TLSCertShaFilename constant from locationconsts.go as it's no longer needed - Update getSecurityInfo() in handlemetrics.go to compute SHA256 hash directly from the V2TLSBaseFile content instead of reading a pre-computed SHA from persist Signed-off-by: Shahriyar Jalayeri <shahriyar@posteo.de>
1 parent 37aed60 commit aefb47f

File tree

3 files changed

+15
-84
lines changed

3 files changed

+15
-84
lines changed

pkg/pillar/cmd/zedagent/handlemetrics.go

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ package zedagent
88
import (
99
"bytes"
1010
"crypto/sha256"
11-
"encoding/hex"
1211
"fmt"
12+
"io"
1313
"net"
1414
"net/http"
1515
"os"
@@ -898,19 +898,16 @@ func getSecurityInfo(ctx *zedagentContext) *info.SecurityInfo {
898898
hasher.Write(caCert1)
899899
si.ShaRootCa = hasher.Sum(nil)
900900
}
901-
// Add the sha of the root CAs used for TLS
902-
// Note that we have the sha in a logical symlink so we
903-
// just read that file.
904-
line, err := os.ReadFile(types.V2TLSCertShaFilename)
901+
// Add the sha of the root CAs used for TLS, loaded from config directory (integrity protected)
902+
f, err := os.Open(types.V2TLSBaseFile)
905903
if err != nil {
906904
log.Error(err)
907905
} else {
908-
shaStr := strings.TrimSpace(string(line))
909-
sha, err := hex.DecodeString(shaStr)
910-
if err != nil {
911-
log.Errorf("DecodeString %s failed: %s", shaStr, err)
906+
h := sha256.New()
907+
if _, err := io.Copy(h, f); err != nil {
908+
log.Errorf("failed to get sha256 of %s: %v", types.V2TLSBaseFile, err)
912909
} else {
913-
si.ShaTlsRootCa = sha
910+
si.ShaTlsRootCa = h.Sum(nil)
914911
}
915912
}
916913
log.Tracef("getSecurityInfo returns %+v", si)

pkg/pillar/controllerconn/tls.go

Lines changed: 8 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,11 @@ package controllerconn
77

88
import (
99
"bytes"
10-
"crypto/sha256"
1110
"crypto/tls"
1211
"crypto/x509"
1312
"encoding/pem"
1413
"errors"
1514
"fmt"
16-
"io"
1715
"os"
1816
"path/filepath"
1917
"strconv"
@@ -28,52 +26,14 @@ import (
2826
)
2927

3028
// InitializeCertDir is called by zedbox to make sure we have the initial
31-
// files in /persist/certs from /config/ under a sha-based name.
32-
// Also, the currently used base file is indicated by the content of
33-
// /persist/certs/v2tlsbaseroot-certificates.sha256. This is to prepare for a
34-
// future feature where the controller can update the base file.
35-
// Note that programmatically we add any proxy certificates to the list of roots
36-
// we trust separately from the file content.
29+
// certs directory created.
3730
func InitializeCertDir(log *base.LogObject) error {
3831
if _, err := os.Stat(types.CertificateDirname); err != nil {
3932
log.Tracef("Create %s", types.CertificateDirname)
4033
if err := os.MkdirAll(types.CertificateDirname, 0700); err != nil {
4134
return err
4235
}
4336
}
44-
f, err := os.Open(types.V2TLSBaseFile)
45-
if err != nil {
46-
return err
47-
}
48-
h := sha256.New()
49-
if _, err := io.Copy(h, f); err != nil {
50-
err = fmt.Errorf("Failed sha256 of %s: %w",
51-
types.V2TLSBaseFile, err)
52-
return err
53-
}
54-
sha := fmt.Sprintf("%x", h.Sum(nil))
55-
// Copy base file to /persist/certs/<sha> if not exist or zero length
56-
dstfile := fmt.Sprintf("%s/%s", types.CertificateDirname, sha)
57-
st, err := os.Stat(dstfile)
58-
if err != nil || st.Size() == 0 {
59-
log.Noticef("Adding /config/v2tlsbaseroot-certificates.pem to %s",
60-
dstfile)
61-
err := fileutils.CopyFile("/config/v2tlsbaseroot-certificates.pem", dstfile)
62-
if err != nil {
63-
return err
64-
}
65-
}
66-
// Write sha to types.V2TLSCertShaFilename if not exist or zero length
67-
dstfile = types.V2TLSCertShaFilename
68-
st, err = os.Stat(dstfile)
69-
if err != nil || st.Size() == 0 {
70-
log.Noticef("Setting /config/v2tlsbaseroot-certificates.pem as current")
71-
line := sha + "\n"
72-
err = fileutils.WriteRename(dstfile, []byte(line))
73-
if err != nil {
74-
return err
75-
}
76-
}
7737
return nil
7838
}
7939

@@ -143,25 +103,14 @@ func (c *Client) GetTLSConfig(clientCert *tls.Certificate) (*tls.Config, error)
143103
caCertPool := x509.NewCertPool()
144104

145105
if c.v2API {
146-
// Load the well-known CAs
147-
line, err := os.ReadFile(types.V2TLSCertShaFilename)
148-
if err != nil {
149-
return nil, err
150-
}
151-
sha := strings.TrimSpace(string(line))
152-
if len(sha) == 0 {
153-
errStr := fmt.Sprintf("Read zero byte from sha file")
154-
c.log.Error(errStr)
155-
return nil, errors.New(errStr)
156-
}
157-
v2RootFilename := types.CertificateDirname + "/" + sha
158-
caCert, err := os.ReadFile(v2RootFilename)
106+
// Load the well-known CAs from config directory (integrity protected)
107+
caCert, err := os.ReadFile(types.V2TLSBaseFile)
159108
if err != nil {
160109
return nil, err
161110
}
162111
if !caCertPool.AppendCertsFromPEM(caCert) {
163112
errStr := fmt.Sprintf("Failed to append certs from %s",
164-
v2RootFilename)
113+
types.V2TLSBaseFile)
165114
c.log.Error(errStr)
166115
return nil, errors.New(errStr)
167116
}
@@ -295,30 +244,17 @@ func (c *Client) UpdateTLSProxyCerts() bool {
295244

296245
var caCertPool *x509.CertPool
297246
if len(c.prevCertPEM) > 0 {
298-
299247
// previous certs we have are different, lets rebuild from beginning
300248
caCertPool = x509.NewCertPool()
301-
line, err := os.ReadFile(types.V2TLSCertShaFilename)
302-
if err != nil {
303-
errStr := fmt.Sprintf("Failed to read V2TLSCertShaFilename")
304-
c.log.Error(errStr)
305-
return false
306-
}
307-
sha := strings.TrimSpace(string(line))
308-
if len(sha) == 0 {
309-
errStr := fmt.Sprintf("Read zero byte from sha file")
310-
c.log.Error(errStr)
311-
return false
312-
}
313-
v2RootFilename := types.CertificateDirname + "/" + sha
314-
caCert, err := os.ReadFile(v2RootFilename)
249+
// Load the well-known CAs from config directory (integrity protected)
250+
caCert, err := os.ReadFile(types.V2TLSBaseFile)
315251
if err != nil {
316-
errStr := fmt.Sprintf("Failed to read v2RootFilename")
252+
errStr := fmt.Sprintf("Failed to read %s", types.V2TLSBaseFile)
317253
c.log.Error(errStr)
318254
return false
319255
}
320256
if !caCertPool.AppendCertsFromPEM(caCert) {
321-
errStr := fmt.Sprintf("Failed to append certs from %s", v2RootFilename)
257+
errStr := fmt.Sprintf("Failed to append certs from %s", types.V2TLSBaseFile)
322258
c.log.Error(errStr)
323259
return false
324260
}

pkg/pillar/types/locationconsts.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,6 @@ const (
6060
OnboardKeyName = IdentityDirname + "/onboard.key.pem"
6161
// RootCertFileName - what we trust for signatures and object encryption
6262
RootCertFileName = IdentityDirname + "/root-certificate.pem"
63-
// V2TLSCertShaFilename - find TLS root cert for API V2 based on this sha
64-
V2TLSCertShaFilename = CertificateDirname + "/v2tlsbaseroot-certificates.sha256"
6563
// V2TLSBaseFile is where the initial file
6664
V2TLSBaseFile = IdentityDirname + "/v2tlsbaseroot-certificates.pem"
6765
// APIV1FileName - user can statically allow for API v1

0 commit comments

Comments
 (0)