[77] Improve test suite for RSAEncrypter/Decrypter #81

mohemian-92817281 · 2018-05-07T09:07:10Z

Resolves part of issue #77 by extending the RSA encrypter/decrypter test suite.

The test keypair is extended into a set of two keypairs, one for Alice and one for Bob. Additional tests are added to test encryption and decryption of payloads with these keys. Some tests are added for specific error cases, i.e. trying to decrypt with the wrong key.

Looking forward to your comments! 💻 👀

* master: [79] Set deployment target to 10.0 and update project settings (#80)

…n tests

carol-mohemian · 2018-05-07T14:43:19Z

Tests/RSADecrypterTests.swift

-    let cipherTextBase64URL = "YkzUN55RBG1igsrL-7ofPPWruTjxNMS3Bl1_a3i-dVKBjiN6kH88j8G3iB3eLWFxbiKZvx0LmkP7J65-frpOgF41SlltjJ62LEZOICm4q21Q4MNqL_Vf7kIjh8DziEJkElEz5W4flhI6YQ-9wW_PQ-coBIPIZiFlw6peKAolz8xcevbUmnqIH6A3hOFLK23J2cWDSWgxHEBIYtZ6whQCJYL4vq5lAFNaEoDaE_cgL6LItY4t-vR1exTJSOlCGAv4uM1Kelk6uitaFk2c0h79u3UpFN_wa02m_PPdgTguRdxwRsCpsQhOKmEakl8LR6NTbIrdB13UoL2tdybltVeUCw"
+    let cipherTextWithAliceKeyBase64URL = "RHLdaxC52ve0icBLGh9Xn_jNoV3uJlEuLRer8_7OOqY2QCGdWPb8OiuxxjEep8okfvlrw_eheEll6v8xdjpzP2zopVEBoNZ9J8BDKL1Y-tTi176jlGXBXKJdRitsp14sVHx7x-dW9FjiOHEn_CSm3bM2THAJOEtjjeF6vRvxpGxm1KruTbtZ37VTl-vM-e4STda5dCanRPKFemDPYQvjUAF_wuE-P-UeE0fpxDceuCRN3C-H8LL9TIrSETRV_CzPVf6Ki9zJpaZQAnqZh5ix0KhaHFMawV7TJetcKPbCzEHbxAF5ib16mxKFc5Th3QFS3eRKYxjaEdciXWVMCrGfXQ"
+    let cipherTextWithBobKeyBase64URL = "TwSS87gjm4cbFbda5klqW_Io2oCoo2zAxWJaU6L1jAeGEoL6u1Gw7vPdEzGGNvUliqsdBBCe8yQhnft9nqIZ9CD7enfVz6tFWvEJuVZbhy0eZad_LLFBFm8eDEgmlw7n7nAZxeww0WloffMmK6GPUE5SjQSWjCRAYA9xwacbxM8shH6hIwpelwV3f-QJr6M_hoAVkiqZJ2tFfiEciPhVVuqNZxxudMxaNj-bWfSipwh2T11kxYQD3EdVR4MDkyxz2JTUQc20OIf_F4WwQXJlbVNLTs4miDKHmw7Ddd45D2DJs_nH-bMQK8b6iH3nke15arP23rozuagF4SHhenApPw"
+    let cipherTextWithAliceKeyEmptyStringBase64URL = "hyOA/tmOclFkj31UPrRb1EnaRMhR5VZg5TrUyfLMtCUlh3grAva0+sSjqt6zSlWK06A6zUieV69aLRbJ0ZactTTqX2CFrhiZ5nUXhzuUya83VKBI0xrGkpQ8u1y2Iqgb+gbWsFJdJ41cSpZXRpc16Hhd3klTp7YydYZQUG//PLM5bn359kqpT8meJdGqTceehVxmdqTpVwukh/uqLOE8CBCrT7D/2t18mzApGpm/Su4bVbZggJ5g9MRPSnwgq1GjKNcMa1PKd+/OWB/rIeDmorT8dLrusGeLbwFCj1HEz4z5izamiBiyPh96G0m4ZhPtVhR4Fo3ARj9C037GroDQ2w=="


Is this base64URL? 🤔 I guess the padding(==) and the / isn't 😁

Nope, you're right. It's not base64url. @gigi-mohemian you can quickly convert it online here or by hand replace all / with _, all + with - and remove the trailing ==.

Or you just remove the URL from the variable name. 😄

Are the cipher texts are generated by openssl?

I know the original string was a nastly long thing as well, but now that we started touching the tests again, what do you think of staying below 120 characters per line (if possible). With strings its kind of easy to use Swift's multiline string literals:

e.g.:

let cipherText = """ asdf\ jklö """

ghost

Nice! Awesome addition to the test suite, thx @gigi-mohemian! ✅ 🎉 📈

Can you have a quick look at the base64url thing, please? It's a minor issue and should be an easy fix. 🙂

ghost · 2018-05-09T07:29:20Z

.gitignore

@@ -101,3 +101,4 @@ fastlane/test_output

 .idea/
 sonar-reports/
+/JOSESwift.xcworkspace/xcshareddata


ghost · 2018-05-09T07:35:29Z

Tests/RSADecrypterTests.swift

-    let cipherTextBase64URL = "YkzUN55RBG1igsrL-7ofPPWruTjxNMS3Bl1_a3i-dVKBjiN6kH88j8G3iB3eLWFxbiKZvx0LmkP7J65-frpOgF41SlltjJ62LEZOICm4q21Q4MNqL_Vf7kIjh8DziEJkElEz5W4flhI6YQ-9wW_PQ-coBIPIZiFlw6peKAolz8xcevbUmnqIH6A3hOFLK23J2cWDSWgxHEBIYtZ6whQCJYL4vq5lAFNaEoDaE_cgL6LItY4t-vR1exTJSOlCGAv4uM1Kelk6uitaFk2c0h79u3UpFN_wa02m_PPdgTguRdxwRsCpsQhOKmEakl8LR6NTbIrdB13UoL2tdybltVeUCw"
+    let cipherTextWithAliceKeyBase64URL = "RHLdaxC52ve0icBLGh9Xn_jNoV3uJlEuLRer8_7OOqY2QCGdWPb8OiuxxjEep8okfvlrw_eheEll6v8xdjpzP2zopVEBoNZ9J8BDKL1Y-tTi176jlGXBXKJdRitsp14sVHx7x-dW9FjiOHEn_CSm3bM2THAJOEtjjeF6vRvxpGxm1KruTbtZ37VTl-vM-e4STda5dCanRPKFemDPYQvjUAF_wuE-P-UeE0fpxDceuCRN3C-H8LL9TIrSETRV_CzPVf6Ki9zJpaZQAnqZh5ix0KhaHFMawV7TJetcKPbCzEHbxAF5ib16mxKFc5Th3QFS3eRKYxjaEdciXWVMCrGfXQ"
+    let cipherTextWithBobKeyBase64URL = "TwSS87gjm4cbFbda5klqW_Io2oCoo2zAxWJaU6L1jAeGEoL6u1Gw7vPdEzGGNvUliqsdBBCe8yQhnft9nqIZ9CD7enfVz6tFWvEJuVZbhy0eZad_LLFBFm8eDEgmlw7n7nAZxeww0WloffMmK6GPUE5SjQSWjCRAYA9xwacbxM8shH6hIwpelwV3f-QJr6M_hoAVkiqZJ2tFfiEciPhVVuqNZxxudMxaNj-bWfSipwh2T11kxYQD3EdVR4MDkyxz2JTUQc20OIf_F4WwQXJlbVNLTs4miDKHmw7Ddd45D2DJs_nH-bMQK8b6iH3nke15arP23rozuagF4SHhenApPw"
+    let cipherTextWithAliceKeyEmptyStringBase64URL = "hyOA/tmOclFkj31UPrRb1EnaRMhR5VZg5TrUyfLMtCUlh3grAva0+sSjqt6zSlWK06A6zUieV69aLRbJ0ZactTTqX2CFrhiZ5nUXhzuUya83VKBI0xrGkpQ8u1y2Iqgb+gbWsFJdJ41cSpZXRpc16Hhd3klTp7YydYZQUG//PLM5bn359kqpT8meJdGqTceehVxmdqTpVwukh/uqLOE8CBCrT7D/2t18mzApGpm/Su4bVbZggJ5g9MRPSnwgq1GjKNcMa1PKd+/OWB/rIeDmorT8dLrusGeLbwFCj1HEz4z5izamiBiyPh96G0m4ZhPtVhR4Fo3ARj9C037GroDQ2w=="


Nope, you're right. It's not base64url. @gigi-mohemian you can quickly convert it online here or by hand replace all / with _, all + with - and remove the trailing ==.

Or you just remove the URL from the variable name. 😄

ghost · 2018-05-09T07:42:03Z

Tests/RSADecrypterTests.swift

-    let cipherTextBase64URL = "YkzUN55RBG1igsrL-7ofPPWruTjxNMS3Bl1_a3i-dVKBjiN6kH88j8G3iB3eLWFxbiKZvx0LmkP7J65-frpOgF41SlltjJ62LEZOICm4q21Q4MNqL_Vf7kIjh8DziEJkElEz5W4flhI6YQ-9wW_PQ-coBIPIZiFlw6peKAolz8xcevbUmnqIH6A3hOFLK23J2cWDSWgxHEBIYtZ6whQCJYL4vq5lAFNaEoDaE_cgL6LItY4t-vR1exTJSOlCGAv4uM1Kelk6uitaFk2c0h79u3UpFN_wa02m_PPdgTguRdxwRsCpsQhOKmEakl8LR6NTbIrdB13UoL2tdybltVeUCw"
+    let cipherTextWithAliceKeyBase64URL = "RHLdaxC52ve0icBLGh9Xn_jNoV3uJlEuLRer8_7OOqY2QCGdWPb8OiuxxjEep8okfvlrw_eheEll6v8xdjpzP2zopVEBoNZ9J8BDKL1Y-tTi176jlGXBXKJdRitsp14sVHx7x-dW9FjiOHEn_CSm3bM2THAJOEtjjeF6vRvxpGxm1KruTbtZ37VTl-vM-e4STda5dCanRPKFemDPYQvjUAF_wuE-P-UeE0fpxDceuCRN3C-H8LL9TIrSETRV_CzPVf6Ki9zJpaZQAnqZh5ix0KhaHFMawV7TJetcKPbCzEHbxAF5ib16mxKFc5Th3QFS3eRKYxjaEdciXWVMCrGfXQ"
+    let cipherTextWithBobKeyBase64URL = "TwSS87gjm4cbFbda5klqW_Io2oCoo2zAxWJaU6L1jAeGEoL6u1Gw7vPdEzGGNvUliqsdBBCe8yQhnft9nqIZ9CD7enfVz6tFWvEJuVZbhy0eZad_LLFBFm8eDEgmlw7n7nAZxeww0WloffMmK6GPUE5SjQSWjCRAYA9xwacbxM8shH6hIwpelwV3f-QJr6M_hoAVkiqZJ2tFfiEciPhVVuqNZxxudMxaNj-bWfSipwh2T11kxYQD3EdVR4MDkyxz2JTUQc20OIf_F4WwQXJlbVNLTs4miDKHmw7Ddd45D2DJs_nH-bMQK8b6iH3nke15arP23rozuagF4SHhenApPw"
+    let cipherTextWithAliceKeyEmptyStringBase64URL = "hyOA/tmOclFkj31UPrRb1EnaRMhR5VZg5TrUyfLMtCUlh3grAva0+sSjqt6zSlWK06A6zUieV69aLRbJ0ZactTTqX2CFrhiZ5nUXhzuUya83VKBI0xrGkpQ8u1y2Iqgb+gbWsFJdJ41cSpZXRpc16Hhd3klTp7YydYZQUG//PLM5bn359kqpT8meJdGqTceehVxmdqTpVwukh/uqLOE8CBCrT7D/2t18mzApGpm/Su4bVbZggJ5g9MRPSnwgq1GjKNcMa1PKd+/OWB/rIeDmorT8dLrusGeLbwFCj1HEz4z5izamiBiyPh96G0m4ZhPtVhR4Fo3ARj9C037GroDQ2w=="


Are the cipher texts are generated by openssl?

I know the original string was a nastly long thing as well, but now that we started touching the tests again, what do you think of staying below 120 characters per line (if possible). With strings its kind of easy to use Swift's multiline string literals:

e.g.:

let cipherText = """ asdf\ jklö """

ghost · 2018-05-09T07:43:50Z

Tests/RSADecrypterTests.swift


        XCTAssertEqual(plainText, message.data(using: .utf8))
    }

