Re: [PATCH] Slightly rework corosync-keygen.

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

 



18.07.2014 13:59, Jan Friesse wrote:
> Vladislav Bogdanov napsal(a):
>> 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.
> 
> Ok and I definitively appreciate it.
> 
>> And there were requests about at least solaris builds I think...
>>
> 
> To be fair (and it's my personal opinion) Solaris is dead OS. Specially
> after SUN was acquired. What seems to be alive and may have some future
> is Nexenta (we were testing Corosync 2.0 there). And Nexenta ships err.h.

Jan, I really understand your opinion, but can you make that change a
follow-up commit? Then nobody blames me... ;)

> 
> Regards,
>   Honza
> 
>>>
>>> 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