Skip to content

feat(queue)!: add priv/pub key signing #843

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.

Sign up for GitHub

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

Merged
plyr4 merged 29 commits into from
Aug 25, 2023
Merged

feat(queue)!: add priv/pub key signing #843

Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
bd87a68
wip: queue item signing via asymm keys
plyr4 Oct 21, 2022
41a1bba
revert: docker-compose local changes
plyr4 Oct 21, 2022
70e9b23
wip: queue item signing via asymm keys
plyr4 Oct 21, 2022
2fcaf0c
wip: queue signing cleanup
plyr4 Oct 21, 2022
bae5946
fix: generate example keys
plyr4 May 11, 2023
203bdb7
tweak: env var naming
plyr4 May 11, 2023
43141ee
enhance: key validations
plyr4 May 11, 2023
ae70584
chore: comment wording
plyr4 May 11, 2023
ca19837
chore: merge with main
plyr4 May 11, 2023
e48fa44
chore: go mod tidy
plyr4 May 11, 2023
683e0a3
wip: nil return to allow read-only setup
plyr4 May 22, 2023
b1c4c48
chore: merge with main
plyr4 Jul 20, 2023
d9d7364
chore: add base64 encoded comment
plyr4 Jul 20, 2023
fc52931
chore: merge with main
plyr4 Aug 8, 2023
96f7bd5
fix: adding keys to redis test mocks
plyr4 Aug 9, 2023
67f4416
Merge branch 'main' into feat/queue-signing
plyr4 Aug 9, 2023
1324f55
Merge branch 'main' of github.com:go-vela/server into feat/queue-signing
plyr4 Aug 16, 2023
4786391
fix: do not allow empty signing keys
plyr4 Aug 17, 2023
448a95d
Merge branch 'main' into feat/queue-signing
plyr4 Aug 17, 2023
2c9d166
fix: lint
plyr4 Aug 17, 2023
346df6c
fix: lint
plyr4 Aug 17, 2023
00507fb
chore: add middleware tests
plyr4 Aug 17, 2023
0328180
Merge branch 'main' into feat/queue-signing
plyr4 Aug 17, 2023
7935fe5
Merge branch 'main' into feat/queue-signing
plyr4 Aug 23, 2023
c28b5d6
Merge branch 'main' into feat/queue-signing
ecrupper Aug 23, 2023
37926a5
tweak: queue/redis/pop.go suggestion
plyr4 Aug 23, 2023
5815fe9
tweak: variable naming for queue keys
plyr4 Aug 23, 2023
5bf066b
tweak: cli variable naming for queue keys
plyr4 Aug 23, 2023
f24894c
Merge branch 'main' into feat/queue-signing
plyr4 Aug 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix: do not allow empty signing keys
  • Loading branch information
@plyr4
plyr4 committed Aug 17, 2023
commit 4786391a3756166c3546546bd66d57414352fae7
6 changes: 2 additions & 4 deletions queue/redis/opts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ func WithPrivateKey(privateKeyEncoded string) ClientOpt {
c.Logger.Trace("configuring private key in redis queue client")

if len(privateKeyEncoded) == 0 {
// return errors.New("unable to base64 decode private key, provided key is empty")
return nil
return errors.New("unable to base64 decode private key, provided key is empty")
}

privateKeyDecoded, err := base64.StdEncoding.DecodeString(privateKeyEncoded)
Copy link
Contributor

@ecrupper ecrupper Aug 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't expect base64 encoding in any of our other private keys (token manager, DB encryption). Is there a reason why we're going with this pattern here? Should this be standardized across all private keys?

Copy link
Contributor Author

@plyr4 plyr4 Aug 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good question, i used base64 here because the private/public keys i generated arent in unicode.
(using this process to make it easier: https://go.dev/play/p/-go_7SnJbnP)
base64 encoding them made it easy to copy+paste, store elsewhere, put into transit, easier handling.

for the other values in the server? id argue we should base64 anything that isnt in a format that is easy to handle.
though i think the other keys are fine because we recommend generating them using -hex from a unicode-safe characterset.

openssl rand -hex 16

from docs
https://go-vela.github.io/docs/installation/server/docker/#step-4-create-the-private-key

so... honestly yeah we should base64 more, but for queue signing since hex isnt a valid way to generate the keypair it becomes more important to encode the pair.

Copy link
Contributor

@ecrupper ecrupper Aug 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks for the explanation

Expand Down Expand Up @@ -112,8 +111,7 @@ func WithPublicKey(publicKeyEncoded string) ClientOpt {
c.Logger.Tracef("configuring public key in redis queue client")

if len(publicKeyEncoded) == 0 {
// errors.New("unable to base64 decode public key, provided key is empty")
return nil
return errors.New("unable to base64 decode public key, provided key is empty")
}

publicKeyDecoded, err := base64.StdEncoding.DecodeString(publicKeyEncoded)
Expand Down