feat: add system and combined CA pool modules #7406
Conversation
modules/caddytls/capools.go
Outdated
| // LIMITATION: x509.CertPool doesn't expose its certificates, making it impossible | ||
| // to merge multiple pools. Return an error if multiple sources are configured. | ||
| if len(ccp.sources) > 1 { | ||
| return fmt.Errorf("combined CA pool currently supports only a single source due to x509.CertPool API limitations (got %d sources); to use multiple certificate sources, consider using a single 'file' source with multiple pem_file entries, or contribute a fix to expose certificate data from the CA interface", len(ccp.sources)) | ||
| } | ||
|
|
||
| // At this point we have exactly one source | ||
| ccp.pool = ccp.sources[0].CertPool() | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // Syntax: | ||
| // | ||
| // trust_pool combined { | ||
| // source <module_name> { | ||
| // <module_config> | ||
| // } | ||
| // } | ||
| // | ||
| // LIMITATION: Currently only a single source is supported due to x509.CertPool | ||
| // API limitations. Specifying multiple sources will result in a provisioning error. | ||
| // To combine multiple certificate files, use a single 'file' source with multiple | ||
| // pem_file entries instead. |
There was a problem hiding this comment.
I have intentionally added limitation comments cause i wasnt aware how to proceed here.
|
"combined merges multiple roots sources" is not satisfied as per issue #7391 and i am erroring out as you see in above comment.
This approach which i think is a breaking change so i wanted to consult with my planned implementation |
|
Happy new year and i reviewed your code and i have two types of solutions type CA interface {
AppendTo(pool *x509.CertPool) error
}This is breaking change because not all CA pools can expose certificates. The second way is creating a secondary interface (non breaking change) type CertificateProvider interface {
Certificates() []*x509.Certificate
}The combined module would then check whether each source implements this interface and merge certificates only from those that do. |
|
Hello @FreyreCorona , thank you for the suggestions! will work on a non-breaking solution. lastly, sorry for the delayed response. forgot about this pr due to work |
|
Nice work on the implementation, let's wait for the review. No worries, we're all busy, and I've been pretty quiet around here these past few days too. |
2 participants
Summary
I've implemented the
systemtrust pool module which usesx509.SystemCertPool()and works as expected.However, the
combinedmodule has a fundamental limitation: Go'sx509.CertPooldoesn't expose its certificates, making it impossible to merge multiple pools.so should we modify the CA interface to add a method like
Certificates() []*x509.Certificateso sources can expose their certificates for merging? not sure how to proceed here... Would be happy to learn.Assistance Disclosure
I wrote the code, but Claude generated the tests.