-
Notifications
You must be signed in to change notification settings - Fork 6.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
KMS changes #1723
KMS changes #1723
Changes from 8 commits
ecc131c
ca42575
c4efd87
91a4324
fc79ceb
6e0d35e
09927c4
e85865b
927e3cc
293e605
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,7 @@ | |
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
|
||
import base64 | ||
from os import environ | ||
from time import sleep | ||
|
||
|
@@ -89,6 +89,7 @@ class TestKMSSamples: | |
.format(parent, keyring, ecSignId) | ||
|
||
message = 'test message 123' | ||
message_bytes = message.encode('utf-8') | ||
|
||
client = discovery.build('cloudkms', 'v1') | ||
|
||
|
@@ -99,44 +100,54 @@ def test_get_public_key(self): | |
assert isinstance(ec_key, _EllipticCurvePublicKey), 'expected EC key' | ||
|
||
def test_rsa_encrypt_decrypt(self): | ||
ciphertext = sample.encryptRSA(self.message, | ||
self.client, | ||
self.rsaDecrypt) | ||
ciphertext_bytes = sample.encryptRSA(self.message_bytes, | ||
self.client, | ||
self.rsaDecrypt) | ||
ciphertext = base64.b64encode(ciphertext_bytes).decode() | ||
# ciphertext should be 344 characters with base64 and RSA 2048 | ||
assert len(ciphertext) == 344, \ | ||
'ciphertext should be 344 chars; got {}'.format(len(ciphertext)) | ||
assert ciphertext[-2:] == '==', 'cipher text should end with ==' | ||
plaintext = sample.decryptRSA(ciphertext, self.client, self.rsaDecrypt) | ||
plaintext_bytes = sample.decryptRSA(ciphertext_bytes, | ||
self.client, | ||
self.rsaDecrypt) | ||
assert plaintext_bytes == self.message_bytes | ||
plaintext = plaintext_bytes.decode('utf-8') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there much value in checking that plaintext == self.message if you've already checked that plaintext_bytes == self.message_bytes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably not, but I'd rather err on the side of too many tests, especially when dealing with all these encoding changes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most of the time you specify utf-8 for string.encode() and bytes.decode(), except on line 51 where you just assume the default (which is utf-8). Both ways work fine, but this is a minor inconsistency. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good point, we should probably keep that consistent. Fixed |
||
assert plaintext == self.message | ||
|
||
def test_rsa_sign_verify(self): | ||
sig = sample.signAsymmetric(self.message, self.client, self.rsaSign) | ||
sig = sample.signAsymmetric(self.message_bytes, | ||
self.client, | ||
self.rsaSign) | ||
# ciphertext should be 344 characters with base64 and RSA 2048 | ||
assert len(sig) == 344, \ | ||
'sig should be 344 chars; got {}'.format(len(sig)) | ||
assert sig[-2:] == '==', 'sig should end with ==' | ||
success = sample.verifySignatureRSA(sig, | ||
self.message, | ||
self.message_bytes, | ||
self.client, | ||
self.rsaSign) | ||
assert success is True, 'RSA verification failed' | ||
changed_bytes = self.message_bytes + b'.' | ||
success = sample.verifySignatureRSA(sig, | ||
self.message+'.', | ||
changed_bytes, | ||
self.client, | ||
self.rsaSign) | ||
assert success is False, 'verify should fail with modified message' | ||
|
||
def test_ec_sign_verify(self): | ||
sig = sample.signAsymmetric(self.message, self.client, self.ecSign) | ||
sig = sample.signAsymmetric(self.message_bytes, | ||
self.client, | ||
self.ecSign) | ||
assert len(sig) > 50 and len(sig) < 300, \ | ||
'sig outside expected length range' | ||
success = sample.verifySignatureEC(sig, | ||
self.message, | ||
self.message_bytes, | ||
self.client, | ||
self.ecSign) | ||
assert success is True, 'EC verification failed' | ||
changed_bytes = self.message_bytes + b'.' | ||
success = sample.verifySignatureEC(sig, | ||
self.message+'.', | ||
changed_bytes, | ||
self.client, | ||
self.ecSign) | ||
assert success is False, 'verify should fail with modified message' |
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.
The signature passed to verifySignature{RSA,EC} should be raw binary. signAsymmetric should b64decode the signature received in the response.
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.
fixed