From 0b4061f5533c3cb8f8a155137635cf8ea0ece668 Mon Sep 17 00:00:00 2001 From: Roger Ng Date: Tue, 7 May 2024 11:23:11 +0000 Subject: [PATCH 1/6] Add IssuanceChainStorage MySQL implementation --- go.mod | 1 + go.sum | 3 + trillian/ctfe/storage/mysql/mysql.go | 98 ++++++++++++++++++++ trillian/ctfe/storage/mysql/mysql_test.go | 108 ++++++++++++++++++++++ trillian/ctfe/storage/mysql/schema.sql | 24 +++++ 5 files changed, 234 insertions(+) create mode 100644 trillian/ctfe/storage/mysql/mysql.go create mode 100644 trillian/ctfe/storage/mysql/mysql_test.go create mode 100644 trillian/ctfe/storage/mysql/schema.sql diff --git a/go.mod b/go.mod index 84ac1d4536..2d9ec0e6e4 100644 --- a/go.mod +++ b/go.mod @@ -37,6 +37,7 @@ require ( cloud.google.com/go/monitoring v1.18.0 // indirect cloud.google.com/go/trace v1.10.5 // indirect contrib.go.opencensus.io/exporter/stackdriver v0.13.14 // indirect + github.com/DATA-DOG/go-sqlmock v1.5.2 // indirect github.com/aws/aws-sdk-go v1.46.4 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/bgentry/speakeasy v0.1.0 // indirect diff --git a/go.sum b/go.sum index ae56e07c3c..0f3a9b8585 100644 --- a/go.sum +++ b/go.sum @@ -14,6 +14,8 @@ contrib.go.opencensus.io/exporter/stackdriver v0.13.14 h1:zBakwHardp9Jcb8sQHcHpX contrib.go.opencensus.io/exporter/stackdriver v0.13.14/go.mod h1:5pSSGY0Bhuk7waTHuDf4aQ8D2DrhgETRo9fy6k3Xlzc= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/toml v1.3.2/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ= +github.com/DATA-DOG/go-sqlmock v1.5.2 h1:OcvFkGmslmlZibjAjaHm3L//6LiuBgolP7OputlJIzU= +github.com/DATA-DOG/go-sqlmock v1.5.2/go.mod h1:88MAG/4G7SMwSE3CeA0ZKzrT5CiOU3OJ+JlNzwDqpNU= github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY= github.com/aws/aws-sdk-go v1.46.4 h1:48tKgtm9VMPkb6y7HuYlsfhQmoIRAsTEXTsWLVlty4M= github.com/aws/aws-sdk-go v1.46.4/go.mod h1:aVsgQcEevwlmQ7qHE9I3h+dtQgpqhFB+i8Phjh7fkwI= @@ -156,6 +158,7 @@ github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnr github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= +github.com/kisielk/sqlstruct v0.0.0-20201105191214-5f3e10d3ab46/go.mod h1:yyMNCyc/Ib3bDTKd379tNMpB/7/H5TjM2Y9QJ5THLbE= github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= diff --git a/trillian/ctfe/storage/mysql/mysql.go b/trillian/ctfe/storage/mysql/mysql.go new file mode 100644 index 0000000000..072fbf6ec7 --- /dev/null +++ b/trillian/ctfe/storage/mysql/mysql.go @@ -0,0 +1,98 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package mysql defines the IssuanceChainStorage type, which implements IssuanceChainStorage interface with FindByKey and Add methods. +package mysql + +import ( + "context" + "database/sql" + "errors" + "strings" + + "k8s.io/klog/v2" + + "github.com/go-sql-driver/mysql" +) + +const ( + selectIssuanceChainByKeySQL = "SELECT c.ChainValue FROM IssuanceChain AS c WHERE c.IdentityHash = ?" + insertIssuanceChainSQL = "INSERT INTO IssuanceChain(IdentityHash, ChainValue) VALUES (?, ?)" +) + +type IssuanceChainStorage struct { + db *sql.DB +} + +// NewIssuanceChainStorage takes the database connection string as the input and return the IssuanceChainStorage. +func NewIssuanceChainStorage(ctx context.Context, dbConn string) *IssuanceChainStorage { + conn := strings.Split(dbConn, "://") + + db, err := open(ctx, conn[1]) + if err != nil { + klog.Errorf("failed to open database: %v", err) + return nil + } + + return &IssuanceChainStorage{ + db: db, + } +} + +// FindByKey returns the key-value pair of issuance chain by the key. +func (s *IssuanceChainStorage) FindByKey(ctx context.Context, key []byte) ([]byte, error) { + row := s.db.QueryRowContext(ctx, selectIssuanceChainByKeySQL, key) + if err := row.Err(); err != nil { + return nil, err + } + + var chain []byte + if err := row.Scan(&chain); err != nil { + return nil, err + } + + return chain, nil +} + +// Add inserts the key-value pair of issuance chain. +func (s *IssuanceChainStorage) Add(ctx context.Context, key []byte, chain []byte) error { + _, err := s.db.ExecContext(ctx, insertIssuanceChainSQL, key, chain) + if err != nil { + // Ignore duplicated key error. + var mysqlErr *mysql.MySQLError + if errors.As(err, &mysqlErr) && mysqlErr.Number == 1062 { + return nil + } + return err + } + + return nil +} + +// open takes the data source name and returns the sql.DB object. +func open(ctx context.Context, dataSourceName string) (*sql.DB, error) { + db, err := sql.Open("mysql", dataSourceName) + if err != nil { + // Don't log data source name as it could contain credentials + klog.Warningf("Could not open MySQL database, check config: %s", err) + return nil, err + } + + if _, err := db.ExecContext(ctx, "SET sql_mode = 'STRICT_ALL_TABLES'"); err != nil { + klog.Warningf("Failed to set strict mode on mysql db: %s", err) + return nil, err + } + + return db, nil +} diff --git a/trillian/ctfe/storage/mysql/mysql_test.go b/trillian/ctfe/storage/mysql/mysql_test.go new file mode 100644 index 0000000000..ec4b0a2ff8 --- /dev/null +++ b/trillian/ctfe/storage/mysql/mysql_test.go @@ -0,0 +1,108 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package mysql + +import ( + "bytes" + "context" + "crypto/sha256" + "database/sql" + "os" + "testing" + + "github.com/DATA-DOG/go-sqlmock" +) + +func TestIssuanceChainFindByKeySuccess(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("an error '%s' was not expected when opening a stub database connection", err) + } + defer db.Close() + + testVal := readTestData(t, "leaf00.chain") + testKey := sha256.Sum256(testVal) + + issuanceChainMockRows := sqlmock.NewRows([]string{"ChainValue"}).AddRow(testVal) + mock.ExpectQuery(selectIssuanceChainByKeySQL).WillReturnRows(issuanceChainMockRows) + + storage := mockIssuanceChainStorage(db) + got, err := storage.FindByKey(context.Background(), testKey[:]) + if err != nil { + t.Errorf("issuanceChainStorage.FindByKey: %v", err) + } + if !bytes.Equal(got, testVal) { + t.Errorf("got: %v, want: %v", got, testVal) + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("there were unfulfilled expectations: %s", err) + } +} + +func TestIssuanceChainAddSuccess(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("an error '%s' was not expected when opening a stub database connection", err) + } + defer db.Close() + + tests := setupTestData(t, + "leaf00.chain", + "leaf01.chain", + "leaf02.chain", + ) + + storage := mockIssuanceChainStorage(db) + for k, v := range tests { + mock.ExpectExec("INSERT INTO IssuanceChain").WithArgs([]byte(k), v).WillReturnResult(sqlmock.NewResult(1, 1)) + storage.Add(context.Background(), []byte(k), v) + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("there were unfulfilled expectations: %s", err) + } +} + +func readTestData(t *testing.T, filename string) []byte { + t.Helper() + + data, err := os.ReadFile("../../../testdata/" + filename) + if err != nil { + t.Fatal(err) + } + + return data +} + +func setupTestData(t *testing.T, filenames ...string) map[string][]byte { + t.Helper() + + data := make(map[string][]byte, len(filenames)) + + for _, filename := range filenames { + val := readTestData(t, filename) + key := sha256.Sum256(val) + data[string(key[:])] = val + } + + return data +} + +func mockIssuanceChainStorage(db *sql.DB) *IssuanceChainStorage { + return &IssuanceChainStorage{ + db: db, + } +} diff --git a/trillian/ctfe/storage/mysql/schema.sql b/trillian/ctfe/storage/mysql/schema.sql new file mode 100644 index 0000000000..ecac03d347 --- /dev/null +++ b/trillian/ctfe/storage/mysql/schema.sql @@ -0,0 +1,24 @@ +-- Copyright 2024 Google LLC +-- +-- Licensed under the Apache License, Version 2.0 (the "License"); +-- you may not use this file except in compliance with the License. +-- You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. + +-- MySQL / MariaDB version of the CTFE database schema + +-- "IssuanceChain" table contains the hash and value pairs of the issuance chain. +CREATE TABLE IF NOT EXISTS `IssuanceChain` ( + -- Hash of the chain of intermediate certificates and root certificates. + `IdentityHash` VARBINARY(255) NOT NULL, + -- Chain data of intermediate certificates and root certificates. + `ChainValue` LONGBLOB NOT NULL, + PRIMARY KEY (`IdentityHash`) +); From 170cafd1885b0f272c4e9410dbaeefd5d58ad4f0 Mon Sep 17 00:00:00 2001 From: Roger Ng Date: Tue, 7 May 2024 11:27:08 +0000 Subject: [PATCH 2/6] Add err check for issuanceChainStorage.Add in test --- trillian/ctfe/storage/mysql/mysql_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/trillian/ctfe/storage/mysql/mysql_test.go b/trillian/ctfe/storage/mysql/mysql_test.go index ec4b0a2ff8..72b781915b 100644 --- a/trillian/ctfe/storage/mysql/mysql_test.go +++ b/trillian/ctfe/storage/mysql/mysql_test.go @@ -68,7 +68,9 @@ func TestIssuanceChainAddSuccess(t *testing.T) { storage := mockIssuanceChainStorage(db) for k, v := range tests { mock.ExpectExec("INSERT INTO IssuanceChain").WithArgs([]byte(k), v).WillReturnResult(sqlmock.NewResult(1, 1)) - storage.Add(context.Background(), []byte(k), v) + if err := storage.Add(context.Background(), []byte(k), v); err != nil { + t.Errorf("issuanceChainStorage.Add: %v", err) + } } if err := mock.ExpectationsWereMet(); err != nil { From 6c58d3b1f06650f1361f7cff3917470a2d285400 Mon Sep 17 00:00:00 2001 From: Roger Ng Date: Tue, 7 May 2024 12:57:32 +0000 Subject: [PATCH 3/6] Add strict SQL mode comment --- trillian/ctfe/storage/mysql/mysql.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/trillian/ctfe/storage/mysql/mysql.go b/trillian/ctfe/storage/mysql/mysql.go index 072fbf6ec7..0a64765775 100644 --- a/trillian/ctfe/storage/mysql/mysql.go +++ b/trillian/ctfe/storage/mysql/mysql.go @@ -84,11 +84,12 @@ func (s *IssuanceChainStorage) Add(ctx context.Context, key []byte, chain []byte func open(ctx context.Context, dataSourceName string) (*sql.DB, error) { db, err := sql.Open("mysql", dataSourceName) if err != nil { - // Don't log data source name as it could contain credentials + // Don't log data source name as it could contain credentials. klog.Warningf("Could not open MySQL database, check config: %s", err) return nil, err } + // Enable strict SQL mode to ensure consistent behaviour among different storage engines when handling invalid or missing values in data-change statements. if _, err := db.ExecContext(ctx, "SET sql_mode = 'STRICT_ALL_TABLES'"); err != nil { klog.Warningf("Failed to set strict mode on mysql db: %s", err) return nil, err From b5e998437fb2b55965235278e4bc92ca42971f33 Mon Sep 17 00:00:00 2001 From: Roger Ng Date: Tue, 7 May 2024 15:26:52 +0000 Subject: [PATCH 4/6] Panic when the MySQL database cannot be opened --- trillian/ctfe/storage/mysql/mysql.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/trillian/ctfe/storage/mysql/mysql.go b/trillian/ctfe/storage/mysql/mysql.go index 0a64765775..4ad184b696 100644 --- a/trillian/ctfe/storage/mysql/mysql.go +++ b/trillian/ctfe/storage/mysql/mysql.go @@ -19,6 +19,7 @@ import ( "context" "database/sql" "errors" + "fmt" "strings" "k8s.io/klog/v2" @@ -37,12 +38,9 @@ type IssuanceChainStorage struct { // NewIssuanceChainStorage takes the database connection string as the input and return the IssuanceChainStorage. func NewIssuanceChainStorage(ctx context.Context, dbConn string) *IssuanceChainStorage { - conn := strings.Split(dbConn, "://") - - db, err := open(ctx, conn[1]) + db, err := open(ctx, dbConn) if err != nil { - klog.Errorf("failed to open database: %v", err) - return nil + panic(fmt.Sprintf("failed to open database: %v", err)) } return &IssuanceChainStorage{ @@ -82,16 +80,22 @@ func (s *IssuanceChainStorage) Add(ctx context.Context, key []byte, chain []byte // open takes the data source name and returns the sql.DB object. func open(ctx context.Context, dataSourceName string) (*sql.DB, error) { - db, err := sql.Open("mysql", dataSourceName) + // Verify data source name format. + conn := strings.Split(dataSourceName, "://") + if len(conn) != 2 || conn[0] != "mysql" { + return nil, errors.New("could not parse MySQL data source name") + } + + db, err := sql.Open("mysql", conn[1]) if err != nil { // Don't log data source name as it could contain credentials. - klog.Warningf("Could not open MySQL database, check config: %s", err) + klog.Fatalf("could not open MySQL database, check config: %s", err) return nil, err } // Enable strict SQL mode to ensure consistent behaviour among different storage engines when handling invalid or missing values in data-change statements. if _, err := db.ExecContext(ctx, "SET sql_mode = 'STRICT_ALL_TABLES'"); err != nil { - klog.Warningf("Failed to set strict mode on mysql db: %s", err) + klog.Warningf("failed to set strict mode on mysql db: %s", err) return nil, err } From debf087951c54c0172eacf73255a7d46ad7e0109 Mon Sep 17 00:00:00 2001 From: Roger Ng Date: Tue, 7 May 2024 15:39:54 +0000 Subject: [PATCH 5/6] Add a separate error message for mysql data source name prefix check --- trillian/ctfe/storage/mysql/mysql.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/trillian/ctfe/storage/mysql/mysql.go b/trillian/ctfe/storage/mysql/mysql.go index 4ad184b696..bd2cc66ad5 100644 --- a/trillian/ctfe/storage/mysql/mysql.go +++ b/trillian/ctfe/storage/mysql/mysql.go @@ -82,9 +82,12 @@ func (s *IssuanceChainStorage) Add(ctx context.Context, key []byte, chain []byte func open(ctx context.Context, dataSourceName string) (*sql.DB, error) { // Verify data source name format. conn := strings.Split(dataSourceName, "://") - if len(conn) != 2 || conn[0] != "mysql" { + if len(conn) != 2 { return nil, errors.New("could not parse MySQL data source name") } + if conn[0] != "mysql" { + return nil, errors.New("expect data source name to start with mysql") + } db, err := sql.Open("mysql", conn[1]) if err != nil { From 305e921ad6c5951f9fbcf9f3cd1e0f9b7ff268ab Mon Sep 17 00:00:00 2001 From: Roger Ng Date: Tue, 7 May 2024 15:58:36 +0000 Subject: [PATCH 6/6] Replace `panic` with `klog.Exitf` --- trillian/ctfe/storage/mysql/mysql.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/trillian/ctfe/storage/mysql/mysql.go b/trillian/ctfe/storage/mysql/mysql.go index bd2cc66ad5..7e252e6c3b 100644 --- a/trillian/ctfe/storage/mysql/mysql.go +++ b/trillian/ctfe/storage/mysql/mysql.go @@ -40,7 +40,7 @@ type IssuanceChainStorage struct { func NewIssuanceChainStorage(ctx context.Context, dbConn string) *IssuanceChainStorage { db, err := open(ctx, dbConn) if err != nil { - panic(fmt.Sprintf("failed to open database: %v", err)) + klog.Exitf(fmt.Sprintf("failed to open database: %v", err)) } return &IssuanceChainStorage{ @@ -92,7 +92,7 @@ func open(ctx context.Context, dataSourceName string) (*sql.DB, error) { db, err := sql.Open("mysql", conn[1]) if err != nil { // Don't log data source name as it could contain credentials. - klog.Fatalf("could not open MySQL database, check config: %s", err) + klog.Errorf("could not open MySQL database, check config: %s", err) return nil, err }