Vladislav
> Jan,
>
> 16.07.2014 18:05, Jan Friesse wrote:
>> Vladislav,
>>
>>
>>> Hi Jan,
>>>
>>> 16.07.2014 17:18, Jan Friesse wrote:
>>>> Vladislav,
>>>> again, nice idea, but. I would rather see use of CLI option + comments
>>>> inside.
>>>
>>> I thought about that, but decided to go the same way
>>> corosync_overview(8) describes. Otherwise I'd need to patch main.c for
>>
>> Honestly, I don't see any relation between corosync-keygen and
>> corosync_overview. In corosync_overview, it's clear:
>>
>> The *corosync executive process* uses four environment variables *during
>> startup*.
>
> Two btw.
>
>>
>> corosync-keygen is just totally different tool. You can generate key on
>> your own (dd) or (and this is usually the case) you just copy from other
>> node.
>>
>>> consistency and thus possibly change functionality someone already uses.
>>
>> I believe this is acceptable inconsistency. Actually, using key
>> generator with given keyfile is just ... weird. It's like dd taking
>> parameter from env.
>
> Ok.
> Are
> + if (geteuid() != 0 && !keyfile) {
> and
> + if (geteuid() == 0) {
> + res = fchown (authkey_fd, 0, 0);
> ok for you?
I'm not exactly sure what you mean, but I will try to express my opinion.
First test of root can be removed completely. /etc/corosync directory is
writable only for root, so non-root user will get messages about unable
to write file. I don't understand why this code was there.
Second test + fchown seems again useless. I mean, if tool is executed by
root, file will be owned by root. If not, then nothing happens.
I want to make sure that you've understand that I'm not criticizing your
work. Actually, I'm really really happy that somebody look to this code
which was there for AGES without any change. And because you are
changing code anyway, I would like to change it properly + remove old
meaningless dust.
Thanks,
Honza
>
>>
>> Regards,
>> Honza
>>
>>>
>>>>
>>>>
>>>> Vladislav Bogdanov napsal(a):
>>>>> Signed-off-by: Vladislav Bogdanov <bubble@xxxxxxxxxxxxx>
>>>>> ---
>>>>> man/corosync-keygen.8 | 27 +++++++++++++++++++--------
>>>>> tools/corosync-keygen.c | 38 ++++++++++++++++++++++----------------
>>>>> 2 files changed, 41 insertions(+), 24 deletions(-)
>>>>>
>>>>> diff --git a/man/corosync-keygen.8 b/man/corosync-keygen.8
>>>>> index 5dc3f45..71ca40e 100644
>>>>> --- a/man/corosync-keygen.8
>>>>> +++ b/man/corosync-keygen.8
>>>>> @@ -39,26 +39,22 @@ corosync-keygen \- Generate an authentication key for Corosync.
>>>>> .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
>>>>> +COROSYNC_TOTEM_AUTHKEY_FILE environment variable.
>>>>> .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
>>>>> @@ -67,13 +63,21 @@ will ask for user input to assist in generating entropy unless the -l option is
>>>>> .TP
>>>>> .B -l
>>>>> Use a less secure random data source that will not require user input to help generate
>>>>> +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 ENVIRONMENT VARIABLES
>>>>> +.TP
>>>>> +COROSYNC_TOTEM_AUTHKEY_FILE
>>>>> +This specifies the fully qualified path to the shared key to create.
>>>>> .br
>>>>> -entropy. This may be useful when this utility is used from a script.
>>>>> +
>>>>> +The default is /etc/corosync/authkey.
>>>>> +
>>>>> .SH EXAMPLES
>>>>> .TP
>>>>> Generate the key.
>>>>> .PP
>>>>> -$ corosync-keygen
>>>>> +# corosync-keygen
>>>>> .br
>>>>> Corosync Cluster Engine Authentication key generator.
>>>>> .br
>>>>> @@ -81,6 +85,13 @@ Gathering 1024 bits for key from /dev/random.
>>>>> .br
>>>>> Press keys on your keyboard to generate entropy.
>>>>> .br
>>>>> +.PP
>>>>> +$ COROSYNC_TOTEM_AUTHKEY_FILE=/tmp/authkey corosync-keygen -l
>>>>> +.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..519e8d9 100644
>>>>> --- a/tools/corosync-keygen.c
>>>>> +++ b/tools/corosync-keygen.c
>>>>> @@ -40,14 +40,13 @@
>>>>> #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"
>>>>> -
>>>>> static const char usage[] =
>>>>> "Usage: corosync-keygen [-l]\n"
>>>>> " -l / --less-secure - Use a less secure random number source\n"
>>>>> @@ -60,6 +59,7 @@ int main (int argc, char *argv[])
>>>>> {
>>>>> int authkey_fd;
>>>>> int random_fd;
>>>>> + const char *keyfile = getenv("COROSYNC_TOTEM_AUTHKEY_FILE");
>>>>> unsigned char key[128];
>>>>> ssize_t res;
>>>>> ssize_t bytes_read;
>>>>> @@ -89,14 +89,18 @@ int main (int argc, char *argv[])
>>>>> }
>>>>>
>>>>> printf ("Corosync Cluster Engine Authentication key generator.\n");
>>>>> - if (geteuid() != 0) {
>>>>> + if (geteuid() != 0 && !keyfile) {
>>>>> 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 = COROSYSCONFDIR "/authkey";
>>>>> + if (mkdir (COROSYSCONFDIR, 0700)) {
>>>>> + if (errno != EEXIST) {
>>>>> + perror ("Failed to create directory: " COROSYSCONFDIR);
>>>>> + exit (errno);
>>>>> + }
>>>>> }
>>>>> }
>>>>>
>>>>> @@ -134,37 +138,39 @@ 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);
>>>>> + fprintf (stderr, "Could not create %s: %s", keyfile, strerror(errno));
>>>>> 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");
>>>>> - exit (errno);
>>>>> + if (geteuid() == 0) {
>>>>> + res = fchown (authkey_fd, 0, 0);
>>>>> + if (res == -1) {
>>>>> + perror ("Could not fchown key to uid 0 and gid 0\n");
>>>>> + exit (errno);
>>>>> + }
>>>>> }
>>>>> if (fchmod (authkey_fd, 0400)) {
>>>>> perror ("Failed to set key file permissions to 0400\n");
>>>>> 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);
>>>>
>>>> Please use err/errx (see errx(3)) instead of fprintf and exit (err does
>>>> this in one function, and it's standard).
>>>>
>>>> Please DO not use errno as return code. Errno (potentially) can be >
>>>> 127. Error code > 127 is used when signal arrived (this is not the case
>>>> here).
>>>>
>>>> Regards,
>>>> Honza
>>>>
>>>>
>>>>> }
>>>>>
>>>>> if (close (authkey_fd)) {
>>>>> - perror ("Could not write " KEYFILE);
>>>>> + fprintf (stderr, "Could not write %s: %s", keyfile, strerror(errno));
>>>>> exit (errno);
>>>>> }
>>>>>
>>>>>
>>>>
>>>
>>
>
_______________________________________________
discuss mailing list
discuss@xxxxxxxxxxxx
http://lists.corosync.org/mailman/listinfo/discuss
