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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
security(logging): fix aes implementation in audit logging #8323
security(logging): fix aes implementation in audit logging #8323
Changes from 49 commits
3f5fb30
d2d06ca
cc0f613
14308ee
77a3ca3
b80723f
f2721b0
709ed5b
3fdd06b
f075085
18ec9c3
d5a8081
d05989b
d6d81e1
9c25f5c
9e596ae
97cc6d6
c9c53c1
354db49
ea7d8d7
2863841
349e7dc
f95d0e2
22c875c
4e5d02a
88e4ad3
e3abcec
59164b2
af907ec
3b1874e
ef0e75b
e7fd984
2089043
18c0933
4c24657
4b1e033
1c6bcb0
5efd7da
68d2875
32cf0a0
d303f1e
05a622f
d3dd3c7
abe1206
50a00c9
69b4b99
cf56c96
963ae87
1bd2bd0
b0bbaf1
eb275cd
d227b13
2f7cffa
a91c3f8
01cd716
91bafdc
75a1070
a74dd58
4a7e37f
b36a9b5
cbc434c
4b7c5ba
6264fe1
15e6cba
dc07b7c
4e5ba7f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we definer the VerificationText of type []byte and use bytes.Compare instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently we declare a string constant in
x/log_writer.go
. I think this is reasonable since this should be a constant, and we can't declare an array/slice to be a constant. We could cast the x.VerificationText as a byte slice and compare the bytes but I don't think this makes a difference at allThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't realize that byte array/slice can't be declared as a constant. This article has a few ideas https://blog.boot.dev/golang/golang-constant-maps-slices/. I don't mind if we declare a function that returns the constant value. I think it is useful to avoid a allocation that converts byte slice to string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can move this inside the if too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we now return the number of bytes written
n
and use this to increment the iteratorThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we now return the number of bytes written
n
and use this to increment the iteratorThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
more context in the error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again, what kind of context? this will be propagated up and return the invalid encryption key error to the user
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added error wrapper