Re: [PATCH] Slightly rework corosync-keygen.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



18.07.2014 13:38, Jan Friesse wrote:
> Vladislav Bogdanov napsal(a):
>> 18.07.2014 13:24, Jan Friesse wrote:
>>> Vladislav,
>>> patch is almost perfect, but I have one nitpick. Can you please replace
>>> sequence of
>>>
>>> fprintf(stderr, ...)
>>> exit(errno)
>>>
>>> by calling err (err.h, see man errx). This will reduce 2 lines to 1 line
>>> and remove need to call strerror.
>>
>> Yes, but
>> ...
>> CONFORMING TO
>>         These functions are nonstandard BSD extensions.
>> ...
>>
> 
> True. On the other hand, according to gnulib manual, err.h is missing on
> AIX 5.1, HP-UX 11, IRIX 6.5, OSF/1 5.1, Solaris 10, mingw, MSVC 9, BeOS.
> Corosync itself doesn't support (I mean, it will probably not even
> compile there) any of these platforms, so using err.h is really not a
> big deal.

That's up to you. I do not like to make future problems by introducing
code known to be not portable.
And there were requests about at least solaris builds I think...

> 
> Regards,
>   Honza
> 
>>>
>>> Thanks,
>>>    Honza
>>>
>>>
>>>> Allow it to create keyfile not in the hardcoded location.
>>>> Drop root checks.
>>>> Minor cosmetic fixes to the man-page.
>>>>
>>>> Signed-off-by: Vladislav Bogdanov <bubble@xxxxxxxxxxxxx>
>>>> ---
>>>>    man/corosync-keygen.8   |   29 +++++++++++++++++---------
>>>>    tools/corosync-keygen.c |   51
>>>> +++++++++++++++++++++-------------------------
>>>>    2 files changed, 42 insertions(+), 38 deletions(-)
>>>>
>>>> diff --git a/man/corosync-keygen.8 b/man/corosync-keygen.8
>>>> index 5dc3f45..5aaae93 100644
>>>> --- a/man/corosync-keygen.8
>>>> +++ b/man/corosync-keygen.8
>>>> @@ -35,45 +35,47 @@
>>>>    .SH NAME
>>>>    corosync-keygen \- Generate an authentication key for Corosync.
>>>>    .SH SYNOPSIS
>>>> -.B "corosync-keygen [\-l]"
>>>> +.B "corosync-keygen [\-k <filename>] [\-l]"
>>>>    .SH DESCRIPTION
>>>>
>>>>    If you want to configure corosync to use cryptographic techniques to
>>>> ensure authenticity
>>>> -.br
>>>>    and privacy of the messages, you will need to generate a private
>>>> key.
>>>>    .PP
>>>>    .B corosync-keygen
>>>> -creates this key and writes it to /etc/corosync/authkey.
>>>> +creates this key and writes it to /etc/corosync/authkey or to file
>>>> specified by
>>>> +-k option.
>>>>    .PP
>>>>    This private key must be copied to every processor in the cluster.
>>>> If the
>>>> -.br
>>>>    private key isn't the same for every node, those nodes with
>>>> nonmatching private
>>>> -.br
>>>>    keys will not be able to join the same configuration.
>>>>    .PP
>>>>    Copy the key to some security transportable storage or use ssh to
>>>> transmit the
>>>> -.br
>>>>    key from node to node.  Then install the key with the command:
>>>>    .PP
>>>>    unix#: install -D --group=0 --owner=0 --mode=0400
>>>> /path_to_authkey/authkey /etc/corosync/authkey
>>>>    .PP
>>>>    If a message "Invalid digest" appears from the corosync executive,
>>>> the keys
>>>> -.br
>>>>    are not consistent between processors.
>>>>    .PP
>>>>    .B Note: corosync-keygen
>>>>    will ask for user input to assist in generating entropy unless the
>>>> -l option is used.
>>>>    .SH OPTIONS
>>>>    .TP
>>>> +.B -k <filename>
>>>> +This specifies the fully qualified path to the shared key to create.
>>>> +.br
>>>> +The default is /etc/corosync/authkey.
>>>> +.TP
>>>>    .B -l
>>>>    Use a less secure random data source that will not require user
>>>> input to help generate
>>>> -.br
>>>> -entropy.  This may be useful when this utility is used from a script.
>>>> +entropy.  This may be useful when this utility is used from a script
>>>> or hardware random number
>>>> +generator is not available (f.e. in virtual machine).
>>>> +
>>>>    .SH EXAMPLES
>>>>    .TP
>>>>    Generate the key.
>>>>    .PP
>>>> -$ corosync-keygen
>>>> +# corosync-keygen
>>>>    .br
>>>>    Corosync Cluster Engine Authentication key generator.
>>>>    .br
>>>> @@ -81,6 +83,13 @@ Gathering 1024 bits for key from /dev/random.
>>>>    .br
>>>>    Press keys on your keyboard to generate entropy.
>>>>    .br
>>>> +.PP
>>>> +$ corosync-keygen -l -k /tmp/authkey
>>>> +.br
>>>> +Corosync Cluster Engine Authentication key generator.
>>>> +.br
>>>> +Writing corosync key to /tmp/authkey.
>>>> +.br
>>>>    .SH SEE ALSO
>>>>    .BR corosync_overview (8),
>>>>    .BR corosync.conf (5),
>>>> diff --git a/tools/corosync-keygen.c b/tools/corosync-keygen.c
>>>> index 71ea9d8..112ebaf 100644
>>>> --- a/tools/corosync-keygen.c
>>>> +++ b/tools/corosync-keygen.c
>>>> @@ -40,16 +40,19 @@
>>>>    #include <unistd.h>
>>>>    #include <fcntl.h>
>>>>    #include <errno.h>
>>>> +#include <string.h>
>>>>    #include <getopt.h>
>>>>    #include <sys/types.h>
>>>>    #include <sys/stat.h>
>>>>
>>>>    #include <netinet/in.h>
>>>>
>>>> -#define KEYFILE COROSYSCONFDIR "/authkey"
>>>> +#define DEFAULT_KEYFILE COROSYSCONFDIR "/authkey"
>>>>
>>>>    static const char usage[] =
>>>> -    "Usage: corosync-keygen [-l]\n"
>>>> +    "Usage: corosync-keygen [-k <keyfile>] [-l]\n"
>>>> +    "     -k / --key-file=<filename> -  Write to the specified
>>>> keyfile\n"
>>>> +    "            instead of the default " DEFAULT_KEYFILE ".\n"
>>>>        "     -l / --less-secure -  Use a less secure random number
>>>> source\n"
>>>>        "            (/dev/urandom) that is guaranteed not to require
>>>> user\n"
>>>>        "            input for entropy.  This can be used when this\n"
>>>> @@ -60,6 +63,7 @@ int main (int argc, char *argv[])
>>>>    {
>>>>        int authkey_fd;
>>>>        int random_fd;
>>>> +    char *keyfile = NULL;
>>>>        unsigned char key[128];
>>>>        ssize_t res;
>>>>        ssize_t bytes_read;
>>>> @@ -67,14 +71,18 @@ int main (int argc, char *argv[])
>>>>        int option_index;
>>>>        int less_secure = 0;
>>>>        static struct option long_options[] = {
>>>> -        { "less-secure", no_argument, NULL, 'l' },
>>>> -        { "help",        no_argument, NULL, 'h' },
>>>> -        { 0,             0,           NULL, 0   },
>>>> +        { "key-file",    required_argument, NULL, 'k' },
>>>> +        { "less-secure", no_argument,       NULL, 'l' },
>>>> +        { "help",        no_argument,       NULL, 'h' },
>>>> +        { 0,             0,                 NULL, 0   },
>>>>        };
>>>>
>>>> -    while ((c = getopt_long (argc, argv, "lh",
>>>> +    while ((c = getopt_long (argc, argv, "k:lh",
>>>>                long_options, &option_index)) != -1) {
>>>>            switch (c) {
>>>> +        case 'k':
>>>> +            keyfile = optarg;
>>>> +            break;
>>>>            case 'l':
>>>>                less_secure = 1;
>>>>                break;
>>>> @@ -89,18 +97,13 @@ int main (int argc, char *argv[])
>>>>        }
>>>>
>>>>        printf ("Corosync Cluster Engine Authentication key
>>>> generator.\n");
>>>> -    if (geteuid() != 0) {
>>>> -        printf ("Error: Authorization key must be generated as root
>>>> user.\n");
>>>> -        exit (errno);
>>>> -    }
>>>> -    if (mkdir (COROSYSCONFDIR, 0700)) {
>>>> -        if (errno != EEXIST) {
>>>> -            perror ("Failed to create directory: " COROSYSCONFDIR);
>>>> -            exit (errno);
>>>> -        }
>>>> +
>>>> +    if (!keyfile) {
>>>> +        keyfile = (char *)DEFAULT_KEYFILE;
>>>>        }
>>>>
>>>>        if (less_secure) {
>>>> +        printf ("Gathering %lu bits for key from /dev/urandom.\n",
>>>> (unsigned long)(sizeof (key) * 8));
>>>>            random_fd = open ("/dev/urandom", O_RDONLY);
>>>>        } else {
>>>>            printf ("Gathering %lu bits for key from /dev/random.\n",
>>>> (unsigned long)(sizeof (key) * 8));
>>>> @@ -134,17 +137,9 @@ retry_read:
>>>>        /*
>>>>         * Open key
>>>>         */
>>>> -    authkey_fd = open (KEYFILE, O_CREAT|O_WRONLY, 600);
>>>> +    authkey_fd = open (keyfile, O_CREAT|O_WRONLY, 0600);
>>>>        if (authkey_fd == -1) {
>>>> -        perror ("Could not create " KEYFILE);
>>>> -        exit (errno);
>>>> -    }
>>>> -    /*
>>>> -     * Set security of authorization key to uid = 0 gid = 0 mode =
>>>> 0400
>>>> -     */
>>>> -    res = fchown (authkey_fd, 0, 0);
>>>> -    if (res == -1) {
>>>> -        perror ("Could not fchown key to uid 0 and gid 0\n");
>>>> +        fprintf (stderr, "Could not create %s: %s", keyfile,
>>>> strerror(errno));
>>>>            exit (errno);
>>>>        }
>>>>        if (fchmod (authkey_fd, 0400)) {
>>>> @@ -152,19 +147,19 @@ retry_read:
>>>>            exit (errno);
>>>>        }
>>>>
>>>> -    printf ("Writing corosync key to " KEYFILE ".\n");
>>>> +    printf ("Writing corosync key to %s.\n", keyfile);
>>>>
>>>>        /*
>>>>         * Write key
>>>>         */
>>>>        res = write (authkey_fd, key, sizeof (key));
>>>>        if (res != sizeof (key)) {
>>>> -        perror ("Could not write " KEYFILE);
>>>> +        fprintf (stderr, "Could not write %s: %s", keyfile,
>>>> strerror(errno));
>>>>            exit (errno);
>>>>        }
>>>>
>>>>        if (close (authkey_fd)) {
>>>> -        perror ("Could not write " KEYFILE);
>>>> +        fprintf (stderr, "Could not close %s: %s", keyfile,
>>>> strerror(errno));
>>>>            exit (errno);
>>>>        }
>>>>
>>>>
>>>
>>
> 

_______________________________________________
discuss mailing list
discuss@xxxxxxxxxxxx
http://lists.corosync.org/mailman/listinfo/discuss




[Index of Archives]     [Linux Clusters]     [Corosync Project]     [Linux USB Devel]     [Linux Audio Users]     [Photo]     [Yosemite News]    [Yosemite Photos]    [Linux Kernel]     [Linux SCSI]     [X.Org]

  Powered by Linux