Git Product home page Git Product logo

Comments (17)

Jamstah avatar Jamstah commented on May 28, 2024 3

Have improved my understanding and this is indeed wrong:

I believe a cert is valid as long as it was signed by the CA while it was valid, and that an expired CA can indeed be used as part of a valid trust chain.

The spec just doesn't say the validity period needs to nest. It does say all the certs need to be valid at the time of validation.

The leaf could still be valid, but would require a new ca cert with the same key to validate a whole chain.

So yes, I think we could not include expired CA certs in a trust bundle.

from trust-manager.

SgtCoDFish avatar SgtCoDFish commented on May 28, 2024 1

Hey! Sorry, been a crazy couple of weeks and I've been meaning to get back to this.

I guess the main reason I'd suggest a CertificateHistory field would be API design. CertificateHistory is incredibly specific, and it's not overloaded in any way. It would be clear to me that I could use that history ConfigMap to trust every iteration of a cert in trust-manager, for example.

CertificateRequest objects are used for requesting certs, and so using them for trust purposes would be overloading their purpose. They're also liable to being cleaned up / garbage collected - it's possible to imagine a workaround to prevent them being garbage collected, but equally it's possible to imagine a well-meaning engineer cleaning them up manually, not realising how they're being used.

from trust-manager.

Jamstah avatar Jamstah commented on May 28, 2024 1

Raised a suggestion in cert-manager here: cert-manager/cert-manager#6224

from trust-manager.

SgtCoDFish avatar SgtCoDFish commented on May 28, 2024 1

Trusting an expired CA isn't ideal, but shouldn't typically be unsafe.

That could actually be a useful standalone trust-manager improvement. There's no reason to include expired CAs in our output I can think of (unless people are running massively different clocks on different machines, but I don't think that's worth us supporting).

I think we could filter anything expired out of the ConfigMap we generate.

In any case, I think you're correct that it's safe. If an attacker had the ability to modify the time on a box, they could maybe make use of an expired cert - but I think it's reasonable to assume that attackers cannot modify time in our threat model (AFAIK changing time requires privileged access in most cases, so it's already game over if they can do that)

The issue really is distrusting a still-valid cert that's been rotated. James makes a good point that PublicCertificateHistory makes it very easy to still trust a cert that's been rotated.

Right, so you think there needs to be a way to determine when all the certs issued by a CA have expired? That's a difficult question to answer :)

I think more generally, it would be required to find out when all certs issued by a CA were no longer used, with it being a safe assumption that an expired cert is no longer being used (and that no cert issued by an expired CA is being used). Doing that is absolutely gonna be very complex though, as you correctly note!

from trust-manager.

SgtCoDFish avatar SgtCoDFish commented on May 28, 2024

Thanks for taking the time to raise an issue 😁

I worry that CertificateRequests are significantly more disposable than you've listed here - many orgs choose to garbage collect then with some regularity.

Even if they weren't at risk of being garbage collected, I'm squeamish about relying on their information being correct. We know it's safe for a self signed cert, but I can't think of any other situation/issuer where it would be safe. That's because SelfSigned issuers are weird and probably shouldn't exist (there should instead be a selfSigned: true field on Certificate resources).

If the only reason to do this would be for SelfSigned issuers, I'm inlined to say that we're not solving the right problem by adding CertificateRequests as sources 🤔 What do you think?

from trust-manager.

Jamstah avatar Jamstah commented on May 28, 2024

Thanks for taking the time to review it :)

many orgs choose to garbage collect then with some regularity.

I agree that orgs choose to garbage collect them, and there is an option to have cert-manager garbage collect older revisions too, but the key word here is choose. If an organisation decided to rely on these for their trust bundle, then they would choose to not garbage collect them, or to garbage collect differently (perhaps on expiry).

SelfSigned issuers are weird and probably shouldn't exist

I sort of agree here, but also understand that they exist because of the cert flow - by having them we keep the flow and the responsible controllers the same for all issuance, instead of having to treat them as a special case. Either way - this proposal doesn't actually involve the issuer at all, we're trusting revisions of a certificate.

If the only reason to do this would be for SelfSigned issuers, I'm inlined to say that we're not solving the right problem by adding CertificateRequests as sources thinking What do you think?

I agree that this solution is targeted at self signed issuers, with other issuers the root ca certs that need to be in a bundle can't be discovered in the same way.

I've seen numerous requests for a way to handle root rotation in these scenarios. Many solutions I've seen proposed end up being a little convoluted and only work in specific cases, I thought this suggestion might be a composable way satisfy the need using a fairly generic mechanism that already exists.

I can think of one scenario where restricting to the self-signed issuer is not necessarily the right thing to do. For example, lets say we have a root CA for an organisation, and we use that to sign intermediate CAs that are for specific teams/services. Its possible that an application would want to recognise certificates for a specific team/service but not for the organisation as a whole, so would want to use this on a CA issuer to build the trust bundle based on the intermediate not the root. I don't know of anyone doing this (I expect subject checking is more sensible).

Thoughts on restricting to self-signed

(split this out as my comment got too long!)

If we want to restrict this to the self-signed issuer, we could potentially update the issuer instead with a field like PublicCertificateHistory: true so that all issued certificates are copied automatically into ConfigMap objects that can then be collected by trust-manager. That could help make using this pattern more explicit and stop users trying to do this with other issuers, but could confuse users as multiple Certificates using the same issuer would end up being in the same trust bundle automatically.

We could also recognise the use case as a more first class use case and create a new issuer SelfSignedCA that exhibits the above behaviour. That feels like a good way to make the user think about what they're doing, but feels more complicated than it needs to be.

The other way to restrict it to the self-signed issuer would be to check the issuer type when building the bundle, and refuse (or add a condition) when its not the self-signed issuer.

As an aside: The first iteration of this I thought of was to add a field to the Certificate resource called something like PublicCertificatePrefix, and on issuing a certificate, to also create a ConfigMap named <prefix>-<revision> containing the public cert, user specified labels, and no owner references. Then we could add label selectors to trust-manager to collect those into a bundle based on those labels. It was while I was investigating how that would be implemented that I found that CertificateRequest resources already follow the same pattern.

from trust-manager.

SgtCoDFish avatar SgtCoDFish commented on May 28, 2024

On the Proposal

This is the only bit that really matters, but I added some more thoughts below!

If we want to restrict this to the self-signed issuer, we could potentially update the issuer instead with a field like PublicCertificateHistory: true...

That actually seems like a really attractive option. It applies nicely to all certificates, and it seems like something people would want for different certs too. It helps with this kind of trust-manager problem, but also helps for auditing purposes (e.g. "I've had 30 different Let's Encrypt certs on this cluster, here they all are").

I'd actually add it to the Certificate resource rather than an issuer, I think.

Unfortunately, I'm not sure if any of the maintainers would have time to implement (and it's possible that there might be blockers to implementing it which I haven't thought of!).

On Trusting Intermediates

I can think of one scenario where restricting to the self-signed issuer is not necessarily the right thing to do. For example, lets say we have a root CA for an organisation, and we use that to sign intermediate CAs that are for specific teams/services.

I tend to think that this is an antipattern and I'd advise against it if I saw it, because this turns the structure of an org's PKI into an implicit auth decision.

It's so easy for a team to trust the root certificate and have everything work, only to discover later that trusting the root means trusting everything. It also makes rotation much more difficult (because every intermediate has to be treated carefully).

This little bit isn't really relevant to the problem we're discussing, but I find that I weirdly care about PKI and I like talking about it 😂

On SelfSigned

I sort of agree here, but also understand that they exist because of the cert flow - by having them we keep the flow and the responsible controllers the same for all issuance, instead of having to treat them as a special case. Either way - this proposal doesn't actually involve the issuer at all, we're trusting revisions of a certificate.

My thing is that SelfSigned is already a special case which has to be treated differently, because it's the only issuer where the issuer needs to know the private key of the cert to be signed. All other issuers use CSRs. SelfSigned is already implemented in a weird way where the private key has to be shared with the issuer!

You're totally correct that the proposal doesn't involve the issuer, of course! My reason for bringing up the weirdness of SelfSigned is that I tend to be wary of anything which treats it as more of a special case. Because it's weird, if I see a proposal which seems to apply mostly to SelfSigned I'm extra careful!

from trust-manager.

Jamstah avatar Jamstah commented on May 28, 2024

That actually seems like a really attractive option. It applies nicely to all certificates, and it seems like something people would want for different certs too. It helps with this kind of trust-manager problem, but also helps for auditing purposes (e.g. "I've had 30 different Let's Encrypt certs on this cluster, here they all are").

I'd actually add it to the Certificate resource rather than an issuer, I think.

That was my first thought, to add that to the Certificate. It was as I was looking at the code to see how complicated it would be that I realised that CSRs already had the required data in them. Those CSRs could also already be used for audit purposes if they aren't cleaned up, and cleaning them up is an active decision that gets made by the user, so if they want to use them for audit, they just don't clean them up.

Why do you think it would be better to use a PublicCertificateHistory field that duplicates each public cert to a configmap, instead of using the existing CertificateSigningRequest that already follows that pattern? (I don't mind contributing it, I would probably open an issue on cert-manager first to get more opinions before writing code though, and that's a question I expect would come up!)

Also, I'm not expecting anyone to code it for me, I'm happy to contribute, but wanted to get some feedback before doing a lot of work. I have an untested changeset for trust-manager to pick up data from the CSR already, and have put some thought into how PublicCertificateHistory would need to change the Certificate issuing flow in cert-manager, but with no code behind it yet.

I tend to think that this is an antipattern and I'd advise against it if I saw it, because this turns the structure of an org's PKI into an implicit auth decision.

Yeah, you're right and my example was stretching. Your example (audit trail) is more appropriate.

from trust-manager.

Jamstah avatar Jamstah commented on May 28, 2024

This little bit isn't really relevant to the problem we're discussing, but I find that I weirdly care about PKI and I like talking about it 😂

Yes, there's something about PKI I enjoy too, I've never been able to explain it. My favourite module at university was the crypto module.

from trust-manager.

Jamstah avatar Jamstah commented on May 28, 2024

Before I start a discussion over in the cert-manager repo, one question that might help me write that request:

Why do you think it would be better to use a PublicCertificateHistory field that duplicates each public cert to a configmap, instead of using the existing CertificateSigningRequest that already follows that pattern?

from trust-manager.

james-w avatar james-w commented on May 28, 2024

I think that linking cert-manager with trust-manager in general is going to be a very useful thing. However I think there's another issue that is important and I don't think it's been touched on here yet.

There is a differentiation between which CA cert is being used to sign, and which CA cert(s) should be trusted. This is obvious during a rotation. You will need to start trusting the new CA before it is being used to sign, and stop trusting the old one after there are no more certs signed with it in use.

The CertificateHistory idea is starting to touch on this, but I think the solution is more complex than that allows.

Having automatic connection between cert-manager and trust-manager would be really useful, but I wonder if the use cases would be limited without something quite a bit more complex.

from trust-manager.

Jamstah avatar Jamstah commented on May 28, 2024

Having automatic connection between cert-manager and trust-manager would be really useful, but I wonder if the use cases would be limited without something quite a bit more complex.

Can you describe how you think the use cases would be limited?

from trust-manager.

james-w avatar james-w commented on May 28, 2024

Can you describe how you think the use cases would be limited?

If you just trust the current signing cert then you will have issues at rotation time. If you trust all historical certs then you are trusting more than you want to ideally. These two extremes may work for some use cases, but anything more nuanced needs a more complex approach IMO.

from trust-manager.

Jamstah avatar Jamstah commented on May 28, 2024

Right, so you think there needs to be a way to determine when all the certs issued by a CA have expired? That's a difficult question to answer :)

Trusting an expired CA isn't ideal, but shouldn't typically be unsafe.

from trust-manager.

Jamstah avatar Jamstah commented on May 28, 2024

That could actually be a useful standalone trust-manager improvement. There's no reason to include expired CAs in our output I can think of...

I believe a cert is valid as long as it was signed by the CA while it was valid, and that an expired CA can indeed be used as part of a valid trust chain.

I don't think we can just not include any expired certs :(

from trust-manager.

SgtCoDFish avatar SgtCoDFish commented on May 28, 2024

I believe a cert is valid as long as it was signed by the CA while it was valid, and that an expired CA can indeed be used as part of a valid trust chain.

The first part of this surprised me! I definitely agree that an expired CA can be used in a valid chain, but I think it would have no effect. I tested this by writing a program which generates a root and leaf certificate.

First I generated a root which hadn't expired to test that the chain was valid. This test was done before 2023-07-31T10:00:00Z.

# Root cert:
# Validity
#   Not Before: Jul 31 09:37:36 2022 GMT
#   Not After : Aug 10 09:37:36 2023 GMT
-----BEGIN CERTIFICATE-----
MIIBBjCBuaADAgECAgIFOTAFBgMrZXAwETEPMA0GA1UEChMGdGVzdE9VMB4XDTIy
MDczMTA5MzczNloXDTIzMDgxMDA5MzczNlowETEPMA0GA1UEChMGdGVzdE9VMCow
BQYDK2VwAyEA9i8wVyrym754KyvVasojACJNBYELhbBaA54jxi1yJp2jNTAzMBIG
A1UdEwEB/wQIMAYBAf8CAQIwHQYDVR0OBBYEFHb9oGnwr7cm0df4WBuLfTD8H7qi
MAUGAytlcANBAIZHRZDfzCLmU5vnwr3Q+vMoq6al6E//G/EQ3WfgxZKs1lndiCv5
Y03GwGoKVfvM7EZxVlaF3GXWqLBR/AuOUAs=
-----END CERTIFICATE-----

