Test (missing) RSA key support in Secure Session (#710)

* Add Go tests
* Add JS tests
* Add Python tests, fix Python wrapper
* Add Ruby tests, fix Ruby wrapper
* Add C++ tests, fix typos
* Check Secure Session constuction failure in ObjCThemis
  Verify that Secure Session has been constructed successfully. This
  should catch RSA keys being passed to the initializer earlier.
* Update changelog, mention fixes of NodeJS (#698), Python, Ruby,
  ObjC, Swift wrappers
* Update .gitignore by adding few things like Ruby `.gem` packages,
  Python package metadata, another Rust `target` directory

The 'tests' above are:
* Whether Secure Session constructor accepts EC private key
* Whether Secure Session constructor rejects EC public and
  RSA private and RSA public keys

The fixes mentioned above were wrong/missing checks whether
`secure_session_create()` returned NULL. Python and Ruby were doing it
wrong and didn't throw the exception while ObjC was completely missing
the check.

Co-authored-by: Alexei Lozovsky <[email protected]>

.gitignore

@@ -46,6 +46,10 @@ __pycache__/
 *.py[cod]
 *$py.class
 
+src/wrappers/themis/python/dist
+src/wrappers/themis/python/files3.txt
+src/wrappers/themis/python/pythemis.egg-info
+
 tests/phpthemis/vendor
 tests/phpthemis/composer.json
 tests/phpthemis/composer.phar
@@ -75,9 +79,13 @@ docs/examples/objc/iOS-Carthage/Carthage
 
 # rust
 /target
+/src/wrappers/themis/rust/target
 **/*.rs.bk
 Cargo.lock
 
+# ruby
+/src/wrappers/themis/ruby/*.gem
+
 # vscode
 .settings
-.project
+.project

CHANGELOG.md

@@ -12,22 +12,36 @@ _Code:_
   - `make deb` and `make rpm` with `ENGINE=boringssl` will now produce `libthemis-boringssl` packages with embedded BoringSSL ([#683](https://github.com/cossacklabs/themis/pull/683), [#686](https://github.com/cossacklabs/themis/pull/686)).
   - `secure_session_create()` now allows only EC keys, returning an error for RSA ([#693](https://github.com/cossacklabs/themis/pull/693)).
 
+- **Objective-C**
+
+  - Updated Objective-C examples (iOS and macOS, Carthage and CocoaPods) to showcase usage of the newest Secure Cell API: generating symmetric keys and using Secure Cell with Passphrase ([#688](https://github.com/cossacklabs/themis/pull/688)) and to use latest Themis 0.13.2 ([#701](https://github.com/cossacklabs/themis/pull/701), [#703](https://github.com/cossacklabs/themis/pull/703), [#706](https://github.com/cossacklabs/themis/pull/706)).
+  - `TSSession` initializer now returns an error (`nil`) when given incorrect key type ([#710](https://github.com/cossacklabs/themis/pull/710)).
+
 - **PHP**
 
   - `libphpthemis` packages for Debian/Ubuntu now have accurate dependencies ([#683](https://github.com/cossacklabs/themis/pull/683)).
 
-- **Swift**
+- **Node.js**
 
-  - Updated Swift examples (iOS and macOS, Carthage and CocoaPods) to showcase usage of the newest Secure Cell API: generating symmetric keys and using Secure Cell with Passphrase ([#688](https://github.com/cossacklabs/themis/pull/688)) and to use latest Themis 0.13.2 ([#701](https://github.com/cossacklabs/themis/pull/701), [#703](https://github.com/cossacklabs/themis/pull/703), [#706](https://github.com/cossacklabs/themis/pull/706)).
+  - `SecureSession` constructor now throws an exception when given incorrect key type ([#698](https://github.com/cossacklabs/themis/pull/698)).
 
-- **Objective-C**
+- **Python**
 
-  - Updated Objective-C examples (iOS and macOS, Carthage and CocoaPods) to showcase usage of the newest Secure Cell API: generating symmetric keys and using Secure Cell with Passphrase ([#688](https://github.com/cossacklabs/themis/pull/688)) and to use latest Themis 0.13.2 ([#701](https://github.com/cossacklabs/themis/pull/701), [#703](https://github.com/cossacklabs/themis/pull/703), [#706](https://github.com/cossacklabs/themis/pull/706)).
+  - `SSession` constructor now throws an exception when given incorrect key type ([#710](https://github.com/cossacklabs/themis/pull/710)).
+
+- **Ruby**
+
+  - `Ssession` constructor now throws an exception when given incorrect key type ([#710](https://github.com/cossacklabs/themis/pull/710)).
 
 - **Rust**
 
   - Dropped `libthemis-src` crate support and removed the `vendored` feature. RustThemis wrapper now requires Themis Core to be installed in the system ([#691](https://github.com/cossacklabs/themis/pull/691)).
 
+- **Swift**
+
+  - Updated Swift examples (iOS and macOS, Carthage and CocoaPods) to showcase usage of the newest Secure Cell API: generating symmetric keys and using Secure Cell with Passphrase ([#688](https://github.com/cossacklabs/themis/pull/688)) and to use latest Themis 0.13.2 ([#701](https://github.com/cossacklabs/themis/pull/701), [#703](https://github.com/cossacklabs/themis/pull/703), [#706](https://github.com/cossacklabs/themis/pull/706)).
+  - `TSSession` initializer now returns an error (`nil`) when given incorrect key type ([#710](https://github.com/cossacklabs/themis/pull/710)).
+
 _Infrastructure:_
 
 - Improved package split making `libthemis` thinner ([#678](https://github.com/cossacklabs/themis/pull/678)).

gothemis/session/session_test.go

@@ -223,6 +223,50 @@ func testSession(keytype int, t *testing.T) {
 	<-finCh
 }
 
+func TestValidKeyHandling(t *testing.T) {
+	kpa, err := keys.New(keys.TypeEC)
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	kpb, err := keys.New(keys.TypeEC)
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	clb := &testCallbacks{kpa, kpb}
+
+	client, err := New(clientID, kpa.Private, clb)
+	if client == nil || err != nil {
+		t.Error("Creating Secure Session with EC key", err)
+		return
+	}
+}
+
+func TestInvalidKeyHandling(t *testing.T) {
+	kpa, err := keys.New(keys.TypeRSA)
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	kpb, err := keys.New(keys.TypeRSA)
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	clb := &testCallbacks{kpa, kpb}
+
+	client, err := New(clientID, kpa.Private, clb)
+	if client != nil || err == nil {
+		t.Error("Creating Secure Session with RSA key")
+		return
+	}
+}
+
 func TestSession(t *testing.T) {
 	testSession(keys.TypeEC, t)
 }

src/wrappers/themis/Obj-C/objcthemis/ssession.m

@@ -34,6 +34,9 @@ - (nullable instancetype)initWithUserId:(NSData *)userId
     if (self) {
         self.session = secure_session_create([userId bytes], [userId length],
                 [privateKey bytes], [privateKey length], [callbacks callbacks]);
+        if (!self.session) {
+            return nil;
+        }
     }
     return self;
 }

src/wrappers/themis/jsthemis/test/test.js

@@ -236,6 +236,9 @@ describe("jsthemis", function(){
         let serverID = Buffer.from('server')
         let messageA = Buffer.from('testing, testing')
         let messageB = Buffer.from('everything seems to be in order')
+        // JS API does not provide methods for generating RSA keys,
+        // we have to import them from buffers instead
+        let rsaKeyPair = addon.KeyPair(Buffer.from("UlJBMgAABJBm8NGvTdCr5R7/iNvQy+WrC6DsYPg5ze5dc1/UxsthqEE4t+4Euwq6IVdddjisxUBvzWQ0VuyQbaExbCrrIaMYHgF9DSNOyrCznrHKBlNZrfxM+pHIkVB6q+b2hLEX4j3wnMZKE0dE0W3SGO/p0oYCsF0gjC7kNTjsh67WwjsIo6EqOrAG7RF9p3UaGUNm4tGEKCAM4NE8d8URcaWSIhQIHeOrVVbziWdMFiOX2GRGwpi2lWFxTLhI1Tyyov+NLy/J5b7NL8n7qusC13eG2XPMrEucuhZtihczn5l4wGkrPi/+jAW+Kn09rQERAPe1rOXHcpu2PzVOdO0bBkqK6IzVylgxQepMriLBQzaEZwQ3BER0byG/k6H1wcvdt9qo+6byB/jHs3hnbqJP4H9HdrJQJshxko8HqS2UHxhSFUaW2H7LR2vD68YBMnUf+JP/79D68sVWXj8/s68PqXw/iLjZcLY+94adKa8FmqYB/MD0tYXGc4+G+ZmPYZGUF1apkQOIvdI5y36+d+UmxdpGHH4KR+JcOsyIzDDTlWKbFk3sIqZzg18cFEVgK6ujc4lCOAPCbhiDic1GKeYxNDEShhI54l+b3FV2Fxy7nPVqRBBumx/Zp56qxHK32Fm2O8WuslReRxXQbOxslfyeNlnGIXMLWBXCw+eBw90pEhaJ0G//a5MXfyVF50TZB7Im9M/LeSKjzd1OQ1snqiv8yBTPfzU9tN5eAi7wkGN4t74yGz81qiY4DXJ9i/qUp6mzVuIUInNLORkA9P5GIBtOyJAgj+6b+hMufzs1Qas5YsHdWRNQm0c+iIDPP9qs5k4ik1s2sovp8rCNtOaJQzcH+RgRehvS4xxHOQuEP8lPSbN21/Ly2ygYUnnta2yK8t6Idxh7DnsjinQL5O3swzWVgCmgwgOjitRqRL9+w5LJeU1n3ZGC5I3ANXTxzT4fnSqT5RC3EVtsIQhQfX+ism7nHN4jIPDsv20seeBAPSD77ARNbl4kQacZjC1EI97u3E0uw1vtiroO2AyVIO937I7l1IR4u9SIj6imKiTrfDkie/Yi98Z9OESQJtb48Zg5IaDSccs33vbY1cE4SNVgh+b1oQTNUGL7TFCPfh2Y0i4VFnvrwQ9EDoP6UqreP/CKaI3IezvmPH3cDyS9TsCzVnwSBm+2h5dIFjZbIE3EiaM2Rtzvz2XvH7TpZ2e6Ps4l6Pg9I0MRiyABTwA5L3a8hN169D13sKXtJDpZjbixtGUCbRNK+18/wRIsWkrJF1kn5hzO63s1hFhXcuSYDxH85C3+EYu7/O76bXz9A+/yxXSDbYB5PKTzgf/WoPuLtVtZKPa7kFvekM/WHSQBtwrdZlpRqzTx+WhX5kA/FC45ADdVO4jpGJH7+9JOHsjWg1u9/7qnssTCFXz8zB+8Gf8AHfgMdARt2KN81j7EgBrL7+YC5rUu5vR+tdUheGkDjbAP6Zl266UB+iTTxBO08qXMYV9/fFSn6SAK7crwaSInntXBYgjl/JM0WKnh2e0kIgCITfswK2a9o/CM2Kk9AAEAAQ==", "base64"), Buffer.from("VVJBMgAAARB1/lOFuj7OJej4PSNDEYsgAU8AOS92vITdevQ9d7Cl7SQ6WY24sbRlAm0TSvtfP8ESLFpKyRdZJ+Yczut7NYRYV3LkmA8R/OQt/hGLu/zu+m18/QPv8sV0g22AeTyk84H/1qD7i7VbWSj2u5Bb3pDP1h0kAbcK3WZaUas08floV+ZAPxQuOQA3VTuI6RiR+/vSTh7I1oNbvf+6p7LEwhV8/MwfvBn/AB34DHQEbdijfNY+xIAay+/mAua1Lub0frXVIXhpA42wD+mZduulAfok08QTtPKlzGFff3xUp+kgCu3K8GkiJ57VwWII5fyTNFip4dntJCIAiE37MCtmvaPwjNipPQABAAE=", "base64"))
 
         function makeKeyMaterial() {
             let keys = {}
@@ -307,6 +310,27 @@ describe("jsthemis", function(){
                 expect_code(addon.INVALID_PARAMETER)
             )
         })
+        it('allows EC private key', function() {
+            let keys = makeKeyMaterial()
+            let client = new addon.SecureSession(clientID, keys.client.private(), keys.clientCallback)
+            assert.ok(client)
+        })
+        it('does not allow EC public key', function() {
+            let keyPair = new addon.KeyPair()
+            assert.throws(() => new addon.SecureSession(clientID, keyPair.public(), function(){}),
+                expect_code(addon.INVALID_PARAMETER)
+            )
+        })
+        it('does not allow RSA key', function() {
+            assert.throws(() => new addon.SecureSession(clientID, rsaKeyPair.private(), function(){}),
+                expect_code(addon.INVALID_PARAMETER)
+            )
+        })
+        it('does not allow RSA public key', function() {
+            assert.throws(() => new addon.SecureSession(clientID, rsaKeyPair.public(), function(){}),
+                expect_code(addon.INVALID_PARAMETER)
+            )
+        })
         it('does not allow empty message', function() {
             let keys = makeKeyMaterial()
             let client = new addon.SecureSession(clientID, keys.client.private(), keys.clientCallback)

src/wrappers/themis/python/pythemis/scomparator.py

@@ -42,7 +42,7 @@ class SComparator(object):
     def __init__(self, shared_secret):
         self.session_ctx = ctypes.POINTER(ctypes.c_int)
         self.comparator_ctx = scomparator_create()
-        if self.comparator_ctx is None:
+        if not self.comparator_ctx:
             raise exception.ThemisError(THEMIS_CODES.FAIL,
                                         "Secure Comparator failed creating")
         res = themis.secure_comparator_append_secret(

src/wrappers/themis/python/pythemis/ssession.py

@@ -143,7 +143,7 @@ def __init__(self, user_id, sign_key, transport):
                 ctypes.byref(ctypes.create_string_buffer(sign_key)),
                 len(sign_key),
                 ctypes.byref(self.transport_))
-        if self.session_ctx is None:
+        if not self.session_ctx:
             raise exception.ThemisError(THEMIS_CODES.FAIL,
                                         "Secure Session failed creating")
 

src/wrappers/themis/ruby/lib/rbthemis.rb

@@ -274,7 +274,7 @@ def initialize(id, private_key, transport)
       @session = secure_session_create(
         id_buf, id_length, private_key_buf, private_key_length, @callbacks)
 
-      raise ThemisError, 'Secure Session failed creating' unless @session
+      raise ThemisError, 'Secure Session failed creating' if @session.null?
     end
 
     def is_established
@@ -925,7 +925,7 @@ def initialize(shared_secret)
       shared_secret_buf, shared_secret_length =
         string_to_pointer_size(shared_secret)
       @comparator = secure_comparator_create
-      raise ThemisError, 'Secure Comparator failed creating' unless @comparator
+      raise ThemisError, 'Secure Comparator failed creating' if @comparator.null?
       res = secure_comparator_append_secret(
         @comparator, shared_secret_buf, shared_secret_length)
       if res != SUCCESS

tests/pythemis/test_ssession.py

@@ -21,6 +21,7 @@
 
 from pythemis import ssession
 from pythemis import skeygen
+from pythemis import exception
 
 q1 = deque([])
 q2 = deque([])
@@ -133,6 +134,36 @@ def test_ssession2(self):
         self.assertEqual(data, client_session.unwrap(server_session.wrap(data)))
         self.assertEqual(data, server_session.unwrap(client_session.wrap(data)))
 
+    def test_invalid_key_types(self):
+        keypair2 = skeygen.GenerateKeyPair(skeygen.KEY_PAIR_TYPE.EC)
+        user_id2 = b'user_id2'
+
+        transport = ssession.SimpleMemoryTransport(
+            user_id2,
+            keypair2.export_public_key()
+        )
+
+        keypair1 = skeygen.GenerateKeyPair(skeygen.KEY_PAIR_TYPE.EC)
+        user_id1 = b'user_id1'
+
+        self.assertIsNotNone(
+            ssession.SSession(user_id1, keypair1.export_private_key(), transport),
+            msg="EC private key should be accepted"
+        )
+
+        with self.assertRaises(exception.ThemisError, msg="EC public key should be rejected"):
+            ssession.SSession(user_id1, keypair1.export_public_key(), transport)
+
+        keypair1 = skeygen.GenerateKeyPair(skeygen.KEY_PAIR_TYPE.RSA)
+
+        with self.assertRaises(exception.ThemisError, msg="RSA key should be rejected, not supported"):
+            ssession.SSession(user_id1, keypair1.export_private_key(), transport)
+
+        with self.assertRaises(exception.ThemisError, msg="RSA public key should be rejected"):
+            ssession.SSession(user_id1, keypair1.export_public_key(), transport)
+
+        with self.assertRaises(exception.ThemisError, msg="non-valid asymm key should be rejected"):
+            ssession.SSession(user_id1, b'totally not a valid key', transport)
 
 if __name__ == '__main__':
     unittest.main()

tests/rbthemis/ssession_test.rb

@@ -56,6 +56,7 @@ class TestSession < Test::Unit::TestCase
   def setup
     @server_priv_key, @server_pub_key = Themis::SKeyPairGen.new.ec
     @client_priv_key, @client_pub_key = Themis::SKeyPairGen.new.ec
+    @client_priv_key_rsa, @client_pub_key_rsa = Themis::SKeyPairGen.new.rsa
     @q1 = Queue.new
     @q2 = Queue.new
     @pub_keys = {'server' => @server_pub_key, 'client' => @client_pub_key}
@@ -93,4 +94,32 @@ def test_session
       Thread.new { client }
     ].each(&:join)
   end
+
+  def test_valid_key_private_ec
+    assert_nothing_raised do
+      session = Themis::Ssession.new(
+        'client', @client_priv_key, CallbacksForThemis.new(@pub_keys))
+    end
+  end
+
+  def test_invalid_key_public_ec
+    assert_raise(Themis::ThemisError) do
+      session = Themis::Ssession.new(
+        'client', @client_pub_key, CallbacksForThemis.new(@pub_keys))
+    end
+  end
+
+  def test_invalid_key_private_rsa
+    assert_raise(Themis::ThemisError) do
+      session = Themis::Ssession.new(
+        'client', @client_priv_key_rsa, CallbacksForThemis.new(@pub_keys))
+    end
+  end
+
+  def test_invalid_key_public_rsa
+    assert_raise(Themis::ThemisError) do
+      session = Themis::Ssession.new(
+        'client', @client_pub_key_rsa, CallbacksForThemis.new(@pub_keys))
+    end
+  end
 end