From 82ffd9ef3112ed081336c3148dad675c6ed92b06 Mon Sep 17 00:00:00 2001 From: Tobie Morgan Hitchcock Date: Wed, 4 Apr 2018 19:11:59 +0100 Subject: [PATCH] Fix bug in putC/delC for mysql storage backend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When using encryption, the expected value (once encrypted) would never be the same as the encrypted value in the storage layer. As a result, it would never be possible to putC or delC a KV item. Now the mysql package first gets the key from the storage layer, and then checks it’s decrypted value, before attempting to overwirte the value in the storage layer. --- kvs/mysql/err.go | 22 ++++++++++++++ kvs/mysql/sql.go | 15 --------- kvs/mysql/tx.go | 77 ++++++++++++++++++++++++++++++++++------------- kvs/mysql/util.go | 23 ++++++++++++++ 4 files changed, 101 insertions(+), 36 deletions(-) create mode 100644 kvs/mysql/err.go diff --git a/kvs/mysql/err.go b/kvs/mysql/err.go new file mode 100644 index 00000000..733bc7c9 --- /dev/null +++ b/kvs/mysql/err.go @@ -0,0 +1,22 @@ +// Copyright © 2016 Abcum Ltd +// +// 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 ( + "errors" +) + +// ErrKvNotExpectedValue occurs when conditionally putting or deleting a key-value item. +var ErrTxNotExpectedValue = errors.New("KV val is not expected value") diff --git a/kvs/mysql/sql.go b/kvs/mysql/sql.go index 36a04e65..b4c91101 100644 --- a/kvs/mysql/sql.go +++ b/kvs/mysql/sql.go @@ -73,13 +73,6 @@ const sqlDel = ` LIMIT 1 ` -const sqlDelC = ` - DELETE FROM kv - WHERE t<=? AND k=? AND v=? - ORDER BY t DESC - LIMIT 1 -` - const sqlDelP = ` DELETE q1 FROM kv JOIN ( @@ -120,11 +113,3 @@ const sqlPutN = ` VALUES (?, ?, ?) ` - -const sqlPutC = ` - UPDATE kv - SET v=? - WHERE t<=? AND k=? AND v=? - ORDER BY t DESC - LIMIT 1 -` diff --git a/kvs/mysql/tx.go b/kvs/mysql/tx.go index 0d257b66..0741619c 100644 --- a/kvs/mysql/tx.go +++ b/kvs/mysql/tx.go @@ -36,12 +36,10 @@ type TX struct { getP *sql.Stmt getR *sql.Stmt del *sql.Stmt - delC *sql.Stmt delP *sql.Stmt delR *sql.Stmt put *sql.Stmt putN *sql.Stmt - putC *sql.Stmt } } @@ -286,21 +284,41 @@ func (tx *TX) Del(ver int64, key []byte) (kvs.KV, error) { func (tx *TX) DelC(ver int64, key []byte, exp []byte) (kvs.KV, error) { var err error + var now kvs.KV var res *sql.Rows - exp, err = enc(exp) - if err != nil { - return nil, err - } - tx.lock.Lock() defer tx.lock.Unlock() - if tx.stmt.delC == nil { - tx.stmt.delC, _ = tx.pntr.Prepare(sqlDelC) + // Get the item at the key + + if tx.stmt.get == nil { + tx.stmt.get, _ = tx.pntr.Prepare(sqlGet) } - res, err = tx.stmt.delC.Query(ver, key, exp) + res, err = tx.stmt.get.Query(ver, key) + if err != nil { + return nil, err + } + + now, err = one(res, err) + if err != nil { + return nil, err + } + + // Check if the values match + + if !alter(now.Val(), exp) { + return nil, ErrTxNotExpectedValue + } + + // If they match then delete + + if tx.stmt.del == nil { + tx.stmt.del, _ = tx.pntr.Prepare(sqlDel) + } + + res, err = tx.stmt.del.Query(ver, key) return one(res, err) @@ -376,6 +394,7 @@ func (tx *TX) Put(ver int64, key []byte, val []byte) (kvs.KV, error) { func (tx *TX) PutC(ver int64, key []byte, val []byte, exp []byte) (kvs.KV, error) { var err error + var now kvs.KV var res *sql.Rows val, err = enc(val) @@ -383,14 +402,6 @@ func (tx *TX) PutC(ver int64, key []byte, val []byte, exp []byte) (kvs.KV, error return nil, err } - exp, err = enc(exp) - if err != nil { - return nil, err - } - - tx.lock.Lock() - defer tx.lock.Unlock() - switch exp { case nil: @@ -405,11 +416,35 @@ func (tx *TX) PutC(ver int64, key []byte, val []byte, exp []byte) (kvs.KV, error default: - if tx.stmt.putC == nil { - tx.stmt.putC, _ = tx.pntr.Prepare(sqlPutC) + // Get the item at the key + + if tx.stmt.get == nil { + tx.stmt.get, _ = tx.pntr.Prepare(sqlGet) } - res, err = tx.stmt.putC.Query(val, ver, key, exp) + res, err = tx.stmt.get.Query(ver, key) + if err != nil { + return nil, err + } + + now, err = one(res, err) + if err != nil { + return nil, err + } + + // Check if the values match + + if !check(now.Val(), exp) { + return nil, ErrTxNotExpectedValue + } + + // If they match then delete + + if tx.stmt.del == nil { + tx.stmt.put, _ = tx.pntr.Prepare(sqlPut) + } + + res, err = tx.stmt.put.Query(ver, key, val, val) return one(res, err) diff --git a/kvs/mysql/util.go b/kvs/mysql/util.go index 8b94a1f5..87c96ba0 100644 --- a/kvs/mysql/util.go +++ b/kvs/mysql/util.go @@ -15,6 +15,7 @@ package mysql import ( + "bytes" "crypto/aes" "crypto/cipher" "crypto/rand" @@ -23,6 +24,28 @@ import ( var chars = []byte("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789") +// Used to see if we can conditionally put a value. We can only put +// a value if the value is the same, or if both items are nil. +func check(a, b []byte) bool { + if a != nil && b != nil { + return bytes.Equal(a, b) + } else if a == nil && b == nil { + return true + } + return false +} + +// Used to see if we can conditionally del a value. We can only del +// a value if the value is the same, and neither item is nil. +func alter(a, b []byte) bool { + if a != nil && b != nil { + return bytes.Equal(a, b) + } else if a == nil && b == nil { + return false + } + return false +} + func encrypt(key []byte, src []byte) (dst []byte, err error) { if key == nil || len(key) == 0 || len(src) == 0 {