fix(server): improve error message for encrypted TLS private keys (#4742)

Added TLS certificate validation that detects encrypted (password-protected)
private keys and provides a clear error message with instructions on how to
decrypt them using openssl. This addresses user confusion when Go's standard
library fails with the cryptic 'tls: failed to parse private key' error.

Changes:
- Added validateTLSCertificates function to validate certs before server start
- Added isEncryptedPEM helper to detect both PKCS#8 and legacy encrypted keys
- Added comprehensive tests for TLS validation including encrypted key detection
- Added integration test that starts server with TLS and verifies HTTPS works
- Added test certificates (valid for 100 years) with SAN for localhost

Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan Quintão
2025-11-28 17:08:34 -05:00
committed by GitHub
parent a87b6a50a6
commit 9913235542
7 changed files with 352 additions and 6 deletions
+150
View File
@@ -1,13 +1,20 @@
package server
import (
"context"
"crypto/tls"
"crypto/x509"
"fmt"
"io/fs"
"net/http"
"net/url"
"os"
"path/filepath"
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/tests"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
@@ -107,3 +114,146 @@ var _ = Describe("createUnixSocketFile", func() {
})
})
})
var _ = Describe("TLS support", func() {
Describe("validateTLSCertificates", func() {
const testDataDir = "server/testdata"
When("certificate and key are valid and unencrypted", func() {
It("returns nil", func() {
certFile := filepath.Join(testDataDir, "test_cert.pem")
keyFile := filepath.Join(testDataDir, "test_key.pem")
err := validateTLSCertificates(certFile, keyFile)
Expect(err).ToNot(HaveOccurred())
})
})
When("private key is encrypted with PKCS#8 format", func() {
It("returns an error with helpful message", func() {
certFile := filepath.Join(testDataDir, "test_cert_encrypted.pem")
keyFile := filepath.Join(testDataDir, "test_key_encrypted.pem")
err := validateTLSCertificates(certFile, keyFile)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("encrypted"))
Expect(err.Error()).To(ContainSubstring("openssl"))
})
})
When("private key is encrypted with legacy format (Proc-Type header)", func() {
It("returns an error with helpful message", func() {
certFile := filepath.Join(testDataDir, "test_cert.pem")
keyFile := filepath.Join(testDataDir, "test_key_encrypted_legacy.pem")
err := validateTLSCertificates(certFile, keyFile)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("encrypted"))
Expect(err.Error()).To(ContainSubstring("openssl"))
})
})
When("key file does not exist", func() {
It("returns an error", func() {
certFile := filepath.Join(testDataDir, "test_cert.pem")
keyFile := filepath.Join(testDataDir, "nonexistent.pem")
err := validateTLSCertificates(certFile, keyFile)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("reading TLS key file"))
})
})
When("key file does not contain valid PEM", func() {
It("returns an error", func() {
// Create a temp file with invalid PEM content
tmpFile, err := os.CreateTemp("", "invalid_key*.pem")
Expect(err).ToNot(HaveOccurred())
DeferCleanup(func() {
_ = os.Remove(tmpFile.Name())
})
_, err = tmpFile.WriteString("not a valid PEM file")
Expect(err).ToNot(HaveOccurred())
_ = tmpFile.Close()
certFile := filepath.Join(testDataDir, "test_cert.pem")
err = validateTLSCertificates(certFile, tmpFile.Name())
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("valid PEM block"))
})
})
When("certificate file does not exist", func() {
It("returns an error from tls.LoadX509KeyPair", func() {
certFile := filepath.Join(testDataDir, "nonexistent_cert.pem")
keyFile := filepath.Join(testDataDir, "test_key.pem")
err := validateTLSCertificates(certFile, keyFile)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("loading TLS certificate/key pair"))
})
})
})
Describe("Server TLS", func() {
const testDataDir = "server/testdata"
When("server is started with valid TLS certificates", func() {
It("accepts HTTPS connections", func() {
DeferCleanup(configtest.SetupConfig())
// Create server with mock dependencies
ds := &tests.MockDataStore{}
server := New(ds, nil, nil)
// Load the test certificate to create a trusted CA pool
certFile := filepath.Join(testDataDir, "test_cert.pem")
keyFile := filepath.Join(testDataDir, "test_key.pem")
caCert, err := os.ReadFile(certFile)
Expect(err).ToNot(HaveOccurred())
caCertPool := x509.NewCertPool()
caCertPool.AppendCertsFromPEM(caCert)
// Create an HTTPS client that trusts our test certificate
httpClient := &http.Client{
Timeout: 5 * time.Second,
Transport: &http.Transport{
TLSClientConfig: &tls.Config{
RootCAs: caCertPool,
MinVersion: tls.VersionTLS12,
},
},
}
// Start the server in a goroutine
ctx, cancel := context.WithCancel(GinkgoT().Context())
defer cancel()
errChan := make(chan error, 1)
go func() {
errChan <- server.Run(ctx, "127.0.0.1", 14534, certFile, keyFile)
}()
Eventually(func() error {
// Make an HTTPS request to the server
resp, err := httpClient.Get("https://127.0.0.1:14534/ping")
if err != nil {
return err
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
return fmt.Errorf("unexpected status code: %d", resp.StatusCode)
}
return nil
}, 2*time.Second, 100*time.Millisecond).Should(Succeed())
// Stop the server
cancel()
// Wait for server to stop (with timeout)
select {
case <-errChan:
// Server stopped
case <-time.After(2 * time.Second):
Fail("Server did not stop in time")
}
})
})
})
})