Repository: caddyserver/caddy (language: go)

Issue:
Caddy fails to renew certificate for admin interface
with the configuration at the bottom, caddy renews the internal certificate for the admin interface, but fails to reload it. instead the following error is logged:

```
panic: certificate worker: runtime error: invalid memory address or nil pointer dereference
goroutine 35 [running]:
github.com/caddyserver/certmagic.(*jobManager).worker.func1()
	github.com/caddyserver/certmagic@v0.17.2/async.go:58 +0x65
panic({0x18763c0, 0x2a9a9c0})
	runtime/panic.go:884 +0x212
github.com/caddyserver/certmagic.(*Cache).replaceCertificate(_, {{{0xc000348660, 0x2, 0x2}, {0x199a3e0, 0xc000348870}, {0x0, 0x0, 0x0}, {0x0, ...}, ...}, ...}, ...)
	github.com/caddyserver/certmagic@v0.17.2/cache.go:292 +0x31
github.com/caddyserver/certmagic.(*Config).reloadManagedCertificate(_, {_, _}, {{{0xc000348660, 0x2, 0x2}, {0x199a3e0, 0xc000348870}, {0x0, 0x0, ...}, ...}, ...})
	github.com/caddyserver/certmagic@v0.17.2/certificates.go:341 +0x418
github.com/caddyserver/certmagic.(*Cache).queueRenewalTask.func1()
	github.com/caddyserver/certmagic@v0.17.2/maintain.go:240 +0x510
github.com/caddyserver/certmagic.(*jobManager).worker(0x2aa5720)
	github.com/caddyserver/certmagic@v0.17.2/async.go:73 +0x11b
created by github.com/caddyserver/certmagic.(*jobManager).Submit
	github.com/caddyserver/certmagic@v0.17.2/async.go:50 +0x29b
```

not sure if this is a bug in caddy or certmagic but since it can be triggered with the following caddy configuration, created it as a caddy issue.
 
```json
{
  "admin": {
    "identity": {
      "identifiers": [
        "127.0.0.2"
      ],
      "issuers": [
        {
          "ca": "local",
          "lifetime": "3m",
          "module": "internal",
          "sign_with_root": false
        }
      ]
    },
    "remote": {
      "listen": "127.0.0.2:2021"
    }
  },
  "apps": {
    "http": {
      "servers": {}
    },
    "pki": {
      "certificate_authorities": {
        "local": {
          "install_trust": false,
          "intermediate_common_name": "Example Intermediate CA",
          "name": "Example CA",
          "root_common_name": "ca.example.bug"
        }
      }
    }
  },
  "logging": {
    "logs": {
      "default": {
        "level": "DEBUG"
      }
    }
  }
}
```


Discussion / hints:
I'm mobile now but I think this is already fixed! Try a build from master.
still happens with build from master:
```
panic: certificate worker: runtime error: invalid memory address or nil pointer dereference
goroutine 35 [running]:
github.com/caddyserver/certmagic.(*jobManager).worker.func1()
	github.com/caddyserver/certmagic@v0.17.2/async.go:58 +0x65
panic({0x18763c0, 0x2a9b9e0})
	runtime/panic.go:884 +0x212
github.com/caddyserver/certmagic.(*Cache).replaceCertificate(_, {{{0xc00066e270, 0x2, 0x2}, {0x199a3e0, 0xc00066e3c0}, {0x0, 0x0, 0x0}, {0x0, ...}, ...}, ...}, ...)
	github.com/caddyserver/certmagic@v0.17.2/cache.go:292 +0x31
github.com/caddyserver/certmagic.(*Config).reloadManagedCertificate(_, {_, _}, {{{0xc00066e270, 0x2, 0x2}, {0x199a3e0, 0xc00066e3c0}, {0x0, 0x0, ...}, ...}, ...})
	github.com/caddyserver/certmagic@v0.17.2/certificates.go:341 +0x418
github.com/caddyserver/certmagic.(*Cache).queueRenewalTask.func1()
	github.com/caddyserver/certmagic@v0.17.2/maintain.go:240 +0x510
github.com/caddyserver/certmagic.(*jobManager).worker(0x2aa6740)
	github.com/caddyserver/certmagic@v0.17.2/async.go:73 +0x11b
created by github.com/caddyserver/certmagic.(*jobManager).Submit
	github.com/caddyserver/certmagic@v0.17.2/async.go:50 +0x29b
```
Oh, sorry, I should have clarified, the bug is in CertMagic so you'd have to build with the latest in CertMagic. (Sorry, I'm still mobile all day today)
@mholt CertMagic has no unreleased commits on master. Latest is 0.17.2 which is already in Caddy 2.6.2
the problem seams to be that cfg.certCache is nil
setting it in getConfig in certmagic fixes the issue for me, but not sure if that is the right place for it:

