Merge branch 'topic/ssl-check-pkey-private'

* topic/ssl-check-pkey-private:
  ssl: reject keys without private components
  ssl: remove unneeded instance variable x509 and key from SSL::SSLSocket
  pkey: remove unused things

4 files changed, 36 insertions, 36 deletions

diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c
index 2058af74..7b237337 100644
--- a/ext/openssl/ossl_pkey.c
+++ b/ext/openssl/ossl_pkey.c
@@ -15,7 +15,7 @@
 VALUE mPKey;
 VALUE cPKey;
 VALUE ePKeyError;
-ID id_private_q;
+static ID id_private_q;
 
 /*
  * callback for generating keys
@@ -197,20 +197,6 @@ DupPKeyPtr(VALUE obj)
     return pkey;
 }
 
-EVP_PKEY *
-DupPrivPKeyPtr(VALUE obj)
-{
-    EVP_PKEY *pkey;
-
-    if (rb_funcallv(obj, id_private_q, 0, NULL) != Qtrue) {
-	ossl_raise(rb_eArgError, "Private key is needed.");
-    }
-    SafeGetPKey(obj, pkey);
-    EVP_PKEY_up_ref(pkey);
-
-    return pkey;
-}
-
 /*
  * Private
  */
@@ -272,9 +258,7 @@ ossl_pkey_sign(VALUE self, VALUE digest, VALUE data)
     VALUE str;
     int result;
 
-    if (rb_funcallv(self, id_private_q, 0, NULL) != Qtrue)
-	ossl_raise(rb_eArgError, "Private key is needed.");
-    GetPKey(self, pkey);
+    pkey = GetPrivPKeyPtr(self);
     md = GetDigestPtr(digest);
     StringValue(data);
     str = rb_str_new(0, EVP_PKEY_size(pkey)+16);
diff --git a/ext/openssl/ossl_pkey.h b/ext/openssl/ossl_pkey.h
index 1f68352c..218f2ebb 100644
--- a/ext/openssl/ossl_pkey.h
+++ b/ext/openssl/ossl_pkey.h
@@ -13,7 +13,6 @@
 extern VALUE mPKey;
 extern VALUE cPKey;
 extern VALUE ePKeyError;
-extern ID id_private_q;
 extern const rb_data_type_t ossl_evp_pkey_type;
 
 #define OSSL_PKEY_SET_PRIVATE(obj) rb_iv_set((obj), "private", Qtrue)
@@ -53,7 +52,6 @@ VALUE ossl_pkey_new_from_file(VALUE);
 EVP_PKEY *GetPKeyPtr(VALUE);
 EVP_PKEY *DupPKeyPtr(VALUE);
 EVP_PKEY *GetPrivPKeyPtr(VALUE);
