-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 signing and encryption #1025
Conversation
Holds ISO country code.
Seemingly always displayed prefixed with the main test name. Redundancy is unnecessary. Follows examples in the testing package docs.
Thanks for looking it over. That really is a lot of code for DSA. I can remove it if it's no good. Somewhere I read DSA is no longer secure, so I wonder if it's actually a security benefit to leave it out. |
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.
This mostly LGTM, though as I mentioned in #900 (comment), we'll hold off on merging it for now. You can ignore the inline code comments, they are mostly for when we decide to resume work on the issue.
I know that this PR will probably become too big, but can you also push the rest of the encryption-related changes that you've done in bookmoons/encrypt
here and edit the PR to be "Add signing and encryption"? Even if they aren't fully done, that way we'd have a single starting point for when we decide to take up the crypto functionality again, after we've done our evaluations and benchmarks and we have proper handling of binary data...
js/modules/k6/crypto/code.go
Outdated
err := errors.New("not a byte array") | ||
return 0, err | ||
} | ||
decoded := byte(encoded) |
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.
This is a potential error - if the encoded
was more than 255, golang would silently truncate it: https://play.golang.org/p/GDkYkXO5vAk
assert.EqualError(t, err, "unrecognized binary encoding") | ||
}) | ||
|
||
t.Run("ByteArray", func(t *testing.T) { |
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.
Please add another test that verifies that if you pass a greater-than-255 value in the array, it will be an error
Made that push. I appreciate the attention to this. Including here notes about the encryption piece.
Encryption usage is: import x509 from "k6/crypto/x509";
import { encrypt, encryptString } from "k6/crypto";
const pem = getPublicKey();
const pub = x509.parsePublicKey(pem);
const message = "They know, get out now!";
const binary = [ 0x01, 0x02, 0x03 ];
encrypt(pub, binary, "hex"); // ciphertext as hex string
encryptString(pub, message, "base64"); // ciphertext as base64 string Decryption usage is: import x509 from "k6/crypto/x509";
const pem = getPrivateKey();
const priv = x509.parsePrivateKey(pem, "super-secret-password");
const ciphertext = [ 0x01, 0x02, 0x03 ];
decrypt(priv, ciphertext, "hex"); // plaintext as hex string
decryptString(priv, ciphertext); // plaintext as string, interpreted as UTF-8 |
I'll close this, since we likely won't merge it anytime soon. Someone from the k6 community, @szkiba, recently created an xk6 extension with a subset of the features here: https://github.com/szkiba/xk6-crypto If that is insufficient, people can make PRs to that extension or make a new one: https://k6.io/blog/extending-k6-with-xk6 |
Adds cryptographic signing and signature verification. Adds encryption and decryption.
Suggesting here to include convenience string functions. They interpret strings as UTF-8.
Signing usage is:
Verification usage is:
Toward #900.