Feat/Persistent session storage#1255
Merged
alexvanin merged 8 commits intonspcc-dev:masterfrom Mar 29, 2022
carpawell:feat/persistent-sessions
Merged
Feat/Persistent session storage#1255alexvanin merged 8 commits intonspcc-dev:masterfrom carpawell:feat/persistent-sessions
alexvanin merged 8 commits intonspcc-dev:masterfrom
carpawell:feat/persistent-sessions
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1255 +/- ##
==========================================
+ Coverage 35.87% 36.03% +0.15%
==========================================
Files 296 301 +5
Lines 18518 18778 +260
==========================================
+ Hits 6644 6766 +122
- Misses 11375 11484 +109
- Partials 499 528 +29
Continue to review full report at Codecov.
|
Move in-memory session storage to the separate directory of `storage`. It is done for future support of different kind of session storages. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
fyrchik
reviewed
Mar 22, 2022
Comment on lines
+48
to
+67
| // nil value is a hallmark | ||
| // of the nested buckets | ||
| if v == nil { |
Contributor
There was a problem hiding this comment.
Do we have anything other that sub-buckets in the root bucket?
Member
Author
There was a problem hiding this comment.
in fact, no, but the idea that a node could potentially use storage that has not been created by it haunts me
Comment on lines
+157
to
+159
| // This test was added to fix bolt's behaviour since the persistent | ||
| // storage uses cursor and there is an issue about `cursor.Delete` | ||
| // method: https://github.com/etcd-io/bbolt/issues/146. |
Contributor
There was a problem hiding this comment.
Wow, unexpected. As I understand, this is not fixed yet?
Member
Author
There was a problem hiding this comment.
this is an old issue (origins to the original archived repo) and I could reproduce key skipping when did such test in the one tx (updating and iterating in one Update call), so technically it is still an issue (at least there is no detailed behavior description about such use-cases)
however, code written in that PR works fine, but I've decided to add that test to know when bbolt's maintainers have done something breaking about that issue
Comment on lines
+70
to
+83
| rawKey, err := x509.MarshalECPrivateKey(cfg.privateKey) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("could not marshal provided private key: %w", err) | ||
| } | ||
|
|
||
| // tagOffset is a constant offset for | ||
| // tags when marshalling ECDSA key in | ||
| // ASN.1 DER form | ||
| const tagOffset = 7 | ||
|
|
||
| // using first 32 bytes from | ||
| // the marshalled private key | ||
| // as a secret | ||
| c, err := aes.NewCipher(rawKey[tagOffset : tagOffset+32]) |
Contributor
There was a problem hiding this comment.
https://github.com/golang/go/blob/master/src/crypto/x509/sec1.go#L58
rawKey := make([]byte, (key.Curve.Params().N.BitLen()+7)/8)
cfg.privateKey.D.FillBytes(rawKey)
Also works for every private key type, not just 32-byte one.
Member
Author
There was a problem hiding this comment.
looks better, does the same, thanks
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Add `WithEncryption` option that passes ECDSA key to the persistent session storage. It uses 32 bytes from marshalled ECDSA key in ASN.1 DER from in AES-256 algorithm encryption in Galois/Counter Mode. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
alexvanin
approved these changes
Mar 28, 2022
|
|
||
| data := []byte("nice encryption, awesome tests") | ||
|
|
||
| encryptedData, err := ts.encrypt(data) |
Contributor
There was a problem hiding this comment.
Compare that encrypted and decrypted data are not the same.
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Use persistent storage usage in the node if it was configured so. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
cthulhu-rider
pushed a commit
to cthulhu-rider/neofs-node
that referenced
this pull request
Sep 13, 2022
…hod's docs Method never returns `PersistentStatePathDefault` value, and this is correct. Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
cthulhu-rider
pushed a commit
to cthulhu-rider/neofs-node
that referenced
this pull request
Sep 13, 2022
…efault In current implementation storage node app uses in-memory session storage if `persistent_sessions.path` value is empty/missing in the config. Clarify default behavior in `config/example/node.yaml`. Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
aprasolova
pushed a commit
to aprasolova/neofs-node
that referenced
this pull request
Oct 19, 2022
Move in-memory session storage to the separate directory of `storage`. It is done for future support of different kind of session storages. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
aprasolova
pushed a commit
to aprasolova/neofs-node
that referenced
this pull request
Oct 19, 2022
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
aprasolova
pushed a commit
to aprasolova/neofs-node
that referenced
this pull request
Oct 19, 2022
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
aprasolova
pushed a commit
to aprasolova/neofs-node
that referenced
this pull request
Oct 19, 2022
Add `WithEncryption` option that passes ECDSA key to the persistent session storage. It uses 32 bytes from marshalled ECDSA key in ASN.1 DER from in AES-256 algorithm encryption in Galois/Counter Mode. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
aprasolova
pushed a commit
to aprasolova/neofs-node
that referenced
this pull request
Oct 19, 2022
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
aprasolova
pushed a commit
to aprasolova/neofs-node
that referenced
this pull request
Oct 19, 2022
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
aprasolova
pushed a commit
to aprasolova/neofs-node
that referenced
this pull request
Oct 19, 2022
Use persistent storage usage in the node if it was configured so. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
aprasolova
pushed a commit
to aprasolova/neofs-node
that referenced
this pull request
Oct 19, 2022
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
This was referenced Jun 25, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1189. Not sure if node admin should be able to configure encryption.
Please, double-check the encryption approach.