From: Nirbhay Choubey
Date: June 27 2011 12:06pm
Subject: Re: bzr commit into mysql-trunk branch (Georgi.Kodinov:3224) Bug#11753167
List-Archive: http://lists.mysql.com/commits/139918
Message-Id: <4E087240.9020506@oracle.com>
MIME-Version: 1.0
Content-Type: multipart/alternative;
 boundary="------------030503020805070502010608"

--------------030503020805070502010608
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Hi Joro,

Good work.

Please consider some suggestions in-lined.

Patch approved.

Best,
Nirbhay

On Wednesday 22 June 2011 07:21 PM, Georgi Kodinov wrote:
> #At file:///Users/kgeorge/mysql/work/B11753167-trunk/ based on revid:dmitry.lenev@stripped
>
>   3224 Georgi Kodinov	2011-06-22
>        Bug #11753167: 44559: SSL KEYS WITH PASSPHRASES
>
>        Implemented a default password reading callback for yaSSL
>        using mysql client's get_tty_password().
>        It does that by:
>        - Implementing an extended version of get_tty_password()
>        called get_tty_password_ext() that takes a strdup function
>        pointer.
>        - Adds client/get_password.c to the yassl lib and uses the
>        C preprocessor to rename get_tty_passord() and
>        get_tty_password_ext() to names prefixed
>        with yassl and internal to the yassl library.
>        Since the password can't be read from a file, only a manual
>        test performed and no automatic test case added.
>
>      modified:
>        client/get_password.c
>        extra/yassl/CMakeLists.txt
>        extra/yassl/src/yassl_int.cpp
>        include/mysql.h.pp
>        include/mysql_com.h
> === modified file 'client/get_password.c'
> --- a/client/get_password.c	2008-02-19 17:45:11 +0000
> +++ b/client/get_password.c	2011-06-22 13:51:30 +0000
> @@ -63,12 +63,13 @@
>   /* were just going to fake it here and get input from
>      the keyboard */
>
> -char *get_tty_password(const char *opt_message)
> +char *get_tty_password_ext(const char *opt_message,
> +                           char *(* strdup_function)(const char *, int))
>   {
>     char to[80];
>     char *pos=to,*end=to+sizeof(to)-1;
>     int i=0;
> -  DBUG_ENTER("get_tty_password");
> +  DBUG_ENTER("get_tty_password_ext");
>     _cputs(opt_message ? opt_message : "Enter password: ");
>     for (;;)
>     {
> @@ -94,7 +95,7 @@ char *get_tty_password(const char *opt_m
>       pos--;					/* Allow dummy space at end */
>     *pos=0;
>     _cputs("\n");
> -  DBUG_RETURN(my_strdup(to,MYF(MY_FAE)));
> +  DBUG_RETURN(strdup_function(to,MYF(MY_FAE)));
>   }
>
>   #else
> @@ -149,7 +150,8 @@ static void get_password(char *to,uint l
>   #endif /* ! HAVE_GETPASS */
>
>
> -char *get_tty_password(const char *opt_message)
> +char *get_tty_password_ext(const char *opt_message,
> +                           char *(* strdup_function)(const char *, int))
>   {
>   #ifdef HAVE_GETPASS
>     char *passbuff;
> @@ -158,7 +160,7 @@ char *get_tty_password(const char *opt_m
>   #endif /* HAVE_GETPASS */
>     char buff[80];
>
> -  DBUG_ENTER("get_tty_password");
> +  DBUG_ENTER("get_tty_password_ext");
>
>   #ifdef HAVE_GETPASS
>     passbuff = getpass(opt_message ? opt_message : "Enter password: ");
> @@ -205,7 +207,12 @@ char *get_tty_password(const char *opt_m
>       fputc('\n',stderr);
>   #endif /* HAVE_GETPASS */
>
> -  DBUG_RETURN(my_strdup(buff,MYF(MY_FAE)));
> +  DBUG_RETURN(strdup_function(buff,MYF(MY_FAE)));
>   }
>
>   #endif /*__WIN__*/
> +
> +char *get_tty_password(const char *opt_message)
> +{
> +  return get_tty_password_ext(opt_message, my_strdup);
> +}
>
> === modified file 'extra/yassl/CMakeLists.txt'
> --- a/extra/yassl/CMakeLists.txt	2011-04-04 08:47:25 +0000
> +++ b/extra/yassl/CMakeLists.txt	2011-06-22 13:51:30 +0000
> @@ -21,9 +21,14 @@ INCLUDE_DIRECTORIES(
>
>   ADD_DEFINITIONS(${SSL_DEFINES})
>
> +# rename get_tty_password to avoid collisions with the main binary
> +ADD_DEFINITIONS(-Dget_tty_password_ext=yassl_mysql_get_tty_password_ext)
> +ADD_DEFINITIONS(-Dget_tty_password=yassl_mysql_get_tty_password)
> +
>   SET(YASSL_SOURCES  src/buffer.cpp src/cert_wrapper.cpp src/crypto_wrapper.cpp src/handshake.cpp src/lock.cpp
>   				src/log.cpp src/socket_wrapper.cpp src/ssl.cpp src/timer.cpp src/yassl_error.cpp
> -				src/yassl_imp.cpp src/yassl_int.cpp)
> +				src/yassl_imp.cpp src/yassl_int.cpp
> +                                ../../client/get_password.c )
>
>   ADD_CONVENIENCE_LIBRARY(yassl ${YASSL_SOURCES})
>   RESTRICT_SYMBOL_EXPORTS(yassl)
>
> === modified file 'extra/yassl/src/yassl_int.cpp'
> --- a/extra/yassl/src/yassl_int.cpp	2011-04-04 08:47:25 +0000
> +++ b/extra/yassl/src/yassl_int.cpp	2011-06-22 13:51:30 +0000
> @@ -68,6 +68,8 @@
>
>   #endif // YASSL_PURE_C
>
> +/* for the definition of get_tty_password() */
> +#include<mysql.h>
>
>   namespace yaSSL {
>
> @@ -1799,8 +1801,41 @@ bool SSL_METHOD::multipleProtocol() cons
>   }
>
>
> +/** Implement a my_strdup replacement, so we can reuse get_password() */
> +extern "C" char *yassl_mysql_strdup(const char *from, int my_flags)
> +{
> +  return from ? strdup(from) : NULL;
> +}

Probably we should add __attribute__((unused)) for my_flags above.

> +
> +static int
> +default_password_callback(char * buffer, int size, int rwflag,
> +                          void * callback_data __attribute__((unused)))
> +{
> +  char *passwd;
> +  size_t passwd_len, ssize= (size_t) size;
> +
> +  passwd= yassl_mysql_get_tty_password_ext("Enter PEM pass phrase:",
> +                                           yassl_mysql_strdup);

We should check for NULL returns, mind the strlen that follows.

> +  passwd_len= strlen(passwd);
> +
> +  if (!passwd_len)
> +    return 0;
> +
> +  if (ssize>  0)
> +  {
> +    size_t result_len= ssize - 1>  passwd_len ?
> +      passwd_len : ssize - 1;
> +    memcpy(buffer, passwd, result_len);
> +    buffer[result_len]= 0;
> +  }
> +  free(passwd);
> +  return passwd_len;
> +}
> +
> +
>   SSL_CTX::SSL_CTX(SSL_METHOD* meth)
> -    : method_(meth), certificate_(0), privateKey_(0), passwordCb_(0),
> +    : method_(meth), certificate_(0), privateKey_(0),
> +      passwordCb_(default_password_callback),
>         userData_(0), sessionCacheOff_(false), sessionCacheFlushOff_(false),
>         verifyCallback_(0)
>   {}
>
> === modified file 'include/mysql.h.pp'
> --- a/include/mysql.h.pp	2011-05-31 13:52:09 +0000
> +++ b/include/mysql.h.pp	2011-06-22 13:51:30 +0000
> @@ -138,6 +138,8 @@ my_bool check_scramble(const unsigned ch
>   void get_salt_from_password(unsigned char *res, const char *password);
>   void make_password_from_salt(char *to, const unsigned char *hash_stage2);
>   char *octet2hex(char *to, const char *str, unsigned int len);
> +char *get_tty_password_ext(const char *opt_message,
> +                           char *(* strdup_function)(const char *, int));
>   char *get_tty_password(const char *opt_message);
>   const char *mysql_errno_to_sqlstate(unsigned int mysql_errno);
>   my_bool my_thread_init(void);
>
> === modified file 'include/mysql_com.h'
> --- a/include/mysql_com.h	2011-05-31 13:52:09 +0000
> +++ b/include/mysql_com.h	2011-06-22 13:51:30 +0000
> @@ -536,6 +536,8 @@ char *octet2hex(char *to, const char *st
>
>   /* end of password.c */
>
> +char *get_tty_password_ext(const char *opt_message,
> +                           char *(* strdup_function)(const char *, int));
>   char *get_tty_password(const char *opt_message);
>   const char *mysql_errno_to_sqlstate(unsigned int mysql_errno);
>
>
>
>
>


--------------030503020805070502010608--