crypto/x509: add a PKITS test

[FiloSottile]

PKITS is a large X.509 test suite. Some of the stuff in there we don't actually support, but it would be good to run through it all and explicitly mark the things we don't support.

[dandare100]

Hello

I have started with this and just want to know if I'm on the right track before carrying on with CRL tests

I understand that you are looking for a list of unsupported tests/functionality and I am assuming these will be fixed/catered for on a case by case basis ?

Are you also looking for a PKITS test in a golang test case format ?

Ran the tests for 4.1.1 through to 4.5.8 for now.

Here are the observations for test failures:

4.1.4

test 4.1.4 [Signature Verification : Valid DSA Signatures Test4]: failed. x509: certificate signed by unknown authority (possibly because of "x509: cannot verify signature: algorithm unimplemented" while trying to verify candidate authority certificate "DSA CA

fails in x509.go
func checkSignature(algo SignatureAlgorithm, signed, signature []byte, publicKey crypto.PublicKey) (err error)
because only rsa, ecdsa and ed25519 public keys are supported
It looks like DSA signature checks are not implemented/supported. Docs say is deprecated but the test requires it

4.1.5

4.1.5 [Signature Verification : Valid DSA Parameter Inheritance Test5] skipping test : failed to generate ca chain asn1: syntax error: sequence truncated

Failed parsing of DSAParametersInheritedCACert.crt in

paramsData := keyData.Algorithm.Parameters.FullBytes
		params := new(dsaAlgorithmParameters)
		rest, err = asn1.Unmarshal(paramsData, params)

paramsData and params are nil

4.1.6

test 4.1.6 [Signature Verification : Invalid DSA Signature Test6]: x509: certificate signed by unknown authority (possibly because of "x509: cannot verify signature: algorithm unimplemented" while trying to verify candidate authority certificate "DSA CA")

fails in x509.go
```func checkSignature(algo SignatureAlgorithm, signed, signature []byte, publicKey crypto.PublicKey) (err error)``

because only rsa, ecdsa and ed25519 public keys are supported
It looks like DSA signature checks are not implemented/supported. Docs say is deprecated but the test requires it

4.3.3,4,5,10,11

test 4.3.3 [Verifying Name Chaining : Valid Name Chaining Whitespace Test3]:  failed. x509: certificate signed by unknown authority
test 4.3.4 [Verifying Name Chaining : Valid Name Chaining Whitespace Test4]:  failed. x509: certificate signed by unknown authority
test 4.3.5 [Verifying Name Chaining : Valid Name Chaining Capitalization Test5]:  failed. x509: certificate signed by unknown authority
test 4.3.10 [Verifying Name Chaining : Valid Rollover from PrintableString to UTF8String Test10]:  failed. x509: certificate signed by unknown authority
test 4.3.11 [Verifying Name Chaining : Valid UTF8String Case Insensitive Match Test11]:  failed. x509: certificate signed by unknown authority

in cert_pool.go

func (s *CertPool) findPotentialParents(cert *Certificate) []*Certificate {

for _, c := range s.byName[string(cert.RawIssuer)] {

The raw issuer does not match raw subject because of spaces, capitilization.
The reason is certs are keyed by string of cert.RawSubject

// AddCert adds a certificate to a pool.

func (s *CertPool) AddCert(cert *Certificate) {
	if cert == nil {
		panic("adding nil Certificate to CertPool")
	}
	s.addCertFunc(sha256.Sum224(cert.Raw), string(cert.RawSubject), func() (*Certificate, error) {
		return cert, nil
	})
}

also in verify.go

if !bytes.Equal(child.RawIssuer, c.RawSubject) {
			return CertificateInvalidError{c, NameMismatch, ""}
		}

Conforming implementations MUST use the LDAP StringPrep profile
(including insignificant space handling), as specified in [RFC4518], (2.6.1)
as the basis for comparison of distinguished name attributes encoded
in either PrintableString or UTF8String. Conforming implementations
MUST support name comparisons using caseIgnoreMatch.

I also suspect that some tests pass (and should) but only a subset of checks are done

For example:

test 4.3.6 [Verifying Name Chaining : Valid Name Chaining UIDs Test6]: passed.

The subject and issuer uids are catered for in the asn.1 certificate struct but not ever copied over to Certificate in the parseCertificate function

in asn.1 certificate struct

UniqueId           asn1.BitString   `asn1:"optional,tag:1"`
SubjectUniqueId    asn1.BitString   `asn1:"optional,tag:2"`

func parseCertificate(in *certificate) (*Certificate, error) {

Is it ok for me to complete the test analysis like the above, package it in a better format when complete or did you have another approach in mind?

[dandare100]

results.4.1.1-4.5.8.txt

[FiloSottile]

Hello @dandare100, thank you for giving this a spin!

I encourage you to send a small CL/PR with the tests you have so far, so we can discuss the approach without reviewing a massive change set. The test should be a standard test in a _test.go file in the crypto/x509 package.

In general, it's expected there will be test cases that PKITS calls valid but we reject. We tend to be stricter than the standard where we can afford it. For those, we want to clearly document in the test what is going on, and check that they fail.

What we don't want is the opposite, invalid chains that we accept.

[gopherbot]

Change https://golang.org/cl/339292 mentions this issue: crypto: PKITS tests

[dandare100]

Hello.
Thank you.
I have submitted a small CL/PR to start the discussion as requested.

[gopherbot]

Change https://go.dev/cl/618415 mentions this issue: crypto/x509: run the NIST PKI test suite