-EVP_PKEY *DupPrivPKeyPtr(VALUE);
 void Init_ossl_pkey(void);
 
 /*
diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c
index 5044c6d1..7ac99247 100644
--- a/ext/openssl/ossl_ssl.c
+++ b/ext/openssl/ossl_ssl.c
@@ -67,15 +67,11 @@ static VALUE eSSLErrorWaitWritable;
 
 #define ossl_ssl_get_io(o)           rb_iv_get((o),"@io")
 #define ossl_ssl_get_ctx(o)          rb_iv_get((o),"@context")
-#define ossl_ssl_get_x509(o)         rb_iv_get((o),"@x509")
-#define ossl_ssl_get_key(o)          rb_iv_get((o),"@key")
 
 #define ossl_ssl_set_io(o,v)         rb_iv_set((o),"@io",(v))
 #define ossl_ssl_set_ctx(o,v)        rb_iv_set((o),"@context",(v))
 #define ossl_ssl_set_sync_close(o,v) rb_iv_set((o),"@sync_close",(v))
 #define ossl_ssl_set_hostname_v(o,v) rb_iv_set((o),"@hostname",(v))
-#define ossl_ssl_set_x509(o,v)       rb_iv_set((o),"@x509",(v))
-#define ossl_ssl_set_key(o,v)        rb_iv_set((o),"@key",(v))
 #define ossl_ssl_set_tmp_dh(o,v)     rb_iv_set((o),"@tmp_dh",(v))
 #define ossl_ssl_set_tmp_ecdh(o,v)   rb_iv_set((o),"@tmp_ecdh",(v))
 
@@ -225,28 +221,30 @@ ossl_call_client_cert_cb(VALUE obj)
 {
     VALUE cb, ary, cert, key;
 
-    cb = rb_funcall(obj, rb_intern("client_cert_cb"), 0);
-    if (NIL_P(cb)) return Qfalse;
+    cb = ossl_sslctx_get_client_cert_cb(ossl_ssl_get_ctx(obj));
+    if (NIL_P(cb))
+	return Qnil;
+
     ary = rb_funcall(cb, rb_intern("call"), 1, obj);
     Check_Type(ary, T_ARRAY);
     GetX509CertPtr(cert = rb_ary_entry(ary, 0));
-    GetPKeyPtr(key = rb_ary_entry(ary, 1));
-    ossl_ssl_set_x509(obj, cert);
-    ossl_ssl_set_key(obj, key);
+    GetPrivPKeyPtr(key = rb_ary_entry(ary, 1));
 
-    return Qtrue;
+    return rb_ary_new3(2, cert, key);
 }
 
 static int
 ossl_client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
 {
-    VALUE obj, success;
+    VALUE obj, ret;
 
     obj = (VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx);
-    success = rb_protect(ossl_call_client_cert_cb, obj, NULL);
-    if (!RTEST(success)) return 0;
-    *x509 = DupX509CertPtr(ossl_ssl_get_x509(obj));
-    *pkey = DupPKeyPtr(ossl_ssl_get_key(obj));
+    ret = rb_protect(ossl_call_client_cert_cb, obj, NULL);
+    if (NIL_P(ret))
+	return 0;
+
+    *x509 = DupX509CertPtr(RARRAY_AREF(ret, 0));
+    *pkey = DupPKeyPtr(RARRAY_AREF(ret, 1));
 
     return 1;
 }
@@ -776,7 +774,7 @@ ossl_sslctx_setup(VALUE self)
     val = ossl_sslctx_get_cert(self);
     cert = NIL_P(val) ? NULL : GetX509CertPtr(val); /* NO DUP NEEDED */
     val = ossl_sslctx_get_key(self);
-    key = NIL_P(val) ? NULL : GetPKeyPtr(val); /* NO DUP NEEDED */
+    key = NIL_P(val) ? NULL : GetPrivPKeyPtr(val); /* NO DUP NEEDED */
     if (cert && key) {
         if (!SSL_CTX_use_certificate(ctx, cert)) {
             /* Adds a ref => Safe to FREE */
diff --git a/test/test_ssl.rb b/test/test_ssl.rb
index a8d6539f..ad3d5af1 100644
--- a/test/test_ssl.rb
+++ b/test/test_ssl.rb
@@ -236,6 +236,26 @@ class OpenSSL::TestSSL < OpenSSL::SSLTestCase
     }
   end
 
+  def test_client_auth_public_key
+    vflag = OpenSSL::SSL::VERIFY_PEER|OpenSSL::SSL::VERIFY_FAIL_IF_NO_PEER_CERT
+    start_server(vflag, true, ignore_listener_error: true) do |server, port|
+      assert_raise(ArgumentError) {
+        ctx = OpenSSL::SSL::SSLContext.new
+        ctx.key = @cli_key.public_key
+        ctx.cert = @cli_cert
+        server_connect(port, ctx) { }
+      }
+
+      assert_raise(OpenSSL::SSL::SSLError) {
+        ctx = OpenSSL::SSL::SSLContext.new
+        ctx.client_cert_cb = Proc.new{ |ssl|
+          [@cli_cert, @cli_key.public_key]
+        }
+        server_connect(port, ctx) { }
+      }
+    end
+  end
+
   def test_client_ca
     ctx_proc = Proc.new do |ctx|
       ctx.client_ca = [@ca_cert]