-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add support for ECDSA in dgraph cert #3269
Add support for ECDSA in dgraph cert #3269
Conversation
Add short flag -r for RSA bit size. Add flags --curve and -e for EDCSA curve type. When EDCSA value is set, RSA is ignored.
… types Change signature of readKey and makeKey to support any key type that implements crypto.PrivateKey. This change is made to add support for EDCSA keys, but in the future any other type should be simple to add. Note that crypto.PrivateKey is just a plain dynamic interface type, so it should already support crypto.Signer used in certificate functions.
dgraph/cmd/cert/info.go
Outdated
} | ||
switch k := key.(type) { | ||
case *ecdsa.PrivateKey: | ||
h := md5.Sum(elliptic.Marshal(k.PublicKey.Curve, k.PublicKey.X, k.PublicKey.Y)) |
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.
G401: Use of weak cryptographic primitive (from gosec
)
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.
Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @golangcibot and @srfrog)
dgraph/cmd/cert/info.go, line 88 at r1 (raw file):
Previously, golangcibot (Bot from GolangCI) wrote…
G401: Use of weak cryptographic primitive (from
gosec
)
There is another PR pending merge to replace MD5.
…ue-2642_support_ecdsa_in_dgraph_cert
Now that we support different key encryption types, we must display the name and parameters when displaying its info.
Add a new line to `dgraph cert ls` to display the key encryption type. Also, don't panic when command parameters fail, simply exit with error code.
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.
Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions (waiting on @codexnull and @golangcibot)
dgraph/cmd/cert/info.go, line 88 at r1 (raw file):
Previously, codexnull (Javier Alvarado) wrote…
There is another PR pending merge to replace MD5.
Done.
dgraph/cmd/cert/info.go, line 133 at r1 (raw file):
Previously, golangcibot (Bot from GolangCI) wrote…
G401: Use of weak cryptographic primitive (from
gosec
)
Done.
dgraph/cmd/cert/info.go, line 136 at r1 (raw file):
Previously, golangcibot (Bot from GolangCI) wrote…
G401: Use of weak cryptographic primitive (from
gosec
)
Done.
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.
Reviewed 3 of 4 files at r2.
Reviewable status: 3 of 4 files reviewed, 4 unresolved discussions (waiting on @codexnull, @golangcibot, and @srfrog)
dgraph/cmd/cert/run.go, line 142 at r2 (raw file):
} if f.encType != "" { fmt.Printf("%14s: %s\n", "Encryption", f.encType)
I would suggest changing "Encryption" to "Key Type" or "Key Algorithm" or just "Algorithm". openssl, for example, calls it "Public Key Algorithm."
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.
Reviewed 1 of 4 files at r2.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @golangcibot and @srfrog)
… 'Algorithm' in the listing
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.
Reviewable status: 2 of 4 files reviewed, 4 unresolved discussions (waiting on @codexnull and @golangcibot)
dgraph/cmd/cert/run.go, line 142 at r2 (raw file):
Previously, codexnull (Javier Alvarado) wrote…
I would suggest changing "Encryption" to "Key Type" or "Key Algorithm" or just "Algorithm". openssl, for example, calls it "Public Key Algorithm."
Done.
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.
Reviewed 2 of 2 files at r3.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @golangcibot)
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.
Didn't look too carefully, but the change looks pretty contained in the TLS package.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @golangcibot)
Add support for ECDSA keys in dgraph cert and update cert API for adding more encryption types.
This PR adds support for Weierstrass curves to create private and public keys in Dgraph TLS certificates,
dgraph cert
command.Summary of changes:
makeKey()
andreadKey()
to use crypto.PrivateKey and support any type that implements it.--elliptic-curve
and-e
.key
files to know which type it is (RSA or ECDSA).-r
dgraph cert ls
Reference #2642
This change is