Skip to content

Commit

Permalink
updated per code review
Browse files Browse the repository at this point in the history 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
  • Loading branch information
@Two-Hearts
Two-Hearts committed Oct 26, 2023
1 parent 0d825f0 commit 28b581b
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 16 deletions.
21 changes: 11 additions & 10 deletions verifier/truststore/truststore.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,41 +67,42 @@ func (trustStore *x509TrustStore) GetCertificates(ctx context.Context, storeType
return nil, TrustStoreError{Msg: fmt.Sprintf("unsupported trust store type: %s", storeType)}
}
if !file.IsValidFileName(namedStore) {
return nil, TrustStoreError{Msg: fmt.Sprintf("named store name needs to follow [a-zA-Z0-9_.-]+ format: %s is invalid", namedStore)}
return nil, TrustStoreError{Msg: fmt.Sprintf("trust store name needs to follow [a-zA-Z0-9_.-]+ format, %s is invalid", namedStore)}
}
path, err := trustStore.trustStorefs.SysPath(dir.X509TrustStoreDir(string(storeType), namedStore))
if err != nil {
return nil, TrustStoreError{InnerError: fmt.Errorf("failed to get path of trust store %s with type %s: %w", namedStore, storeType, err)}
return nil, TrustStoreError{InnerError: err, Msg: fmt.Sprintf("failed to get path of trust store %s of type %s", namedStore, storeType)}
}
// throw error if path is not a directory or is a symlink or does not exist.
fileInfo, err := os.Lstat(path)
if err != nil {
if os.IsNotExist(err) {
return nil, TrustStoreError{InnerError: err, Msg: fmt.Sprintf("the trust store %q of type %q doesn't exist", namedStore, storeType)}
return nil, TrustStoreError{InnerError: err, Msg: fmt.Sprintf("the trust store %q of type %q does not exist", namedStore, storeType)}
}
return nil, TrustStoreError{InnerError: fmt.Errorf("failed to access the trust store %q: %w", path, err)}
return nil, TrustStoreError{InnerError: err, Msg: fmt.Sprintf("failed to access the trust store %q of type %q", namedStore, storeType)}
}
mode := fileInfo.Mode()
if !mode.IsDir() || mode&fs.ModeSymlink != 0 {
return nil, TrustStoreError{Msg: fmt.Sprintf("trust store %q is not a regular directory (symlinks are not supported)", path)}
return nil, TrustStoreError{Msg: fmt.Sprintf("the trust store %s of type %s with path %s is not a regular directory (symlinks are not supported)", namedStore, storeType, path)}
}
files, err := os.ReadDir(path)
if err != nil {
return nil, TrustStoreError{InnerError: fmt.Errorf("failed to access the trust store %q: %w", path, err)}
return nil, TrustStoreError{InnerError: err, Msg: fmt.Sprintf("failed to access the trust store %q of type %q", namedStore, storeType)}
}

var certificates []*x509.Certificate
for _, file := range files {
joinedPath := filepath.Join(path, file.Name())
certFileName := file.Name()
joinedPath := filepath.Join(path, certFileName)
if file.IsDir() || file.Type()&fs.ModeSymlink != 0 {
return nil, CertificateError{Msg: fmt.Sprintf("trusted certificate %q is not a regular file (directories or symlinks are not supported)", joinedPath)}
return nil, CertificateError{Msg: fmt.Sprintf("trusted certificate %s in trust store %s of type %s is not a regular file (directories or symlinks are not supported)", certFileName, namedStore, storeType)}
}
certs, err := corex509.ReadCertificateFile(joinedPath)
if err != nil {
return nil, CertificateError{InnerError: fmt.Errorf("failed to read the trusted certificate %q: %w", joinedPath, err)}
return nil, CertificateError{InnerError: err, Msg: fmt.Sprintf("failed to read the trusted certificate %s in trust store %s of type %s", certFileName, namedStore, storeType)}
}
if err := ValidateCertificates(certs); err != nil {
return nil, CertificateError{InnerError: fmt.Errorf("failed to validate the trusted certificate %q: %w", joinedPath, err)}
return nil, CertificateError{InnerError: err, Msg: fmt.Sprintf("failed to validate the trusted certificate %s in trust store %s of type %s", certFileName, namedStore, storeType)}
}
certificates = append(certificates, certs...)
}
Expand Down
12 changes: 6 additions & 6 deletions verifier/truststore/truststore_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,26 +49,26 @@ func TestLoadValidTrustStoreWithSelfSignedSigningCertificate(t *testing.T) {
}

func TestLoadTrustStoreWithInvalidCerts(t *testing.T) {
failurePath := filepath.FromSlash("../testdata/truststore/x509/ca/trust-store-with-invalid-certs/invalid")
expectedErr := fmt.Errorf("failed to read the trusted certificate %q: x509: malformed certificate", failurePath)
// testing ../testdata/truststore/x509/ca/trust-store-with-invalid-certs/invalid
expectedErr := fmt.Errorf("failed to read the trusted certificate %s in trust store %s of type %s", "invalid", "trust-store-with-invalid-certs", "ca")
_, err := trustStore.GetCertificates(context.Background(), "ca", "trust-store-with-invalid-certs")
if err == nil || err.Error() != expectedErr.Error() {
t.Fatalf("invalid certs should return error: %q", expectedErr)
}
}

func TestLoadTrustStoreWithLeafCerts(t *testing.T) {
failurePath := filepath.FromSlash("../testdata/truststore/x509/ca/trust-store-with-leaf-certs/non-ca.crt")
expectedErrMsg := fmt.Sprintf("failed to validate the trusted certificate %q: certificate with subject \"CN=wabbit-networks.io,O=Notary,L=Seattle,ST=WA,C=US\" is not a CA certificate or self-signed signing certificate", failurePath)
// testing ../testdata/truststore/x509/ca/trust-store-with-leaf-certs/non-ca.crt
expectedErrMsg := fmt.Sprintf("failed to validate the trusted certificate %s in trust store %s of type %s", "non-ca.crt", "trust-store-with-leaf-certs", "ca")
_, err := trustStore.GetCertificates(context.Background(), "ca", "trust-store-with-leaf-certs")
if err == nil || err.Error() != expectedErrMsg {
t.Fatalf("leaf cert in a trust store should return error: %s, got: %v", expectedErrMsg, err)
}
}

func TestLoadTrustStoreWithLeafCertsInSingleFile(t *testing.T) {
failurePath := filepath.FromSlash("../testdata/truststore/x509/ca/trust-store-with-leaf-certs-in-single-file/RootAndLeafCerts.crt")
expectedErrMsg := fmt.Sprintf("failed to validate the trusted certificate %q: certificate with subject \"CN=wabbit-networks.io,O=Notary,L=Seattle,ST=WA,C=US\" is not a CA certificate or self-signed signing certificate", failurePath)
// testing ../testdata/truststore/x509/ca/trust-store-with-leaf-certs-in-single-file/RootAndLeafCerts.crt
expectedErrMsg := fmt.Sprintf("failed to validate the trusted certificate %s in trust store %s of type %s", "RootAndLeafCerts.crt", "trust-store-with-leaf-certs-in-single-file", "ca")
_, err := trustStore.GetCertificates(context.Background(), "ca", "trust-store-with-leaf-certs-in-single-file")
if err == nil || err.Error() != expectedErrMsg {
t.Fatalf("leaf cert in a trust store should return error: %s, got: %v", expectedErrMsg, err)
Expand Down

0 comments on commit 28b581b

Please sign in to comment.