CC @golang/security.

Based on a review of the implementation, we believe this regression was caused by the call to alreadyInChain within Certificate.buildChains.

The problem is that because the leaf certificate has the same subject and public key as the intermediate, the intermediate is treated as a duplicate and ignored. However, only intermediate certificates should be considered as duplicates of other intermediate certificates. That is, the leaf certificate ought to be ignored for this check, so it should be changed to currentChain[1:]. Alternatively, alreadyInChain should do a byte-for-byte check on the whole certificate for chain[0], and only do the subject/key matching check for chain[1:].

My intuition is that this is not supposed to work. The leaf is the leaf, and that leaf does not chain to a root. A chain needs to have an unbroken sequence of Subject/Issuer links, and the leaf Issuer is not the Subject of any next link. Sure, you could pretend the intermediate is actually the leaf because Subject and SPKI match, and the intermediate does chain to a root, but I don't think that's expected behavior? For example, what would we return as the leaf of the verified chain? The leaf that does not chain to the root or the intermediate that does not match the leaf?

Are there spec references for your desired behavior?

@FiloSottile First, I would like to stress that this was an undocumented breaking change introduced in Go 1.19 with no workaround. Go 1.18 accepts this.

A chain needs to have an unbroken sequence of Subject/Issuer links, and the leaf Issuer is not the Subject of any next link.

This statement is false. The leaf issuer matches the subject of the intermediate. In fact, that is the very reason that the intermediate is being ignored due to the alreadyInChain check.

Are there spec references for your desired behavior?

I don't see anything in RFC 5280 that forbids self-signed leaf certificates in this regard. Are there spec references for the 1.19 behavior?

Ah, I was confused by the fact that the intermediate's CN is Leaf. The chain is the following then.

Subject: CN = Leaf
Issuer: CN = Leaf

Subject: CN = Leaf
Issuer: CN = Root

Subject: CN = Root
Issuer: CN = Root

Correct, which is what happens in 1.18.

@FiloSottile If a fix is made, can it be backported to 1.19 and 1.20, since this is a regression?

I think this is working as intended. ITU-T X.509 disallows certificates from repeating in chains, although is somewhat vague in determining what a repeated certificate is. alreadyInChain checks for matching subject, public key, and SANs, matching the suggested implementation in RFC 4158 Section 2.4.2.

In the case you present the chain seems redundant, the leaf and intermediate have the exact same "meaning" , why would you not just send the intermediate by itself (assuming the root is trusted)? crypto/x509 targets the web PKI, as such it doesn't necessarily implement every possible corner case for X.509. As far as I know it seems extremely unlikely that any publicly trusted CA would be willing to issue certificates with a SPKI that matches their issuing intermediate. Is the chain you're failing to verify come from some common system?

Additionally , ITU-T X.509 Section 12.5.1 says self-signed certificates which are not trust anchors should be ignored entirely during path building. We don't entirely do this, but this is more evidence that this particular use case is not really intended to be supported.

@rolandshoemaker Again, I would like to stress that this was an undocumented breaking change introduced in Go 1.19 with no workaround. Go 1.18 accepts this.

In the case you present the chain seems redundant, the leaf and intermediate have the exact same "meaning" , why would you not just send the intermediate by itself (assuming the root is trusted)?

The intent was to verify that the cross-signed intermediate was properly issued, such that anything the self-signed certificate had issued would be trusted.

ITU-T X.509 Section 12.5.1 says self-signed certificates which are not trust anchors should be ignored entirely during path building.

Within the actual chain (excluding the leaf) this is reasonable. Self-signed intermediate certificates within the chain always serve no purpose and can simply be skipped. RFC 5280 even specifically states that they don't factor into path length. I would imagine that is the reason that ITU-T X.509 says to ignore them during path building. However, the leaf certificate obviously cannot be ignored during path building.

@FiloSottile @rolandshoemaker following up on this

The intent was to verify that the cross-signed intermediate was properly issued, such that anything the self-signed certificate had issued would be trusted.

I'm not sure I understand this use case. Anything "issued" by the self-signed intermediate will clearly be considered invalid in a chain (leaf -> ss leaf -> intermediate -> root), but will be considered valid if the self-signed leaf is excluded (leaf -> intermediate -> root). If you present one of these chains (with the extra leaf) to the verifier it will succeed, since it'll ignore the invalid path containing the non-trust anchor self-signed certificate (as ITU-T X.509 suggests) and find the valid path that does not contain it. The verifier can not do this when there is no additional leaf, because it clearly cannot "ignore" the invalid certificate since that is the thing being verified.

I'm also slightly confused by referring to this as a cross-signature, what is cross signing what? In a traditional cross-signature setup you'd have two parents signing the same certificate (i.e. creating two certificates with the same subject + key pair but different signatures), but in this case you appear to have a linear chain with a cutback when there is a true leaf.

Regardless, there is a workaround here, if you are verifying chains that contain the full path, they will succeed, since the verifier will find the valid path that excludes the self-signed intermediate (as discussed above, and suggested by ITU-T X.509). If you want to manually verify that the signature on the self-signed intermediate can be verified by the "parent" intermediate, you can do so with CheckSignatureFrom.

ITU-T X.509 Section 9.1.5 covers use cases for self-signed certificates, and I'm pretty confident in my reading that this particular scenario is unsupported when verifying a chain that does not contain an actual leaf. OpenSSL would seem to agree here, refusing to verify the chain as the "leaf" is self-signed and not a trust anchor.

Removing "support" for this unexpected use case in 1.19 may have been a breaking change, but given this behavior was unintended, I do not think there is a strong case for us to re-introduce it. Given this has been in a major release for an entire cycle and this is the first time we are hearing about it, and as there is no specification support for this, only reinforces my opinion that this is currently working-as-intended.

@rolandshoemaker Your analysis is incorrect. Here we have B -> B' -> A, where B is our self-signed leaf, A is our self-signed root, and B' is B signed by A. Now suppose that B had previously signed some certificate C. If I were to use B as my root of trust, then the chain would be C -> B. Now if I use A as my root of trust, the chain will be C -> B' -> A. However, this only works if B' was properly issued. If for example B' had the wrong subject, then the chain C -> B' -> A would not work. This is the kind of situation that verifying the chain B -> B' -> A protects against.

I'm also slightly confused by referring to this as a cross-signature, what is cross signing what?

We refer to it as a cross-sign internally, because we have one self-signed certificate ("root") signing another self-signed certificate ("leaf") to produce a third certificate.

Regardless, there is a workaround here, if you are verifying chains that contain the full path, they will succeed, since the verifier will find the valid path that excludes the self-signed intermediate (as discussed above, and suggested by ITU-T X.509).

The self-signed certificate is NOT an intermediate. It is the leaf. That is the bug in crypto/x509. It is ignoring the not-self-signed intermediate, because it has the same subject as the self-signed leaf, and thus fails to build a chain from leaf to root.

OpenSSL would seem to agree here, refusing to verify the chain as the "leaf" is self-signed and not a trust anchor.

OpenSSL simply stops building the chain when it reaches a self-signed certificate, instead of stopping when it reaches a root. That would imply that including a self-signed intermediate at all can cause it to fail spuriously. So I wouldn't follow it as an example of correctness here.

Removing "support" for this unexpected use case in 1.19 may have been a breaking change, but given this behavior was unintended, I do not think there is a strong case for us to re-introduce it. Given this has been in a major release for an entire cycle and this is the first time we are hearing about it, and as there is no specification support for this, only reinforces my opinion that this is currently working-as-intended.

I would like to provide an experience report on this point. There were three breaking changes in 1.19 - rejecting duplicate extensions, enforcing name constraints on intermediates, and rejecting self-signed leafs. While the first two were at least documented, it still makes it hard for us to upgrade, as we have to monitor that we will not be affected by the breaking change. These really should have been behind GODEBUG flags. In addition, the fact that it produces a vague, misleading error made it take quite a while for us to get to the bottom of why everything was failing. So even if you are going to leave the new 1.19 behavior, it really should return a better error message in case the leaf is self-signed.

OpenSSL simply stops building the chain when it reaches a self-signed certificate, instead of stopping when it reaches a root. That would imply that including a self-signed intermediate at all can cause it to fail spuriously. So I wouldn't follow it as an example of correctness here.

Are you aware of any other path building implementation that supports this use case that I can look at?

@rolandshoemaker @FiloSottile Following up on this. Like it or not, this was clearly a breaking change in violation of the Go 1 compatibility promise. In addition, the behavior is inconsistent: a self-signed leaf is still allowed if it is also a root.

If the breaking change is not addressed, then at least there should be a specific error message so developers immediately know what the problem is.

@rolandshoemaker @FiloSottile Following up.

Not every detail of crypto/x509's behavior is covered by the Go 1 Compatibility Promise, Hyrum's Law notwithstanding. In particular, we recently clarified the primary target of crypto/x509 is compatibility with the WebPKI, and this is definitely not a compliant WebPKI chain.

In assessing whether this is something we want to support, it would help to answer @rolandshoemaker's question about other ecosystem examples. #58792 (comment)

In any case, we're always happy to add clearer error messages if they don't require adding happy path complexity.

A workaround for developers who want to upgrade go version:
Recently I came up with similar issue when I tried to upgrade my go version from 1.18 to 1.21. This is happening due to implementation of alreadyInChain as mentioned by @rittneje .

To verify self signed root certificate, itself can be passed as trusted root in verifyOptions:

func main() {
	var rootCertificate *x509.Certificate

	var verifyOptions x509.VerifyOptions
	// Uncomment these two lines to add rootCertificate as trusted root
	// verifyOptions.Roots = x509.NewCertPool()
	// verifyOptions.Roots.AddCert(rootCertificate)

	if _, err := rootCertificate.Verify(verifyOptions); err != nil {
		log.Printf("verify certificate failed: %v", err)
	} else {
		log.Printf("verify certificate successful")
	}
}

note: following intermediate and leaf certificates should not be passed in option like this, instead a trust chain to be built.

@owkraju I don't understand what issue you encountered, but your workaround seems to have nothing to do with this bug.

The bug described here is that if I have three certificates - A, B, and, C - where A is self-signed and B has the same subject/key as A but was issued by C, then Go 1.19+ will fail to build the chain A > B > C, even though all prior Go versions did this correctly. And the reason for this bug is that the faulty implementation of alreadyInChain incorrectly asserts that because A and B have the same subject, it can ignore B during chain building.

It should be noted that Certificate.Verify special-cases the situation in which the leaf is also a root (regardless of whether it is self-signed), and thus no workaround is necessary for that use case.

@rittneje I recently came across the same issue when i built etcd by go1.19.x. Maybe check this commit for some clues 65153e4