Merge lp:~laurynas-biveinis/percona-pam-for-mysql/import-dialog into lp:percona-pam-for-mysql

[Laurynas Biveinis]

Copy plugin/auth_example/dialog.c from lp:mysql-server/5.5 revno 3638.

This is a preparation for dialog changes not to echo passwords with the default ask function. No source proper changes in this MP to have better change history.

MariaDB folks, if I understand correctly, this is unnecessary for you. Please let me know if there is a configure-time check I could use to disable building dialog under MariaDB. Thanks!

[Sergei Golubchik (sergii)]

Hi, Laurynas!

On Dec 06, Laurynas Biveinis wrote:
> Laurynas Biveinis has proposed merging lp:~laurynas-biveinis/percona-pam-for-mysql/import-dialog into lp:percona-pam-for-mysql.
>
> Requested reviews:
> pam-for-mysql-developers (pam-for-mysql-developers)
>
> For more details, see:
> https://code.launchpad.net/~laurynas-biveinis/percona-pam-for-mysql/import-dialog/+merge/84616
>
> Copy plugin/auth_example/dialog.c from lp:mysql-server/5.5 revno 3638.
>
> This is a preparation for dialog changes not to echo passwords with
> the default ask function. No source proper changes in this MP to have
> better change history.
>
> MariaDB folks, if I understand correctly, this is unnecessary for you.
> Please let me know if there is a configure-time check I could use to
> disable building dialog under MariaDB. Thanks!

Copy dialog.c from the latest mariadb-5.2 instead. In has numerous bug
fixes:

1. Works on windows. That is, it loads and callback works.
2. Better behavior, regarding when to send a password (from mysql -pXXX)
3. Does not echo the password input by default. No, it's not just using
   getpass - but a more complex and portable logic, see get_tty_password
   from mysql or mariadb client library.

you may be also interested to see the dialog and pam tests in out test suite.

Regards,
Sergei

[Laurynas Biveinis (laurynas-biveinis)]

Sergei -

Thanks for your comments! I have reviewed MariaDB's dialog.c, compared it against the Oracle version and I think there is not much difference in either picking Oracle version and then cherry-picking your changes or taking yours and cherry-picking Oracle API (get_tty_password vs get_tty_password_buf) and plugin declaration stuff. It seems to me that the former option is easier, thus my original MP still stands.

Also, what about my original question? Would you like to have dialog compilation disabled if building with MariaDB and if yes, what's the official way to do that?

[Sergei Golubchik (sergii)]

Hi, Laurynas!

On Dec 08, Laurynas Biveinis wrote:
> Sergei -
>
> Thanks for your comments! I have reviewed MariaDB's dialog.c, compared
> it against the Oracle version and I think there is not much difference
> in either picking Oracle version and then cherry-picking your changes
> or taking yours and cherry-picking Oracle API (get_tty_password vs
> get_tty_password_buf) and plugin declaration stuff. It seems to me
> that the former option is easier, thus my original MP still stands.

get_tty_password_buf() is something I've added just a few days ago. If
you copy dialog.c into your project, then, I suppose, you need to copy
get_tty_password_buf() too. You cannot expect the client to have it.

> Also, what about my original question? Would you like to have dialog
> compilation disabled if building with MariaDB and if yes, what's the
> official way to do that?

Don't worry about it. When we'll decide to include percona pam plugin,
I'l handle this (and plug.in file, and the rest of the integration).

Regards,
Sergei

[Laurynas Biveinis (laurynas-biveinis)]

2011/12/8 <email address hidden>:
> get_tty_password_buf() is something I've added just a few days ago.  If
> you copy dialog.c into your project, then, I suppose, you need to copy
> get_tty_password_buf() too. You cannot expect the client to have it.

Well, I can just use the original get_tty_password(), cannot I?

--
Laurynas

[Sergei Golubchik (sergii)]

Hi, Laurynas!

On Dec 09, Laurynas Biveinis wrote:
> 2011/12/8 <email address hidden>:
> > get_tty_password_buf() is something I've added just a few days ago.  If
> > you copy dialog.c into your project, then, I suppose, you need to copy
> > get_tty_password_buf() too. You cannot expect the client to have it.
>
> Well, I can just use the original get_tty_password(), cannot I?

From vanilla MySQL? Yes, but it does unnecessary my_strdup().
All I've done was to remove the my_strdup() and use a preallocated
buffer. Compare our libmysql/get_password.c and the vanilla one.

Regards,
Sergei

[Laurynas Biveinis (laurynas-biveinis)]

Sergei -

2011/12/9 Sergei <email address hidden>:
>> Well, I can just use the original get_tty_password(), cannot I?
>
> >From vanilla MySQL? Yes, but it does unnecessary my_strdup().
> All I've done was to remove the my_strdup() and use a preallocated
> buffer. Compare our libmysql/get_password.c and the vanilla one.

Yes, I have compared all the password getters I was able to find. To
use the non-buf version I only have to link with libmysqlclient -
which is one of the installed libs - and I don't have to import any
sources or anything. It has the extra my_strdup(), but it seems a
pretty good trade-off to me.

--
Laurynas

[Sergei Golubchik (sergii)]

Hi, Laurynas!

On Dec 09, Laurynas Biveinis wrote:
> Sergei -
>
> 2011/12/9 Sergei <email address hidden>:
> >> Well, I can just use the original get_tty_password(), cannot I?
> >
> > >From vanilla MySQL? Yes, but it does unnecessary my_strdup().
> > All I've done was to remove the my_strdup() and use a preallocated
> > buffer. Compare our libmysql/get_password.c and the vanilla one.
>
> Yes, I have compared all the password getters I was able to find. To
> use the non-buf version I only have to link with libmysqlclient -
> which is one of the installed libs - and I don't have to import any
> sources or anything. It has the extra my_strdup(), but it seems a
> pretty good trade-off to me.

Ah, that's what you mean. Okay, that should work, I suppose.
Just check that it works also with clients that are linked statically
with libmysqlclient.a (or document that it doesn't).

Regards,
Sergei

[Laurynas Biveinis (laurynas-biveinis)]

Sergei -

Thanks, I'll note to check the issue of linking libmysqlclient statically through the client and dynamically through the plugin at the same time.

Other than that, I'll assume that your comments mean an approval of the MP.

Thank you,
Laurynas

review: Approve