From 77a3ba9197be226bf6018373e9faee3c029e3f55 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Mon, 11 Apr 2022 14:23:10 +0300 Subject: [PATCH] [#393] util/signature: Remove bytes pool and provide buffer explicitly Chained verification is done in a single thread there is no need to use pool to do this. Also because newly allocated items are 5 MiB in size we can run out of memory given that typical header it much less in size. Signed-off-by: Evgenii Stratonikov --- signature/sign.go | 25 ++++++++++++++++++------- util/signature/data.go | 32 ++++++++++++++++++++------------ util/signature/options.go | 8 ++++++++ util/signature/util.go | 30 ------------------------------ 4 files changed, 46 insertions(+), 49 deletions(-) delete mode 100644 util/signature/util.go diff --git a/signature/sign.go b/signature/sign.go index e6bb1941..e11642e1 100644 --- a/signature/sign.go +++ b/signature/sign.go @@ -242,22 +242,32 @@ func VerifyServiceMessage(msg interface{}) error { panic(fmt.Sprintf("unsupported session message %T", v)) } - return verifyMatryoshkaLevel(serviceMessageBody(msg), meta, verify) + body := serviceMessageBody(msg) + size := body.StableSize() + if sz := meta.StableSize(); sz > size { + size = sz + } + if sz := verify.StableSize(); sz > size { + size = sz + } + + buf := make([]byte, 0, size) + return verifyMatryoshkaLevel(body, meta, verify, buf) } -func verifyMatryoshkaLevel(body stableMarshaler, meta metaHeader, verify verificationHeader) error { - if err := verifyServiceMessagePart(meta, verify.GetMetaSignature); err != nil { +func verifyMatryoshkaLevel(body stableMarshaler, meta metaHeader, verify verificationHeader, buf []byte) error { + if err := verifyServiceMessagePart(meta, verify.GetMetaSignature, buf); err != nil { return fmt.Errorf("could not verify meta header: %w", err) } origin := verify.getOrigin() - if err := verifyServiceMessagePart(origin, verify.GetOriginSignature); err != nil { + if err := verifyServiceMessagePart(origin, verify.GetOriginSignature, buf); err != nil { return fmt.Errorf("could not verify origin of verification header: %w", err) } if origin == nil { - if err := verifyServiceMessagePart(body, verify.GetBodySignature); err != nil { + if err := verifyServiceMessagePart(body, verify.GetBodySignature, buf); err != nil { return fmt.Errorf("could not verify body: %w", err) } @@ -268,13 +278,14 @@ func verifyMatryoshkaLevel(body stableMarshaler, meta metaHeader, verify verific return errors.New("body signature at the matryoshka upper level") } - return verifyMatryoshkaLevel(body, meta.getOrigin(), origin) + return verifyMatryoshkaLevel(body, meta.getOrigin(), origin, buf) } -func verifyServiceMessagePart(part stableMarshaler, sigRdr func() *refs.Signature) error { +func verifyServiceMessagePart(part stableMarshaler, sigRdr func() *refs.Signature, buf []byte) error { return signature.VerifyDataWithSource( &StableMarshalerWrapper{part}, sigRdr, + signature.WithBuffer(buf), ) } diff --git a/util/signature/data.go b/util/signature/data.go index cd02d884..387be935 100644 --- a/util/signature/data.go +++ b/util/signature/data.go @@ -29,18 +29,17 @@ func SignDataWithHandler(key *ecdsa.PrivateKey, src DataSource, handler KeySigna return crypto.ErrEmptyPrivateKey } - data, err := dataForSignature(src) - if err != nil { - return err - } - defer bytesPool.Put(data) - cfg := defaultCfg() for i := range opts { opts[i](cfg) } + data, err := readSignedData(cfg, src) + if err != nil { + return err + } + sigData, err := sign(cfg, key, data) if err != nil { return err @@ -56,18 +55,17 @@ func SignDataWithHandler(key *ecdsa.PrivateKey, src DataSource, handler KeySigna } func VerifyDataWithSource(dataSrc DataSource, sigSrc KeySignatureSource, opts ...SignOption) error { - data, err := dataForSignature(dataSrc) - if err != nil { - return err - } - defer bytesPool.Put(data) - cfg := defaultCfg() for i := range opts { opts[i](cfg) } + data, err := readSignedData(cfg, dataSrc) + if err != nil { + return err + } + return verify(cfg, data, sigSrc()) } @@ -78,3 +76,13 @@ func SignData(key *ecdsa.PrivateKey, v DataWithSignature, opts ...SignOption) er func VerifyData(src DataWithSignature, opts ...SignOption) error { return VerifyDataWithSource(src, src.GetSignature, opts...) } + +func readSignedData(cfg *cfg, src DataSource) ([]byte, error) { + size := src.SignedDataSize() + if cfg.buffer == nil || cap(cfg.buffer) < size { + cfg.buffer = make([]byte, size) + } else { + cfg.buffer = cfg.buffer[:size] + } + return src.ReadSignedData(cfg.buffer) +} diff --git a/util/signature/options.go b/util/signature/options.go index dd389866..903ff66e 100644 --- a/util/signature/options.go +++ b/util/signature/options.go @@ -11,6 +11,7 @@ import ( type cfg struct { schemeFixed bool scheme refs.SignatureScheme + buffer []byte } func defaultCfg() *cfg { @@ -51,3 +52,10 @@ func SignWithRFC6979() SignOption { c.scheme = refs.ECDSA_RFC6979_SHA256 } } + +// WithBuffer allows providing pre-allocated buffer for signature verification. +func WithBuffer(buf []byte) SignOption { + return func(c *cfg) { + c.buffer = buf + } +} diff --git a/util/signature/util.go b/util/signature/util.go deleted file mode 100644 index 8278f9aa..00000000 --- a/util/signature/util.go +++ /dev/null @@ -1,30 +0,0 @@ -package signature - -import ( - "errors" - "sync" -) - -var bytesPool = sync.Pool{ - New: func() interface{} { - return make([]byte, 5<<20) - }, -} - -func dataForSignature(src DataSource) ([]byte, error) { - if src == nil { - return nil, errors.New("nil source") - } - - buf := bytesPool.Get().([]byte) - - if size := src.SignedDataSize(); size < 0 { - return nil, errors.New("negative length") - } else if size <= cap(buf) { - buf = buf[:size] - } else { - buf = make([]byte, size) - } - - return src.ReadSignedData(buf) -}