From 1167a47b4ef33b08cc8b999f777bd2972d7309fd Mon Sep 17 00:00:00 2001 From: Matthew McPherrin Date: Fri, 25 Oct 2024 14:08:27 -0400 Subject: [PATCH 1/5] Allow admin command to revoke from a CSR file --- cmd/admin/key.go | 49 ++++++++++++++++++++++++++++++++++++++++--- cmd/admin/key_test.go | 30 ++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/cmd/admin/key.go b/cmd/admin/key.go index 66da63ebeef..1a58ff451f3 100644 --- a/cmd/admin/key.go +++ b/cmd/admin/key.go @@ -3,7 +3,9 @@ package main import ( "bufio" "context" + "crypto/x509" "encoding/hex" + "encoding/pem" "errors" "flag" "fmt" @@ -26,9 +28,13 @@ import ( type subcommandBlockKey struct { parallelism uint comment string - privKey string - spkiFile string - certFile string + + privKey string + spkiFile string + certFile string + csrFile string + + checkSignature bool } var _ subcommand = (*subcommandBlockKey)(nil) @@ -46,6 +52,9 @@ func (s *subcommandBlockKey) Flags(flag *flag.FlagSet) { flag.StringVar(&s.privKey, "private-key", "", "Block issuance for the pubkey corresponding to this private key") flag.StringVar(&s.spkiFile, "spki-file", "", "Block issuance for all keys listed in this file as SHA256 hashes of SPKI, hex encoded, one per line") flag.StringVar(&s.certFile, "cert-file", "", "Block issuance for the public key of the single PEM-formatted certificate in this file") + flag.StringVar(&s.csrFile, "csr-file", "", "Block issuance for the public key of the single PEM-formatted CSR in this file") + + flag.BoolVar(&s.checkSignature, "check-signature", true, "Check self-signature of CSR before revoking") } func (s *subcommandBlockKey) Run(ctx context.Context, a *admin) error { @@ -56,6 +65,7 @@ func (s *subcommandBlockKey) Run(ctx context.Context, a *admin) error { "-private-key": s.privKey != "", "-spki-file": s.spkiFile != "", "-cert-file": s.certFile != "", + "-csr-file": s.csrFile != "", } maps.DeleteFunc(setInputs, func(_ string, v bool) bool { return !v }) if len(setInputs) == 0 { @@ -75,6 +85,8 @@ func (s *subcommandBlockKey) Run(ctx context.Context, a *admin) error { spkiHashes, err = a.spkiHashesFromFile(s.spkiFile) case "-cert-file": spkiHashes, err = a.spkiHashesFromCertPEM(s.certFile) + case "-csr-file": + spkiHashes, err = spkiHashFromCSRPEM(s.csrFile, s.checkSignature) default: return errors.New("no recognized input method flag set (this shouldn't happen)") } @@ -146,6 +158,37 @@ func (a *admin) spkiHashesFromCertPEM(filename string) ([][]byte, error) { return [][]byte{spkiHash[:]}, nil } +func spkiHashFromCSRPEM(filename string, checkSignature bool) ([][]byte, error) { + csrFile, err := os.ReadFile(filename) + if err != nil { + return nil, fmt.Errorf("reading CSR file %q: %w", filename, err) + } + + data, _ := pem.Decode(csrFile) + if data == nil { + return nil, fmt.Errorf("no PEM data found in %q", filename) + } + + csr, err := x509.ParseCertificateRequest(data.Bytes) + if err != nil { + return nil, fmt.Errorf("parsing CSR %q: %w", filename, err) + } + + if checkSignature { + err = csr.CheckSignature() + if err != nil { + return nil, fmt.Errorf("checking CSR signature: %w", err) + } + } + + spkiHash, err := core.KeyDigest(csr.PublicKey) + if err != nil { + return nil, fmt.Errorf("computing SPKI hash: %w", err) + } + + return [][]byte{spkiHash[:]}, nil +} + func (a *admin) blockSPKIHashes(ctx context.Context, spkiHashes [][]byte, comment string, parallelism uint) error { u, err := user.Current() if err != nil { diff --git a/cmd/admin/key_test.go b/cmd/admin/key_test.go index 0bb19223609..40ce3ed3325 100644 --- a/cmd/admin/key_test.go +++ b/cmd/admin/key_test.go @@ -68,6 +68,36 @@ func TestSPKIHashesFromFile(t *testing.T) { } } +// This CSR has had its final bit flipped in the signature +// The key is the p256 test key from RFC9500 +const badCSR = ` +-----BEGIN CERTIFICATE REQUEST----- +MIG6MGICAQAwADBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABEIlSPiPt4L/teyj +dERSxyoeVY+9b3O+XkjpMjLMRcWxbEzRDEy41bihcTnpSILImSVymTQl9BQZq36Q +pCpJQnKgADAKBggqhkjOPQQDAgNIADBFAiBadw3gvL9IjUfASUTa7MvmkbC4ZCvl +21m1KMwkIx/+CQIhAKvuyfCcdZ0cWJYOXCOb1OavolWHIUzgEpNGUWul6O0t +-----END CERTIFICATE REQUEST----- +` + +// TestCSR checks that we get the correct SPKI from a CSR, even if its signature is invalid +func TestCSR(t *testing.T) { + csrFile := path.Join(t.TempDir(), "bad.csr") + err := os.WriteFile(csrFile, []byte(badCSR), 0600) + test.AssertNotError(t, err, "writing bad csr") + + _, err = spkiHashFromCSRPEM(csrFile, true) + test.AssertError(t, err, "expected invalid signature") + + hashes, err := spkiHashFromCSRPEM(csrFile, false) + test.AssertNotError(t, err, "expected to read CSR with bad signature") + + if len(hashes) != 1 { + t.Fatalf("expected to read 1 SPKI from CSR, read %d", len(hashes)) + } + expected := "b2b04340cfaee616ec9c2c62d261b208e54bb197498df52e8cadede23ac0ba5e" + test.AssertEquals(t, hex.EncodeToString(hashes[0]), expected) +} + // mockSARecordingBlocks is a mock which only implements the AddBlockedKey gRPC // method. type mockSARecordingBlocks struct { From 8f473ead3e74dbc994476e7e3c5b4a0b87d8170b Mon Sep 17 00:00:00 2001 From: Matthew McPherrin Date: Fri, 25 Oct 2024 15:48:01 -0400 Subject: [PATCH 2/5] Extend test to include a good CSR --- cmd/admin/key_test.go | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/cmd/admin/key_test.go b/cmd/admin/key_test.go index 40ce3ed3325..c5f7a1e6080 100644 --- a/cmd/admin/key_test.go +++ b/cmd/admin/key_test.go @@ -68,19 +68,35 @@ func TestSPKIHashesFromFile(t *testing.T) { } } -// This CSR has had its final bit flipped in the signature // The key is the p256 test key from RFC9500 -const badCSR = ` +const goodCSR = ` -----BEGIN CERTIFICATE REQUEST----- MIG6MGICAQAwADBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABEIlSPiPt4L/teyj dERSxyoeVY+9b3O+XkjpMjLMRcWxbEzRDEy41bihcTnpSILImSVymTQl9BQZq36Q pCpJQnKgADAKBggqhkjOPQQDAgNIADBFAiBadw3gvL9IjUfASUTa7MvmkbC4ZCvl -21m1KMwkIx/+CQIhAKvuyfCcdZ0cWJYOXCOb1OavolWHIUzgEpNGUWul6O0t +21m1KMwkIx/+CQIhAKvuyfCcdZ0cWJYOXCOb1OavolWHIUzgEpNGUWul6O0s -----END CERTIFICATE REQUEST----- ` // TestCSR checks that we get the correct SPKI from a CSR, even if its signature is invalid func TestCSR(t *testing.T) { + expectedSPKIHash := "b2b04340cfaee616ec9c2c62d261b208e54bb197498df52e8cadede23ac0ba5e" + + goodCSRFile := path.Join(t.TempDir(), "good.csr") + err := os.WriteFile(goodCSRFile, []byte(goodCSR), 0600) + test.AssertNotError(t, err, "writing good csr") + + goodHash, err := spkiHashFromCSRPEM(goodCSRFile, true) + test.AssertNotError(t, err, "expected to read CSR") + + if len(goodHash) != 1 { + t.Fatalf("expected to read 1 SPKI from CSR, read %d", len(goodHash)) + } + test.AssertEquals(t, hex.EncodeToString(goodHash[0]), expectedSPKIHash) + + // Flip a bit, in the signature, to make a bad CSR: + badCSR := strings.Replace(goodCSR, "Wul6", "Wul7", 1) + csrFile := path.Join(t.TempDir(), "bad.csr") err := os.WriteFile(csrFile, []byte(badCSR), 0600) test.AssertNotError(t, err, "writing bad csr") @@ -88,14 +104,13 @@ func TestCSR(t *testing.T) { _, err = spkiHashFromCSRPEM(csrFile, true) test.AssertError(t, err, "expected invalid signature") - hashes, err := spkiHashFromCSRPEM(csrFile, false) + badHash, err := spkiHashFromCSRPEM(csrFile, false) test.AssertNotError(t, err, "expected to read CSR with bad signature") - if len(hashes) != 1 { - t.Fatalf("expected to read 1 SPKI from CSR, read %d", len(hashes)) + if len(badHash) != 1 { + t.Fatalf("expected to read 1 SPKI from CSR, read %d", len(badHash)) } - expected := "b2b04340cfaee616ec9c2c62d261b208e54bb197498df52e8cadede23ac0ba5e" - test.AssertEquals(t, hex.EncodeToString(hashes[0]), expected) + test.AssertEquals(t, hex.EncodeToString(badHash[0]), expectedSPKIHash) } // mockSARecordingBlocks is a mock which only implements the AddBlockedKey gRPC From 56bc6c1b7cb794486160bd9bd8d562ee47f049dd Mon Sep 17 00:00:00 2001 From: Matthew McPherrin Date: Fri, 25 Oct 2024 16:11:23 -0400 Subject: [PATCH 3/5] Typo --- cmd/admin/key_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/admin/key_test.go b/cmd/admin/key_test.go index c5f7a1e6080..e2200d8e3ef 100644 --- a/cmd/admin/key_test.go +++ b/cmd/admin/key_test.go @@ -98,7 +98,7 @@ func TestCSR(t *testing.T) { badCSR := strings.Replace(goodCSR, "Wul6", "Wul7", 1) csrFile := path.Join(t.TempDir(), "bad.csr") - err := os.WriteFile(csrFile, []byte(badCSR), 0600) + err = os.WriteFile(csrFile, []byte(badCSR), 0600) test.AssertNotError(t, err, "writing bad csr") _, err = spkiHashFromCSRPEM(csrFile, true) From af51f6e388432f9dd1bb1f0049d604457729f6f5 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 1 Nov 2024 14:46:23 -0700 Subject: [PATCH 4/5] Check for expected CN --- cmd/admin/key.go | 18 ++++++++++++------ cmd/admin/key_test.go | 6 +++--- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/cmd/admin/key.go b/cmd/admin/key.go index 1a58ff451f3..19c5575c168 100644 --- a/cmd/admin/key.go +++ b/cmd/admin/key.go @@ -29,10 +29,11 @@ type subcommandBlockKey struct { parallelism uint comment string - privKey string - spkiFile string - certFile string - csrFile string + privKey string + spkiFile string + certFile string + csrFile string + csrFileExpectedCN string checkSignature bool } @@ -53,6 +54,7 @@ func (s *subcommandBlockKey) Flags(flag *flag.FlagSet) { flag.StringVar(&s.spkiFile, "spki-file", "", "Block issuance for all keys listed in this file as SHA256 hashes of SPKI, hex encoded, one per line") flag.StringVar(&s.certFile, "cert-file", "", "Block issuance for the public key of the single PEM-formatted certificate in this file") flag.StringVar(&s.csrFile, "csr-file", "", "Block issuance for the public key of the single PEM-formatted CSR in this file") + flag.StringVar(&s.csrFileExpectedCN, "csr-file-expected-cn", "The key that signed this CSR has been publicly disclosed. It should not be used for any purpose.", "The Subject CN of a CSR will be verified to match this before blocking") flag.BoolVar(&s.checkSignature, "check-signature", true, "Check self-signature of CSR before revoking") } @@ -86,7 +88,7 @@ func (s *subcommandBlockKey) Run(ctx context.Context, a *admin) error { case "-cert-file": spkiHashes, err = a.spkiHashesFromCertPEM(s.certFile) case "-csr-file": - spkiHashes, err = spkiHashFromCSRPEM(s.csrFile, s.checkSignature) + spkiHashes, err = spkiHashFromCSRPEM(s.csrFile, s.checkSignature, s.csrFileExpectedCN) default: return errors.New("no recognized input method flag set (this shouldn't happen)") } @@ -158,7 +160,7 @@ func (a *admin) spkiHashesFromCertPEM(filename string) ([][]byte, error) { return [][]byte{spkiHash[:]}, nil } -func spkiHashFromCSRPEM(filename string, checkSignature bool) ([][]byte, error) { +func spkiHashFromCSRPEM(filename string, checkSignature bool, expectedCN string) ([][]byte, error) { csrFile, err := os.ReadFile(filename) if err != nil { return nil, fmt.Errorf("reading CSR file %q: %w", filename, err) @@ -181,6 +183,10 @@ func spkiHashFromCSRPEM(filename string, checkSignature bool) ([][]byte, error) } } + if csr.Subject.CommonName != expectedCN { + return nil, fmt.Errorf("Got CSR CommonName %q, expected %q", csr.Subject.CommonName, expectedCN) + } + spkiHash, err := core.KeyDigest(csr.PublicKey) if err != nil { return nil, fmt.Errorf("computing SPKI hash: %w", err) diff --git a/cmd/admin/key_test.go b/cmd/admin/key_test.go index e2200d8e3ef..136212a2ad6 100644 --- a/cmd/admin/key_test.go +++ b/cmd/admin/key_test.go @@ -86,7 +86,7 @@ func TestCSR(t *testing.T) { err := os.WriteFile(goodCSRFile, []byte(goodCSR), 0600) test.AssertNotError(t, err, "writing good csr") - goodHash, err := spkiHashFromCSRPEM(goodCSRFile, true) + goodHash, err := spkiHashFromCSRPEM(goodCSRFile, true, "") test.AssertNotError(t, err, "expected to read CSR") if len(goodHash) != 1 { @@ -101,10 +101,10 @@ func TestCSR(t *testing.T) { err = os.WriteFile(csrFile, []byte(badCSR), 0600) test.AssertNotError(t, err, "writing bad csr") - _, err = spkiHashFromCSRPEM(csrFile, true) + _, err = spkiHashFromCSRPEM(csrFile, true, "") test.AssertError(t, err, "expected invalid signature") - badHash, err := spkiHashFromCSRPEM(csrFile, false) + badHash, err := spkiHashFromCSRPEM(csrFile, false, "") test.AssertNotError(t, err, "expected to read CSR with bad signature") if len(badHash) != 1 { From bd8e55495079cdc4e155b407b36da5b975c0936e Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 1 Nov 2024 14:52:57 -0700 Subject: [PATCH 5/5] Audit log CSR bytes. --- cmd/admin/key.go | 6 ++++-- cmd/admin/key_test.go | 8 +++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/cmd/admin/key.go b/cmd/admin/key.go index 19c5575c168..250eb0225d3 100644 --- a/cmd/admin/key.go +++ b/cmd/admin/key.go @@ -88,7 +88,7 @@ func (s *subcommandBlockKey) Run(ctx context.Context, a *admin) error { case "-cert-file": spkiHashes, err = a.spkiHashesFromCertPEM(s.certFile) case "-csr-file": - spkiHashes, err = spkiHashFromCSRPEM(s.csrFile, s.checkSignature, s.csrFileExpectedCN) + spkiHashes, err = a.spkiHashFromCSRPEM(s.csrFile, s.checkSignature, s.csrFileExpectedCN) default: return errors.New("no recognized input method flag set (this shouldn't happen)") } @@ -160,7 +160,7 @@ func (a *admin) spkiHashesFromCertPEM(filename string) ([][]byte, error) { return [][]byte{spkiHash[:]}, nil } -func spkiHashFromCSRPEM(filename string, checkSignature bool, expectedCN string) ([][]byte, error) { +func (a *admin) spkiHashFromCSRPEM(filename string, checkSignature bool, expectedCN string) ([][]byte, error) { csrFile, err := os.ReadFile(filename) if err != nil { return nil, fmt.Errorf("reading CSR file %q: %w", filename, err) @@ -171,6 +171,8 @@ func spkiHashFromCSRPEM(filename string, checkSignature bool, expectedCN string) return nil, fmt.Errorf("no PEM data found in %q", filename) } + a.log.AuditInfof("Parsing key to block from CSR PEM: %x", data) + csr, err := x509.ParseCertificateRequest(data.Bytes) if err != nil { return nil, fmt.Errorf("parsing CSR %q: %w", filename, err) diff --git a/cmd/admin/key_test.go b/cmd/admin/key_test.go index 136212a2ad6..ef4428c0a3c 100644 --- a/cmd/admin/key_test.go +++ b/cmd/admin/key_test.go @@ -86,7 +86,9 @@ func TestCSR(t *testing.T) { err := os.WriteFile(goodCSRFile, []byte(goodCSR), 0600) test.AssertNotError(t, err, "writing good csr") - goodHash, err := spkiHashFromCSRPEM(goodCSRFile, true, "") + a := admin{log: blog.NewMock()} + + goodHash, err := a.spkiHashFromCSRPEM(goodCSRFile, true, "") test.AssertNotError(t, err, "expected to read CSR") if len(goodHash) != 1 { @@ -101,10 +103,10 @@ func TestCSR(t *testing.T) { err = os.WriteFile(csrFile, []byte(badCSR), 0600) test.AssertNotError(t, err, "writing bad csr") - _, err = spkiHashFromCSRPEM(csrFile, true, "") + _, err = a.spkiHashFromCSRPEM(csrFile, true, "") test.AssertError(t, err, "expected invalid signature") - badHash, err := spkiHashFromCSRPEM(csrFile, false, "") + badHash, err := a.spkiHashFromCSRPEM(csrFile, false, "") test.AssertNotError(t, err, "expected to read CSR with bad signature") if len(badHash) != 1 {