Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support name constraint extension for self-generated CA authority #5759

Open
sandstrom opened this issue Aug 17, 2023 · 8 comments
Open

Support name constraint extension for self-generated CA authority #5759

sandstrom opened this issue Aug 17, 2023 · 8 comments
Labels
feature ⚙️ New feature or request

Comments

@sandstrom
Copy link

sandstrom commented Aug 17, 2023

When generating a CA cert via caddy and putting that in the trust store, those private keys can also forge certificates for any other domain.

We're only using this for company.dev and two other domains. Would be neat if we could tell Caddy to create a CA with name constraint extension, reducing the scope of its authority to only domains (and their subdomains) that we need it for.

Just an idea, feel free to close this if it isn't relevant.

Also, I'd suggest enabling the "Discussions" tab on Github. Then you'd get fewer issues for ideas like this 😄

Background:
https://security.stackexchange.com/questions/31376/can-i-restrict-a-certification-authority-to-signing-certain-domains-only

@francislavoie
Copy link
Member

francislavoie commented Aug 17, 2023

@maraino do you know if this is an option Smallstep supports? Haven't checked the API yet, wondering if you know.

Also, I'd suggest enabling the "Discussions" tab on Github. Then you'd get fewer issues for ideas like this 😄

We have forums: https://caddy.community, no need for another place for discussion.

@francislavoie francislavoie added the feature ⚙️ New feature or request label Aug 17, 2023
@sandstrom
Copy link
Author

There seems to be some kind of support:
https://smallstep.com/docs/step-ca/templates/#adding-name-constraints

But I don't know certificates and these extensions in enough detail to discern if this support is enough, or how much work it would be (or if it's even a good idea).

@maraino
Copy link
Contributor

maraino commented Aug 23, 2023

@francislavoie: Yes, but you want the name constraints in the intermediate.

@maraino
Copy link
Contributor

maraino commented Aug 23, 2023

Caddy supports a root and intermediate coming from the configuration. If the intermediate has name constraints, it should work automatically.

But if you want this from the intermediate that Caddy generates, you will have to add the necessary attributes here:
https://github.com/caddyserver/caddy/blob/4776f62caa36c580d24be8e55ebc6a61ae129f51/modules/caddypki/certificates.go#L38C1-L42

You can do it either in the template or directly in the template. The template is probably more straightforward for you. You will need to get the configuration values from the Caddyfile, so you will need to pass them as a new argument to generateIntermediate.

But again, as this is also an edge case, you can document how to create a new root and intermediate with name constraints, and it will work. So, for example, with step, given a root certificate and key, you can sign an intermediate with the name constraints defined in a template like this (nc.tpl):

{
    "subject": {{ toJson .Subject }},
    "keyUsage": ["certSign", "crlSign"],
    "basicConstraints": {
        "isCA": true,
        "maxPathLen": 0
    },
    "nameConstraints": {
        "critical": true,
        "permittedDNSDomains": ["smallstep.com"]
    }
}

Running:

step certificate create --ca root_ca.crt --ca-key root_ca_key --template nc.tpl \
  'Smallstep Intermediate' intermediate_ca.crt intermediate_ca_key

People using name constraints should know what they exactly mean, as some cases are not obvious. For example, adding just permittedDNSDomains as above does not exclude creating domains with IP addresses or any other type of SAN. Name constraints are defined in RFC5280#4.2.1.10

@francislavoie
Copy link
Member

Thanks @maraino! That helps!

I'm clicking around through the code, and I'm noticing that there's nothing in https://github.com/smallstep/crypto/blob/c8a8532f869761ee64c8520ec4f1951a7865d161/x509util/templates.go for name constraints (and some other template fields) currently. Is it reasonable to ask that that's added? I think that would make the API more natural/resilient for us, by using exported consts or functions instead of hard-coding template keys in Caddy.

@maraino
Copy link
Contributor

maraino commented Aug 23, 2023

You can do it either in the template or directly in the template. The template is probably more straightforward for you

With the second template, I meant the template variable in the code, not the x509util.DefaultIntermediateTemplate. It will be easier to add something like:

template, signer, err := newCert(commonName, x509util.DefaultIntermediateTemplate, lifetime)
if err != nil {
	return nil, nil, err
}
if len(config.PermittedDNSDomains) > 0 {
	template.PermittedDNSDomainsCritical = true
	template.PermittedDNSDomains = config.PermittedDNSDomains
}

That assumes that config are configuration options in the Caddyfile. But you can also configure that template and do if conditions.

@maraino
Copy link
Contributor

maraino commented Aug 24, 2023

I'm clicking around through the code, and I'm noticing that there's nothing in https://github.com/smallstep/crypto/blob/c8a8532f869761ee64c8520ec4f1951a7865d161/x509util/templates.go for name constraints (and some other template fields) currently. Is it reasonable to ask that that's added? I think that would make the API more natural/resilient for us, by using exported consts or functions instead of hard-coding template keys in Caddy.

We support name constraints in the template, nameConstraints object can have these attributes.

The docs from above show that https://smallstep.com/docs/step-ca/templates/#adding-name-constraints

But for your use case, it is probably better to do it like my previous comment, you can even allow users to define the x509util.DefaultIntermediateTemplate, to make it 100% configurable. But if somebody wants 100% configurability, they should create the root and intermediate themselves, and that should be already working.

@mholt
Copy link
Member

mholt commented Sep 7, 2023

Thanks for your wisdom and guidance as usual, @maraino 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature ⚙️ New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants