From 0335715da7bbd1df7726908a661896f68721bce8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sim=C3=A3o=20Gomes=20Viana?= Date: Tue, 26 Oct 2021 10:14:52 +0200 Subject: [PATCH] crypto: check keyBlockDER for nil I got following panic while Caddy was running: 2021/10/26 08:06:34 panic: certificate worker: runtime error: invalid memory address or nil pointer dereference goroutine 43 [running]: github.com/caddyserver/certmagic.(*jobManager).worker.func1() github.com/caddyserver/certmagic@v0.14.5/async.go:58 +0x65 panic({0x145d400, 0x23d6c50}) runtime/panic.go:1038 +0x215 github.com/caddyserver/certmagic.decodePrivateKey({0xc000738c00, 0x0, 0x0}) github.com/caddyserver/certmagic@v0.14.5/crypto.go:75 +0x2a github.com/caddyserver/certmagic.(*Config).reusePrivateKey(0xc0003b77c0, {0xc0003b1640, 0x32}) github.com/caddyserver/certmagic@v0.14.5/config.go:602 +0x2b9 github.com/caddyserver/certmagic.(*Config).obtainCert.func2({0x190d3b8, 0xc000655920}) github.com/caddyserver/certmagic@v0.14.5/config.go:487 +0x1d6 github.com/caddyserver/certmagic.doWithRetry({0x190d310, 0xc0000b0440}, 0xc00003bd40, 0xc0007afba8) github.com/caddyserver/certmagic@v0.14.5/async.go:106 +0x1cc github.com/caddyserver/certmagic.(*Config).obtainCert(0xc0003b77c0, {0x190d310, 0xc0000b0440}, {0xc0003b1640, 0x32}, 0x0) github.com/caddyserver/certmagic@v0.14.5/config.go:572 +0x58e github.com/caddyserver/certmagic.(*Config).ObtainCertAsync(...) github.com/caddyserver/certmagic@v0.14.5/config.go:427 github.com/caddyserver/certmagic.(*Config).manageOne.func1() github.com/caddyserver/certmagic@v0.14.5/config.go:332 +0x6f github.com/caddyserver/certmagic.(*jobManager).worker(0x23e0c60) github.com/caddyserver/certmagic@v0.14.5/async.go:73 +0x112 created by github.com/caddyserver/certmagic.(*jobManager).Submit github.com/caddyserver/certmagic@v0.14.5/async.go:50 +0x288 According to Go documentation: https://pkg.go.dev/encoding/pem#Decode p can be nil (first parameter returned) and so it should be checked before continuing as per this example: https://pkg.go.dev/encoding/pem#example-Decode I also added a test to verify that the fix works. Running the test without the fix causes a panic. Test: go test -count=1 './...' --- crypto.go | 4 ++++ crypto_test.go | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/crypto.go b/crypto.go index a705cdde..558c18cf 100644 --- a/crypto.go +++ b/crypto.go @@ -72,6 +72,10 @@ func encodePrivateKey(key crypto.PrivateKey) ([]byte, error) { func decodePrivateKey(keyPEMBytes []byte) (crypto.Signer, error) { keyBlockDER, _ := pem.Decode(keyPEMBytes) + if keyBlockDER == nil { + return nil, fmt.Errorf("failed to decode PEM block containing private key") + } + if keyBlockDER.Type != "PRIVATE KEY" && !strings.HasSuffix(keyBlockDER.Type, " PRIVATE KEY") { return nil, fmt.Errorf("unknown PEM header %q", keyBlockDER.Type) } diff --git a/crypto_test.go b/crypto_test.go index 71336ef6..e78c85b7 100644 --- a/crypto_test.go +++ b/crypto_test.go @@ -44,6 +44,12 @@ func TestEncodeDecodeRSAPrivateKey(t *testing.T) { t.Error("error loading private key:", err) } + // test load (should fail) + _, err = decodePrivateKey(savedBytes[2:]) + if err == nil { + t.Error("loading private key should have failed") + } + // verify loaded key is correct if !privateKeysSame(privateKey, loadedKey) { t.Error("Expected key bytes to be the same, but they weren't")