+    func testDecryptingAliceSecretWithBobKey() {


mohemian-92817281 · 2018-05-09T15:54:52Z

Thanks for the review guys! Will update the PR 🔜

mohemian-92817281 · 2018-05-14T08:57:47Z

@carol-mohemian @daniel-mohemian I addressed the issues mentioned. Please have a look again 👍

carol-mohemian · 2018-05-14T09:09:48Z

Tests/RSADecrypterTests.swift

+-H8LL9TIrSETRV_CzPVf6Ki9zJpaZQAnqZh5ix0KhaHFMawV7TJetcKPbCzEHbxAF5ib16mxKFc5Th3QFS3eRKYxjaEdciXWVMCrGfXQ
+"""
+
+    let cipherTextWithBobKeyBase64 = """


Hmm, now this is a little confusing, why is cipherTextWithBobKeyBase64 and cipherTextWithAliceKeyBase64base64URL andcipherTextWithAliceKeyEmptyStringBase64` "only" base64?

Ah damn, you're right. I'll clean this up asap.

It's base64 encoded now.

printf "The true sign of intelligence is not knowledge but imagination." | openssl rsautl -encrypt -pubin -inkey alice.pub.pem -out >(base64)

carol-mohemian

👆

ghost · 2018-05-24T15:28:23Z

@gigi-mohemian Any updates on this? 😉

* master: Fix plain text length check for RSA1_5 (#94) Set swift_version in podspec (#91) Prepare 1.1.0 (#84) Add support for RS256 (#83) # Conflicts: # Tests/CryptoTestCase.swift # Tests/JWSTests.swift # Tests/RSAVerifierTests.swift

mohemian-92817281 · 2018-07-26T12:18:48Z

Sorry this took so long @daniel-mohemian @carol-mohemian ⏳

The tests for RSA encryption/decryption should be fine now, I re-created the cipher texts using Alice's and Bob's keys from scratch using openssl, since it's been so long 😅I wanted to make sure that everything is correct.

I'd like to do additional tests for AES and for the signer/verifier, but I think it's best to do it in another PR.

Please have another look and let me know what you think 😘

carol-mohemian · 2018-07-27T08:25:29Z

Nice stuff @gigi-mohemian 😍 One small thing: We encountered that the plainText/cipherTextTooLong/short stuff is a little tricky. Could you maybe add another test which has not 0 and 300 but more like 255, 256, 257 length? Does this make sense?

ghost

Nice stiff @gigi-mohemian!

I agree with @carol-mohemian, some edge case tests for the RSA plaintext length and ciphertext length would be awesome.

For RSA1_5 , the plaintext length should satisfy mLen <= k - 11 where mLen is the plaintext length in bytes, and k is the modulus length in bytes. For 2048 bit keys, k is 256.

The ciphertext length should satisfy C == k.

Please double check those values in the linked RFC, though. 😄

… + 1`

* master: [87] Implement direct encryption and symmetric JWKs (#92) # Conflicts: # .gitignore # Tests/JWERSATests.swift

mohemian-92817281 · 2018-07-30T09:44:07Z

I agree as well! 😄

Double-checked the RFC and updated the tests. Added the following:

Encryption:

test if encryption of maximum length message mLen works
test if encryption fo mLen + 1 fails

Decryption:

test if decryption of len = k works (i.e., does not throw a "length" error)
test if decryption of len = k-1 fails
test if decryption of len = k+1 fails

ghost

Awesome! 👍 🎖

carol-mohemian · 2018-07-30T11:34:58Z

Tests/RSADecrypterTests.swift

+        XCTAssertThrowsError(try decrypter.decrypt(testMessage)) { (error: Error) in
+            // Should throw "decryption failed", but
+            // should _not_ throw cipherTextLenghtNotSatisfied
+            XCTAssertNotEqual(error as? RSAError, RSAError.cipherTextLenghtNotSatisfied)


This test will fail as soon as #96 is done 🤔

Good catch!

carol-mohemian · 2018-07-30T11:35:44Z

Tests/RSADecrypterTests.swift

+
+        let decrypter = RSADecrypter(algorithm: .RSA1_5, privateKey: privateKeyAlice2048!)
+        XCTAssertThrowsError(try decrypter.decrypt(testMessage)) { (error: Error) in
+            XCTAssertEqual(error as? RSAError, RSAError.cipherTextLenghtNotSatisfied)


Same #96 (just to give a heads up)

mohemian-92817281 added 18 commits April 11, 2018 15:45

[JOSE-80] Add shared user data introduced Xcode 9.3 to gitignore

bc7bbdf

Merge branch 'master' into feature/JOSE-80-improve-tests

9c0c504

* master: [79] Set deployment target to 10.0 and update project settings (#80)

[JOSE-80] Add failure test case for decrypting empty data

7d00453

[JOSE-80] Better assertions for failure case

9567ae8

[JOSE-80] privateKey2048 -> privateKeyAlice2048

2265f17

[JOSE-80] publicKey2048 -> publicKeyAlice2048

a87570c

[JOSE-80] Add Bob’s 2048 bit private key

bcec1c5

[JOSE-80] Add encryption test for Bob’s key

17ffa68

[JOSE-80] keyPair2048 -> keyPairAlice2048

f7350e5

[JOSE-80] Read Bob’s keypair from plist file

9650165

[JOSE-80] cipherTextBase64URL -> cipherTextWithAliceKeyBase64URL

79582f8

[JOSE-80] Load Bob’s key data; update cipher texts; add Bob decryptio…

c660a0c

…n tests

[JOSE-80] Add test for encrypting the same data twice

62f222d

[JOSE-80] Add test to check if Alice’s and Bob’s cipher texts differ

40ce19b

[JOSE-80] Add equality for decryption error

a7292e5

[JOSE-80] Add default decryption error

ee5323a

[JOSE-80] Decrypting with the wrong key should throw an error

2d4f09f

[JOSE-80] Update Bob’s cipher text

8c58365

mohemian-92817281 self-assigned this May 7, 2018

mohemian-92817281 requested review from carol-mohemian and a user May 7, 2018 09:07

carol-mohemian reviewed May 7, 2018

View reviewed changes

ghost suggested changes May 9, 2018

View reviewed changes

mohemian-92817281 added 2 commits May 14, 2018 10:49

[JOSE-80] Fix Base64 variable names

f36fdb2

[JOSE-80] Use multiline literals for cipher texts

eeecbde

carol-mohemian reviewed May 14, 2018

View reviewed changes

mohemian-92817281 added 7 commits July 18, 2018 14:42

[JOSE-80] Re-generate Alice’s cypher text with openssl; adjust test

03adc40

[JOSE-80] Re-generate Bob’s cypher text with openssl; adjust test

25fe769

[JOSE-80] Better comments

ba2dac1

[JOSE-80] Remove nonsensical empty string test

3d374e6

[JOSE-80] Rename TestKey.plist -> TestKeys.plist

6a8a3c8

[JOSE-80] Add decrypter test for zero-length data

09b1963

ghost reviewed Jul 30, 2018

View reviewed changes

mohemian-92817281 added 5 commits July 30, 2018 11:22

[JOSE-80] Add tests for encrypting message of length mLen and `mLen…

5a75cf4

… + 1`

[JOSE-80] Better naming for zero length decryption test

b221486

[JOSE-80] Add decryption tests for len=k, len=k-1, len=k+1

130d902

Merge branch 'master' into feature/JOSE-80-improve-tests

f307b16

* master: [87] Implement direct encryption and symmetric JWKs (#92) # Conflicts: # .gitignore # Tests/JWERSATests.swift

[JOSE-80] Use Alice’s key for new tests

fdbfc93

ghost approved these changes Jul 30, 2018

View reviewed changes

mohemian-92817281 merged commit f12ec10 into master Jul 30, 2018

mohemian-92817281 deleted the feature/JOSE-80-improve-tests branch July 30, 2018 11:33

carol-mohemian reviewed Jul 30, 2018

View reviewed changes

ghost mentioned this pull request Jul 30, 2018

Change error handling in RSA #96

Open

ghost mentioned this pull request Aug 30, 2018

Prepare 1.2.0 #104

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[77] Improve test suite for RSAEncrypter/Decrypter #81

[77] Improve test suite for RSAEncrypter/Decrypter #81

mohemian-92817281 commented May 7, 2018

carol-mohemian May 7, 2018

ghost May 9, 2018

ghost May 9, 2018

ghost left a comment

ghost May 9, 2018

ghost May 9, 2018

ghost May 9, 2018

ghost May 9, 2018

mohemian-92817281 commented May 9, 2018

mohemian-92817281 commented May 14, 2018

carol-mohemian May 14, 2018

mohemian-92817281 May 14, 2018

mohemian-92817281 Jul 26, 2018

carol-mohemian left a comment •

edited

Loading

ghost commented May 24, 2018

mohemian-92817281 commented Jul 26, 2018

carol-mohemian commented Jul 27, 2018

ghost left a comment

mohemian-92817281 commented Jul 30, 2018

ghost left a comment

carol-mohemian Jul 30, 2018

ghost Jul 30, 2018

carol-mohemian Jul 30, 2018

[77] Improve test suite for RSAEncrypter/Decrypter #81

[77] Improve test suite for RSAEncrypter/Decrypter #81

Conversation

mohemian-92817281 commented May 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mohemian-92817281 commented May 9, 2018

mohemian-92817281 commented May 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carol-mohemian left a comment • edited Loading

Choose a reason for hiding this comment

ghost commented May 24, 2018

mohemian-92817281 commented Jul 26, 2018

carol-mohemian commented Jul 27, 2018

ghost left a comment

Choose a reason for hiding this comment

mohemian-92817281 commented Jul 30, 2018

ghost left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carol-mohemian left a comment •

edited

Loading