# Leaf cert
# Validity
#   Not Before: Jul 26 09:37:36 2023 GMT
#   Not After : Aug  5 09:37:36 2023 GMT
-----BEGIN CERTIFICATE-----
MIHrMIGeoAMCAQICAhyjMAUGAytlcDARMQ8wDQYDVQQKEwZ0ZXN0T1UwHhcNMjMw
NzI2MDkzNzM2WhcNMjMwODA1MDkzNzM2WjARMQ8wDQYDVQQKEwZ0ZXN0T1UwKjAF
BgMrZXADIQCClq2g4Io7jhtThwL4Lp/rhkWmP7J6CslB34Rm0teMUqMaMBgwFgYD
VR0RBA8wDYILZXhhbXBsZS5jb20wBQYDK2VwA0EAeslRHiTu/W853rWQUdU04Sxd
GdFtvHZNVLrBnE0IA1UwCldCuIRuuhApXBFrVg/Jcnu0vmQCWdmJM43T0XuSAQ==
-----END CERTIFICATE-----

I ran openssl verify to check this was valid, and it was:

$ openssl verify -CAfile root.pem chain.pem
chain.pem: OK

Then I regenerated the chain with a root cert which had expired but with a leaf cert which was still unexpired:

# Root cert
# Validity
#   Not Before: Jul 31 09:41:14 2022 GMT
#   Not After : Jul 21 09:41:14 2023 GMT
-----BEGIN CERTIFICATE-----
MIIBBjCBuaADAgECAgIFOTAFBgMrZXAwETEPMA0GA1UEChMGdGVzdE9VMB4XDTIy
MDczMTA5NDExNFoXDTIzMDcyMTA5NDExNFowETEPMA0GA1UEChMGdGVzdE9VMCow
BQYDK2VwAyEAJ7fX2joHtI/uV/eVFGArjjNZcSCV7GSVYNUdZ8MSQEijNTAzMBIG
A1UdEwEB/wQIMAYBAf8CAQIwHQYDVR0OBBYEFHVc2w1ggTX4qWr3x23xobwQYzk8
MAUGAytlcANBALoMDsLTbusM6qZm8Op+i6Wv+972pYf3S6ZUexv6FUPAbG9/K/6j
pIWec/Ll4IpIE7kfsUNhBvZDPR+0JFEnDAg=
-----END CERTIFICATE-----

# Leaf cert
# Validity
#   Not Before: Jul 26 09:41:14 2023 GMT
#   Not After : Aug  5 09:41:14 2023 GMT
-----BEGIN CERTIFICATE-----
MIHrMIGeoAMCAQICAhyjMAUGAytlcDARMQ8wDQYDVQQKEwZ0ZXN0T1UwHhcNMjMw
NzI2MDk0MTE0WhcNMjMwODA1MDk0MTE0WjARMQ8wDQYDVQQKEwZ0ZXN0T1UwKjAF
BgMrZXADIQAx6lQrUzwFtoIJIAqX5OuPCbGmWXNMGU1zAtgPuJFAkqMaMBgwFgYD
VR0RBA8wDYILZXhhbXBsZS5jb20wBQYDK2VwA0EAaY49vM7wGBOgu7ubD13yHsPz
rdKiG/aN6ywSgw5QThIxprSomK28mPaaO/wILWgbzVE4+0WNJB4DrpbNwfSmCw==
-----END CERTIFICATE-----

Then I tried to verify again:

$ openssl verify -CAfile root.pem chain.pem
O = testOU
error 10 at 0 depth lookup: certificate has expired
error chain.pem: verification failed

I'm now convinced that if a CA in the chain has expired, the leaf will not verify!

from trust-manager.

Jamstah avatar Jamstah commented on May 28, 2024

In openssl :)

It's not in the spec, and the signature is based on the CA key. There is nothing to stop a new ca cert being made with the new key, afaik.

Good post on the subject here: https://security.stackexchange.com/questions/120469/can-a-ssl-certificate-have-longer-validity-period-than-its-parent-in-x-509-chain

I think it would make a good addition to the spec.

from trust-manager.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.