Caching w/o common names

[jwilner]

Is your feature request related to a problem? Please describe.
We don't provide common names for some vault-issued certificates; we rely strictly on URI SANs. We would like to be able to use this library, but its assumption that a common name is defined is problematic in at least one respect that I am able to confirm -- the dir cache implementation breaks down a bit (because it does cacheDir + commonName + ".crt"). Looking through the source, it also seems to assume the presence of commonName at a few other points but I haven't hit any issues elsewhere personally -- our vault policy accepts an empty string common name happily enough.

Describe the solution you'd like
I wonder if we could:

• add the option for a "local" or maybe "logical" name for the certificate, which would take precedence over common name if specified and be used as a local unique identifier, when needed -- we would just set this to the URISAN.String().
• avoid adding empty string common names to lists, etc.

Describe alternatives you've considered
I've considered that my conclusion that a common name is optional is wrong -- but I can't find any evidence to the contrary. Given that, we don't want to use it, as going against a common name would go against our PKI's plan to use URI SANs as the canonical identifier.

I've also considered -- and prepared for -- the possibility that this is going to be simply viewed as out of scope for this project. Totally reasonable!

[johanbrandhorst]

Thanks for the issue report! Certify was designed with a common name being required for issuing client side certificates. I agree that in general x509 use common names are optional, but we need something to decide what to issue the certificate under. I'm not sure I understand how that causes the issue. Do you configure an empty common name in your certify settings?

[jwilner]

Hi @johanbrandhorst.

Yes, we need an empty common name because we rely on URI SANs.

I configure Certify as follows:

	c := certify.Certify{
		Issuer:      &issuer,
		RenewBefore: time.Hour * 24,
		CertConfig: &certify.CertConfig{
			KeyGenerator:               rsaKeyGenerator{2048},
			URISubjectAlternativeNames: []*url.URL{u},
		},
	}

	if dir, err := os.UserHomeDir(); err != nil {
		c.Cache = certify.NewMemCache()
	} else {
		c.Cache = certify.DirCache(path.Join(dir, ".cache/certify"))
	}

This results in a basically broken cache file tree, looking like:

 ~/.cache/
  |_ certify/
  |_ certify.crt
  |_ certify.key

This is clearly a broken cache impl.

Additionally, internally, even if empty, common names will be passed to appendName

I was floating the idea of:

• an alternate, configurable cache key that supersedes CommonName
• not appending the empty CommonName.

This would permit config like:

	c := certify.Certify{
		Issuer:      &issuer,
+               CacheKey: u.String(),
		RenewBefore: time.Hour * 24,
		CertConfig: &certify.CertConfig{
			KeyGenerator:               rsaKeyGenerator{2048},
			URISubjectAlternativeNames: []*url.URL{u},
		},
	}

	if dir, err := os.UserHomeDir(); err != nil {
		c.Cache = certify.NewMemCache()
	} else {
		c.Cache = certify.DirCache(path.Join(dir, ".cache/certify"))
	}

That would preserve old behavior while being friendlier to certs which do not require common names.

Thanks!

[johanbrandhorst]

Thanks for the clarification, if anything it feels like the library should fail more loudly if a CommonName is not supplied - is there a reason you want to avoid the use of a common name to configure Certify? AFAIK there's no harm in adding it, and I think your issues would go away if you just set the CommonName in your setup.

If that really is unacceptable I suppose it would be possible to write some logic to fall back to some other name, but I think it would require adding another option which would have a strange interaction with the CommonName. Have you tried just using CommonName as a proxy for cache key? How does it break?

Yes, we need an empty common name because we rely on URI SANs.

So you are saying that your client side certificates must not have a common name set, is that right?

[jwilner]

Effectively, yes, the client certificates must not have a common name set.

For context, I'm prototyping an mTLS authentication arch and effectively in control of both the PKI (vault) and TLS peers. A major goal of the project is to introduce an unambiguous and consistent client identity, and to that end we're following the SPIFFE standard for structured URI SANS. In the interests of being unambiguous, I don't want to introduce a second name to the architecture where there isn't an essential need for one -- and, although I'd love to use certify, its caching requirements don't really meet that bar. cert-manager in k8s will be our main client-cert management tool and doesn't have the common-name requirement.

I agree from your POV that it would definitely make sense to fail more loudly if CommonName it not supplied, since it is effectively an implicit requirement -- but that would also mean we can't use your library :). In any case, every tool shouldn't meet every use case, but I wanted to explore the possibility with you.

Thanks for your thoughts!

[johanbrandhorst]

Thanks for the clarification, that makes sense. So what we essentially need here is to add a fallback mechanism for identifying the "name" to use for client side certificates when the CommonName is not set. The Certify struct already exposes a CertConfig which looks a bit like the cert-manager configuration in that you can specify SANs, IP SANs and URI SANs explicitly, so we could fall back to checking those in order, if set, when creating a client side certificate.

The slightly worse part is that the existing logic automatically sets the CommonName on the CSR. I don't think I can change that and maintain backwards compatibility with existing users, so would it be OK for the CommonName on your generated certificates to be the name found in your URI SANs? If so, this shouldn't be too much work, just a matter of adding this fallback heuristic in GetClientCertificate. If not, we may have to introduce a mechanism for users to explicitly request that no CommonName is set on the CSR.

What do you think?