fix: fix the certs validation in trust store

[binbin-li]

What?

Change the certificates validation in a trust store.

As described in spec: https://github.com/notaryproject/notaryproject/blob/main/trust-store-trust-policy-specification.md#trust-store, certs in trust store can be either root certs or intermediate certs though the intermediate certs are not recommended. And it's possible that users put a self-signed root cert in the trust store as well.

The current implementation checks if all certs under a trust store directory are CA certs which misses the case that a self-signed root cert in the trust store.

Test

Updated and added unit tests.

Signed-off-by: Binbin Li libinbin@microsoft.com

[priteshbandi]

Single certificate in trust store doesn't means that its a self signed signing certificate. Trust store can have multiple certificate and one out of them can be a self signed signing certificate.

Logic:

func validateCerts(certs[] * x509.Certificate, path string) error {
    // to prevent any trust store misconfigurations, ensure there is at least
    // one certificate from each file.
    if len(certs) < 1 {
        return fmt.Errorf("could not parse a certificate from %q, every file in a trust store must have a PEM or DER certificate in it", path)
    }
    for _, cert: = range certs {
        if !cert.IsCA {
                if err: = cert.CheckSignature(cert.SignatureAlgorithm, cert.RawTBSCertificate, cert.Signature);
                err != nil {
                    fmt.Errorf("certificate with subject %q from file %q is not a CA certificate or self-signed signing certificate", cert.Subject, joinedPath)
                }
            }
        }
    }
}

[binbin-li]

thanks for the comment! Updated.

[binbin-li]

Actually, I found https://pkg.go.dev/crypto/x509#Certificate.CheckSignatureFrom might better fit this case, and it uses CheckSignature under the hood. Let me know if you have any concerns on it. thanks! @priteshbandi

[priteshbandi]

We cannot use CheckSignatureFrom because it imposes some extra constraints that are not valid for self signed signing certificate, instead we should use CheckSignature which only performs self signed check.

[codecov-commenter]

Codecov Report

Merging #147 (cfa3b9d) into main (ea9ee15) will increase coverage by 0.13%.
The diff coverage is 83.33%.

@@            Coverage Diff             @@
##             main     #147      +/-   ##
==========================================
+ Coverage   72.66%   72.79%   +0.13%     
==========================================
  Files          36       36              
  Lines        2491     2503      +12     
==========================================
+ Hits         1810     1822      +12     
  Misses        544      544              
  Partials      137      137              

Impacted Files	Coverage Δ
verification/store.go	78.18% <83.33%> (+6.08%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[priteshbandi]

LGTM

[shizhMSFT]

LGTM