Gareth Palmer:
> Hello,
> 
> The following patch adds support for using SSL when connecting to a
> MySQL server and support for reading my.cnf files which can set other
> connection related options.
> 
> New configuration parameters for SSL connections are -
> 
>   tls_cert_file   - File containing the client's public key.
>   tls_key_file    - File containing the client's private key.
>   tls_CAfile      - File containing the server's public key.
>   tls_CApath      - Directory containing the server's public key.
>   tls_ciphers     - A list of permissible ciphers to use for encryption.
>   tls_verify_cert - Verify that the name of the server matches the
>                     common name in the certificate.
> 
> New configuration parameters for reading my.cnf files are -
> 
>   option_file     - Name of the file to read from.
>   option_group    - Name of the group to read from.
> 
> Both are empty by default so no option file will be read from.

Thanks for cleaning up some of the internal APIs.

However, please confirm that your code works with configuration
files that do not set the new parameters.

In the following code, cfg_get_str() returns a null character pointer
when a parameter is not specified in the Postfix configuration file:

+    dict_mysql->option_file = cfg_get_str(p, "option_file", NULL, 0, 0);
+    dict_mysql->option_group = cfg_get_str(p, "option_group", NULL, 0, 0);
+#if defined(MYSQL_VERSION_ID) && MYSQL_VERSION_ID >= 40000
+    dict_mysql->tls_key_file = cfg_get_str(p, "tls_key_file", NULL, 0, 0);
+    dict_mysql->tls_cert_file = cfg_get_str(p, "tls_cert_file", NULL, 0, 0);
+    dict_mysql->tls_CAfile = cfg_get_str(p, "tls_CAfile", NULL, 0, 0);
+    dict_mysql->tls_CApath = cfg_get_str(p, "tls_CApath", NULL, 0, 0);
+    dict_mysql->tls_ciphers = cfg_get_str(p, "tls_ciphers", NULL, 0, 0);

When the database is closed, the dict_mysql_close() function passes
these null pointers to myfree(). 

+     myfree(dict_mysql->option_file);
+     myfree(dict_mysql->option_group);
+ #if defined(MYSQL_VERSION_ID) && MYSQL_VERSION_ID >= 40000
+     myfree(dict_mysql->tls_key_file);
+     myfree(dict_mysql->tls_cert_file);
+     myfree(dict_mysql->tls_CAfile);
+     myfree(dict_mysql->tls_CApath);
+     myfree(dict_mysql->tls_ciphers);
+ #endif

However.  myfree() does not accept null pointer arguments: It will
abort the program instead.

You can test the dict_mysql_close() routine with the "mail_dict"
test program:

    $ cd src/global
    $ make mail_dict
    $ ./mail_dict mysql:/path/to/file read
    get foo
    ^D

The simplest fix is to replace 

        myfree(dict_mysql->whatever);

with
        if (dict_mysql->whatever)
            myfree(dict_mysql->whatever);

I can make that change if you like.

However it would be good if you can confirm that the code
works even when none of the new parameters is specified.

        Wietse

Reply via email to