```patch
diff --git a/cache.go b/cache.go
index f5ab868..5f0e5a9 100644
--- a/cache.go
+++ b/cache.go
@@ -331,6 +331,9 @@ func (certCache *Cache) getConfig(cert Certificate) (*Config, error) {
                return nil, fmt.Errorf("config returned for certificate %v is not nil and points to different cache; got %p, expected %p (this one)",
                        cert.Names, cfg.certCache, certCache)
        }
+       if cfg.certCache == nil {
+               cfg.certCache = certCache
+       }
        return cfg, nil
 }
```
@bit Thanks for the investigation and patch!

I'm also looking into this today. I want to understand things better before patching. It might actually be a bug in Caddy's `admin.go` file.

The panic says that the certCache is nil.

The admin server sets the certCache to a non-nil value at the end of every config load, when Caddy calls `manageIdentity()` it eventually winds up here:

https://github.com/caddyserver/caddy/blob/bbe366316771ea23381a473a528e71a72a7148a5/admin.go#L456

Note that `true` parameter, which tells Caddy to create a cache value:

https://github.com/caddyserver/caddy/blob/bbe366316771ea23381a473a528e71a72a7148a5/admin.go#L585-L591

That shouldn't be the source of the bug, since it ensures the cache is non-nil.

There's two places where `certmagicConfig()` is called with `false` however. One is in `replaceRemoteAdminServer()`:

https://github.com/caddyserver/caddy/blob/bbe366316771ea23381a473a528e71a72a7148a5/admin.go#L522-L525

This is called after `manageIdentity()` and the cache should already be non-nil, and we even do an error check for that specific case. I imagine that's not the bug either.

The _only_ other place we call `certmagicConfig()` with false is here:

https://github.com/caddyserver/caddy/blob/bbe366316771ea23381a473a528e71a72a7148a5/admin.go#L595-L610

This is an exported method, and it's _only_ called by the HTTPLoader (a config loader), which you're not using:

https://github.com/caddyserver/caddy/blob/bbe366316771ea23381a473a528e71a72a7148a5/caddyconfig/httploader.go#L163

Since you're not using the HTTP loader, and the other path checks for a nil cache, and the first path always creates the cache, I'm genuinely unsure how this panic is happening.

Do you have any idea?
While tracking this down I found out that getConfig gets called per certificate, and we have a config per issuer. It seams that by default that config has no cache. For ACME this would be hidden since the config is set via `annoying.SetConfig(cmCfg)` in `manageIdentity` but the internal issuer does not have a SetConfig interface. But I don't fully understand why the config exists in multiple locations and if setting cache to the outer default as done with my patch will cause issues in other branches.


@bit You are right, the internal issuer is being used (since in your case the identity is internal; loopback). The internal issuer does not need the cache, however. The ACME issuer needs the cache because it has to use it to solve ACME challenges. The internal issue essentially just returns self-signed certificates (for some definition of 'self-signed'), so no cache is needed.

IMO the underlying issuer should be irrelevant -- the config is there, but it just doesn't have a cache. So, either, where did the config come from, or why doesn't it have a cache? :monocle_face: 
had another look at certmagicConfig, the cmCfg returned from New should contain the cache, since that is where it gets set. and indeed keeping that and returning it in GetConfigForCert seams to work:

```patch
diff --git a/admin.go b/admin.go
index db46e713..01cf3c1b 100644
--- a/admin.go
+++ b/admin.go
@@ -589,7 +589,8 @@ func (ident *IdentityConfig) certmagicConfig(logger *zap.Logger, makeCache bool)
                        },
                })
        }
-       return certmagic.New(identityCertCache, *cmCfg)
+       cmCfg = certmagic.New(identityCertCache, *cmCfg)
+       return cmCfg
 }
 
 // IdentityCredentials returns this instance's configured, managed identity credentials
```

Fix patch:
diff --git a/admin.go b/admin.go
index db46e713c42..4b107ad05e5 100644
--- a/admin.go
+++ b/admin.go
@@ -572,12 +572,13 @@ func replaceRemoteAdminServer(ctx Context, cfg *Config) error {
 }
 
 func (ident *IdentityConfig) certmagicConfig(logger *zap.Logger, makeCache bool) *certmagic.Config {
+	var cmCfg *certmagic.Config
 	if ident == nil {
 		// user might not have configured identity; that's OK, we can still make a
 		// certmagic config, although it'll be mostly useless for remote management
 		ident = new(IdentityConfig)
 	}
-	cmCfg := &certmagic.Config{
+	template := certmagic.Config{
 		Storage: DefaultStorage, // do not act as part of a cluster (this is for the server's local identity)
 		Logger:  logger,
 		Issuers: ident.issuers,
@@ -589,7 +590,8 @@ func (ident *IdentityConfig) certmagicConfig(logger *zap.Logger, makeCache bool)
 			},
 		})
 	}
-	return certmagic.New(identityCertCache, *cmCfg)
+	cmCfg = certmagic.New(identityCertCache, template)
+	return cmCfg
 }
 
 // IdentityCredentials returns this instance's configured, managed identity credentials


Test patch: