@marcel91200 / caddy Mirror · read-only / commits / 99f91c4c
Fork 0

Fast and extensible multi-platform HTTP/1-2-3 web server with automatic HTTPS

Mirrored from https://github.com/Slazee/caddy.git

Synced Nov 14, 05:15 UTC

httpcaddyfile: tls: Load repeated cert files only once, with one tag

See end of issue #3004. Loading the same certificate file multiple times with different tags will result in it being de-duplicated in the in- memory cache, because of course they all have the same bytes. This meant that any certs of the same filename loaded with different tags would be overwritten by the next certificate of the same filename, and any conn policies looking for the tags of the previous ones would never find them, causing connections to fail. So, now we remember cert filenames and their tags, instead of loading them multiple times and overwriting previous ones. A user crafting their own JSON might make this error too... maybe we won't see it happen. But if it does, one possibility is, when loading a duplicate cert, instead of discarding it completely, merge the tag list into the one that's already stored in the cache, then discard.

Matthew Holt committed Feb 20, 2020 at 10:18 UTC 99f91c4c6f812ebfae505a8c29a750965af0cfcb
1 file changed +35 -9
caddyconfig/httpcaddyfile/builtins.go
+35 -9
@@ -132,20 +132,41 @@ func parseTLS(h Helper) ([]ConfigValue, error) {
132 }
133 mgr.Email = firstLine[0]
134 case 2:
135 - tag := fmt.Sprintf("cert%d", tagCounter)
136 - fileLoader = append(fileLoader, caddytls.CertKeyFilePair{
137 - Certificate: firstLine[0],
138 - Key: firstLine[1],
139 - Tags: []string{tag},
140 - })
135 + certFilename := firstLine[0]
136 + keyFilename := firstLine[1]
137 +
138 // tag this certificate so if multiple certs match, specifically
139 // this one that the user has provided will be used, see #2588:
143 - // https://github.com/caddyserver/caddy/issues/2588
144 - tagCounter++
140 + // https://github.com/caddyserver/caddy/issues/2588 ... but we
141 + // must be careful about how we do this; being careless will
142 + // lead to failed handshakes
143 +
144 + // we need to remember which cert files we've seen, since we
145 + // must load each cert only once; otherwise, they each get a
146 + // different tag... since a cert loaded twice has the same
147 + // bytes, it will overwrite the first one in the cache, and
148 + // only the last cert (and its tag) will survive, so a any conn
149 + // policy that is looking for any tag but the last one to be
150 + // loaded won't find it, and TLS handshakes will fail (see end)
151 + // of issue #3004)
152 + tag, ok := tlsCertTags[certFilename]
153 + if !ok {
154 + // haven't seen this cert file yet, let's give it a tag
155 + // and add a loader for it
156 + tag = fmt.Sprintf("cert%d", len(tlsCertTags))
157 + fileLoader = append(fileLoader, caddytls.CertKeyFilePair{
158 + Certificate: certFilename,
159 + Key: keyFilename,
160 + Tags: []string{tag},
161 + })
162 + // remember this for next time we see this cert file
163 + tlsCertTags[certFilename] = tag
164 + }
165 certSelector := caddytls.CustomCertSelectionPolicy{Tag: tag}
166 if cp == nil {
167 cp = new(caddytls.ConnectionPolicy)
168 }
169 +
170 cp.CertSelection = caddyconfig.JSONModuleObject(certSelector, "policy", "custom", h.warnings)
171 default:
172 return nil, h.ArgErr()
@@ -405,4 +426,9 @@ func parseHandleErrors(h Helper) ([]ConfigValue, error) {
426 }, nil
427 }
428
408 -var tagCounter = 0
429 +// tlsCertTags maps certificate filenames to their tag.
430 +// This is used to remember which tag is used for each
431 +// certificate files, since we need to avoid loading
432 +// the same certificate files more than once, overwriting
433 +// previous tags
434 +var tlsCertTags = make(map[string]string)