Jan,
17.07.2014 09:31, Jan Friesse wrote:
> 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.
Me too.
>
> 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.
Exactly.
>
> 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.
Sure, I understand. I'm in the same boat.
Stay tuned.
Best,
Vladislav
>
> 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
