Hi, I'm sill looking at how to fix the ACLs support of pacemaker.
AFAICS, if a server is ran by root, when a client connects to it, the "/dev/shm/qb-*" file will be chown to the client's uid/gid. While if a server is ran by an ordinary user and being connected by another ordinary user, since the server doesn't have the permission to chown the file to another user, and the file mode is 0600, so the client will get "permission denied ".
Cib daemon runs as "uid: hacluster, gid: root", and we want all the users in "haclient" group have access to CIB. Is there any way for cib daemon to know the file path or the fd for a request, so that it can change the mode/group of the file? Or are there any other solutions for this?
Thanks, Gao,Yan
On 11/06/12 13:59 +0800, Gao,Yan wrote:
Hi, I'm sill looking at how to fix the ACLs support of pacemaker.
AFAICS, if a server is ran by root, when a client connects to it, the "/dev/shm/qb-*" file will be chown to the client's uid/gid. While if a server is ran by an ordinary user and being connected by another ordinary user, since the server doesn't have the permission to chown the file to another user, and the file mode is 0600, so the client will get "permission denied ".
Cib daemon runs as "uid: hacluster, gid: root", and we want all the users in "haclient" group have access to CIB. Is there any way for cib daemon to know the file path or the fd for a request, so that it can change the mode/group of the file? Or are there any other solutions for this?
One option is to change qb_rb_chown() to do the following: if (myuid == owner) { // no need to change anything return 0; } if (myuid == 0) { // normal chown (as it does now) } else if (mygid == 0 || mygid == group) { // chgrp && chmod 0660 }
Do you think this will solve your problem?
-Angus
Thanks, Gao,Yan -- Gao,Yan ygao@suse.com Software Engineer China Server Team, SUSE. _______________________________________________ quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
Hi Angus,
On 06/11/12 19:06, Angus Salkeld wrote:
On 11/06/12 13:59 +0800, Gao,Yan wrote:
Hi, I'm sill looking at how to fix the ACLs support of pacemaker.
AFAICS, if a server is ran by root, when a client connects to it, the "/dev/shm/qb-*" file will be chown to the client's uid/gid. While if a server is ran by an ordinary user and being connected by another ordinary user, since the server doesn't have the permission to chown the file to another user, and the file mode is 0600, so the client will get "permission denied ".
Cib daemon runs as "uid: hacluster, gid: root", and we want all the users in "haclient" group have access to CIB. Is there any way for cib daemon to know the file path or the fd for a request, so that it can change the mode/group of the file? Or are there any other solutions for this?
One option is to change qb_rb_chown() to do the following: if (myuid == owner) { // no need to change anything return 0; } if (myuid == 0) { // normal chown (as it does now) } else if (mygid == 0 || mygid == group) { // chgrp && chmod 0660 }
Do you think this will solve your problem?
This leads to the direction. :) One issue is:
The "group" passed into qb_rb_chown() is the client's egid. Usually the client's egid is the user's main group, which could be something like "users" group instead of "haclient" group, while it's quite possible that "hacluster" user doesn't belong to "users" group. An ordinary user is not allowed to change the group of a file to a group he doesn't belong to, even his egid is 0.
Is it possible to set the group of the file to the specified "haclient" group, otherwise ACLs would require users having same specified _main_ group.
Besides, we'll need similar changes to qb_ipcs_us_connect():958: res = chown(r->request, c->euid, c->egid);
Regards, Gao,Yan
On 11/06/12 23:25 +0800, Gao,Yan wrote:
Hi Angus,
On 06/11/12 19:06, Angus Salkeld wrote:
On 11/06/12 13:59 +0800, Gao,Yan wrote:
Hi, I'm sill looking at how to fix the ACLs support of pacemaker.
AFAICS, if a server is ran by root, when a client connects to it, the "/dev/shm/qb-*" file will be chown to the client's uid/gid. While if a server is ran by an ordinary user and being connected by another ordinary user, since the server doesn't have the permission to chown the file to another user, and the file mode is 0600, so the client will get "permission denied ".
Cib daemon runs as "uid: hacluster, gid: root", and we want all the users in "haclient" group have access to CIB. Is there any way for cib daemon to know the file path or the fd for a request, so that it can change the mode/group of the file? Or are there any other solutions for this?
One option is to change qb_rb_chown() to do the following: if (myuid == owner) { // no need to change anything return 0; } if (myuid == 0) { // normal chown (as it does now) } else if (mygid == 0 || mygid == group) { // chgrp && chmod 0660 }
Do you think this will solve your problem?
This leads to the direction. :) One issue is:
The "group" passed into qb_rb_chown() is the client's egid. Usually the client's egid is the user's main group, which could be something like "users" group instead of "haclient" group, while it's quite possible that "hacluster" user doesn't belong to "users" group. An ordinary user is not allowed to change the group of a file to a group he doesn't belong to, even his egid is 0.
well we could introduce a function that you call in the autheticate callback something like:
qb_ipcs_connection_auth_set(conn, gid, uid, mode);
Then you could set what you want. I'll have a look at getting you a patch.
-Angus
Is it possible to set the group of the file to the specified "haclient" group, otherwise ACLs would require users having same specified _main_ group.
Besides, we'll need similar changes to qb_ipcs_us_connect():958: res = chown(r->request, c->euid, c->egid);
Yip.
Regards, Gao,Yan -- Gao,Yan ygao@suse.com Software Engineer China Server Team, SUSE. _______________________________________________ quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
Signed-off-by: Angus Salkeld asalkeld@redhat.com --- include/qb/qbipcs.h | 18 ++++++++++++ include/qb/qbrb.h | 8 ++++++ lib/ipc_int.h | 7 +++++ lib/ipc_shm.c | 80 ++++++++++++++++++++++++++------------------------- lib/ipc_us.c | 5 ++-- lib/ipcs.c | 11 +++++++ lib/ringbuffer.c | 19 ++++++++++++ 7 files changed, 107 insertions(+), 41 deletions(-)
diff --git a/include/qb/qbipcs.h b/include/qb/qbipcs.h index 2bbe22e..ff0931b 100644 --- a/include/qb/qbipcs.h +++ b/include/qb/qbipcs.h @@ -117,6 +117,8 @@ struct qb_ipcs_poll_handlers { * The type of checks you should do are authentication, service availabilty * or process resource constraints. * @return 0 to accept or -errno to indicate a failure (sent back to the client) + * + * @note you can call qb_ipcs_connection_auth_set() within this function. */ typedef int32_t (*qb_ipcs_connection_accept_fn) (qb_ipcs_connection_t *c, uid_t uid, gid_t gid); @@ -377,6 +379,22 @@ qb_ipcs_connection_t * qb_ipcs_connection_first_get(qb_ipcs_service_t* pt); qb_ipcs_connection_t * qb_ipcs_connection_next_get(qb_ipcs_service_t* pt, qb_ipcs_connection_t *current);
+/** + * Set the permissions on and shared memory files so that both processes can + * read and write to them. + * + * @param uid the user id to set. + * @param gid the group id to set. + * @param mode the mode to set. + * + * @see chmod() chown() + * @note this must be called within the qb_ipcs_connection_accept_fn() + * callback. + */ +void qb_ipcs_connection_auth_set(qb_ipcs_connection_t *conn, uid_t uid, + gid_t gid, mode_t mode); + + /* *INDENT-OFF* */ #ifdef __cplusplus } diff --git a/include/qb/qbrb.h b/include/qb/qbrb.h index 8a012bd..fd37626 100644 --- a/include/qb/qbrb.h +++ b/include/qb/qbrb.h @@ -285,6 +285,14 @@ qb_ringbuffer_t *qb_rb_create_from_file(int32_t fd, uint32_t flags); */ int32_t qb_rb_chown(qb_ringbuffer_t * rb, uid_t owner, gid_t group);
+/** + * Like 'chmod' it changes the mode of the ringbuffers resources. + * @param mode mode to change to + * @param rb ringbuffer instance + * @retval 0 == ok + * @retval -errno for error + */ +int32_t qb_rb_chmod(qb_ringbuffer_t * rb, mode_t mode);
/* *INDENT-OFF* */ #ifdef __cplusplus diff --git a/lib/ipc_int.h b/lib/ipc_int.h index a569a80..14f2091 100644 --- a/lib/ipc_int.h +++ b/lib/ipc_int.h @@ -156,12 +156,19 @@ enum qb_ipcs_connection_state {
#define CONNECTION_DESCRIPTION (16)
+struct qb_ipcs_connection_auth { + uid_t uid; + gid_t gid; + mode_t mode; +}; + struct qb_ipcs_connection { enum qb_ipcs_connection_state state; int32_t refcount; pid_t pid; uid_t euid; gid_t egid; + struct qb_ipcs_connection_auth auth; struct qb_ipc_one_way setup; struct qb_ipc_one_way request; struct qb_ipc_one_way response; diff --git a/lib/ipc_shm.c b/lib/ipc_shm.c index 19c9788..3e823ea 100644 --- a/lib/ipc_shm.c +++ b/lib/ipc_shm.c @@ -234,6 +234,40 @@ qb_ipcs_shm_disconnect(struct qb_ipcs_connection *c) }
static int32_t +qb_ipcs_shm_rb_open(struct qb_ipcs_connection *c, + struct qb_ipc_one_way *ow, + const char *rb_name) +{ + int32_t res = 0; + + ow->u.shm.rb = qb_rb_open(rb_name, + ow->max_msg_size, + QB_RB_FLAG_CREATE | + QB_RB_FLAG_SHARED_PROCESS, + sizeof(int32_t)); + if (ow->u.shm.rb == NULL) { + res = -errno; + qb_util_perror(LOG_ERR, "qb_rb_open:%s", rb_name); + return res; + } + res = qb_rb_chown(ow->u.shm.rb, c->auth.uid, c->auth.gid); + if (res != 0) { + qb_util_perror(LOG_ERR, "qb_rb_chown:%s", rb_name); + goto cleanup; + } + res = qb_rb_chmod(ow->u.shm.rb, c->auth.mode); + if (res != 0) { + qb_util_perror(LOG_ERR, "qb_rb_chmod:%s", rb_name); + goto cleanup; + } + return res; + +cleanup: + qb_rb_close(ow->u.shm.rb); + return res; +} + +static int32_t qb_ipcs_shm_connect(struct qb_ipcs_service *s, struct qb_ipcs_connection *c, struct qb_ipc_connection_response *r) @@ -249,59 +283,27 @@ qb_ipcs_shm_connect(struct qb_ipcs_service *s, snprintf(r->event, NAME_MAX, "%s-event-%s", s->name, c->description);
- c->request.u.shm.rb = qb_rb_open(r->request, - c->request.max_msg_size, - QB_RB_FLAG_CREATE | - QB_RB_FLAG_SHARED_PROCESS, - sizeof(int32_t)); - if (c->request.u.shm.rb == NULL) { - res = -errno; - qb_util_perror(LOG_ERR, "qb_rb_open:%s", r->request); - goto cleanup; - } - res = qb_rb_chown(c->request.u.shm.rb, c->euid, c->egid); + res = qb_ipcs_shm_rb_open(c, &c->request, + r->request); if (res != 0) { - qb_util_perror(LOG_ERR, "qb_rb_chown:%s", r->request); goto cleanup; }
- c->response.u.shm.rb = qb_rb_open(r->response, - c->response.max_msg_size, - QB_RB_FLAG_CREATE | - QB_RB_FLAG_SHARED_PROCESS, 0); - if (c->response.u.shm.rb == NULL) { - res = -errno; - qb_util_perror(LOG_ERR, "qb_rb_open:%s", r->response); - goto cleanup_request; - } - res = qb_rb_chown(c->response.u.shm.rb, c->euid, c->egid); + res = qb_ipcs_shm_rb_open(c, &c->response, + r->response); if (res != 0) { - qb_util_perror(LOG_ERR, "qb_rb_chown:%s", r->response); goto cleanup_request; }
- c->event.u.shm.rb = qb_rb_open(r->event, - c->event.max_msg_size, - QB_RB_FLAG_CREATE | - QB_RB_FLAG_SHARED_PROCESS, 0); - - if (c->event.u.shm.rb == NULL) { - res = -errno; - qb_util_perror(LOG_ERR, "qb_rb_open:%s", r->event); - goto cleanup_request_response; - } - res = qb_rb_chown(c->event.u.shm.rb, c->euid, c->egid); + res = qb_ipcs_shm_rb_open(c, &c->event, + r->event); if (res != 0) { - qb_util_perror(LOG_ERR, "qb_rb_chown:%s", r->event); - goto cleanup_all; + goto cleanup_request_response; }
r->hdr.error = 0; return 0;
-cleanup_all: - qb_rb_close(c->event.u.shm.rb); - cleanup_request_response: qb_rb_close(c->request.u.shm.rb);
diff --git a/lib/ipc_us.c b/lib/ipc_us.c index 2808708..73c4392 100644 --- a/lib/ipc_us.c +++ b/lib/ipc_us.c @@ -615,8 +615,9 @@ handle_new_connection(struct qb_ipcs_service *s, c->response.max_msg_size = req->max_msg_size; c->event.max_msg_size = req->max_msg_size; c->pid = ugp->pid; - c->euid = ugp->uid; - c->egid = ugp->gid; + c->auth.uid = c->euid = ugp->uid; + c->auth.gid = c->egid = ugp->gid; + c->auth.mode = 0600; c->stats.client_pid = ugp->pid; snprintf(c->description, CONNECTION_DESCRIPTION, "%d-%d-%d", s->pid, ugp->pid, diff --git a/lib/ipcs.c b/lib/ipcs.c index 8370c7d..68cb0a8 100644 --- a/lib/ipcs.c +++ b/lib/ipcs.c @@ -864,3 +864,14 @@ qb_ipcs_stats_get(struct qb_ipcs_service * s, } return 0; } + +void +qb_ipcs_connection_auth_set(qb_ipcs_connection_t *c, uid_t uid, + gid_t gid, mode_t mode) +{ + if (c) { + c->auth.uid = uid; + c->auth.gid = gid; + c->auth.mode = mode; + } +} diff --git a/lib/ringbuffer.c b/lib/ringbuffer.c index 1daa0a9..7902dcd 100644 --- a/lib/ringbuffer.c +++ b/lib/ringbuffer.c @@ -769,3 +769,22 @@ qb_rb_chown(struct qb_ringbuffer_s * rb, uid_t owner, gid_t group) } return 0; } + +int32_t +qb_rb_chmod(qb_ringbuffer_t * rb, mode_t mode) +{ + int32_t res; + + if (rb == NULL) { + return -EINVAL; + } + res = chmod(rb->shared_hdr->data_path, mode); + if (res < 0) { + return -errno; + } + res = chmod(rb->shared_hdr->hdr_path, mode); + if (res < 0) { + return -errno; + } + return 0; +}
Yan, can you test and tell me if this fixes your problem? If it does I'll push the change.
Regards Angus
On 12/06/12 10:25 +1000, Angus Salkeld wrote:
Signed-off-by: Angus Salkeld asalkeld@redhat.com
include/qb/qbipcs.h | 18 ++++++++++++ include/qb/qbrb.h | 8 ++++++ lib/ipc_int.h | 7 +++++ lib/ipc_shm.c | 80 ++++++++++++++++++++++++++------------------------- lib/ipc_us.c | 5 ++-- lib/ipcs.c | 11 +++++++ lib/ringbuffer.c | 19 ++++++++++++ 7 files changed, 107 insertions(+), 41 deletions(-)
diff --git a/include/qb/qbipcs.h b/include/qb/qbipcs.h index 2bbe22e..ff0931b 100644 --- a/include/qb/qbipcs.h +++ b/include/qb/qbipcs.h @@ -117,6 +117,8 @@ struct qb_ipcs_poll_handlers {
- The type of checks you should do are authentication, service availabilty
- or process resource constraints.
- @return 0 to accept or -errno to indicate a failure (sent back to the client)
- @note you can call qb_ipcs_connection_auth_set() within this function.
*/ typedef int32_t (*qb_ipcs_connection_accept_fn) (qb_ipcs_connection_t *c, uid_t uid, gid_t gid); @@ -377,6 +379,22 @@ qb_ipcs_connection_t * qb_ipcs_connection_first_get(qb_ipcs_service_t* pt); qb_ipcs_connection_t * qb_ipcs_connection_next_get(qb_ipcs_service_t* pt, qb_ipcs_connection_t *current);
+/**
- Set the permissions on and shared memory files so that both processes can
- read and write to them.
- @param uid the user id to set.
- @param gid the group id to set.
- @param mode the mode to set.
- @see chmod() chown()
- @note this must be called within the qb_ipcs_connection_accept_fn()
- callback.
- */
+void qb_ipcs_connection_auth_set(qb_ipcs_connection_t *conn, uid_t uid,
gid_t gid, mode_t mode);/* *INDENT-OFF* */ #ifdef __cplusplus } diff --git a/include/qb/qbrb.h b/include/qb/qbrb.h index 8a012bd..fd37626 100644 --- a/include/qb/qbrb.h +++ b/include/qb/qbrb.h @@ -285,6 +285,14 @@ qb_ringbuffer_t *qb_rb_create_from_file(int32_t fd, uint32_t flags); */ int32_t qb_rb_chown(qb_ringbuffer_t * rb, uid_t owner, gid_t group);
+/**
- Like 'chmod' it changes the mode of the ringbuffers resources.
- @param mode mode to change to
- @param rb ringbuffer instance
- @retval 0 == ok
- @retval -errno for error
- */
+int32_t qb_rb_chmod(qb_ringbuffer_t * rb, mode_t mode);
/* *INDENT-OFF* */ #ifdef __cplusplus diff --git a/lib/ipc_int.h b/lib/ipc_int.h index a569a80..14f2091 100644 --- a/lib/ipc_int.h +++ b/lib/ipc_int.h @@ -156,12 +156,19 @@ enum qb_ipcs_connection_state {
#define CONNECTION_DESCRIPTION (16)
+struct qb_ipcs_connection_auth {
- uid_t uid;
- gid_t gid;
- mode_t mode;
+};
struct qb_ipcs_connection { enum qb_ipcs_connection_state state; int32_t refcount; pid_t pid; uid_t euid; gid_t egid;
- struct qb_ipcs_connection_auth auth; struct qb_ipc_one_way setup; struct qb_ipc_one_way request; struct qb_ipc_one_way response;
diff --git a/lib/ipc_shm.c b/lib/ipc_shm.c index 19c9788..3e823ea 100644 --- a/lib/ipc_shm.c +++ b/lib/ipc_shm.c @@ -234,6 +234,40 @@ qb_ipcs_shm_disconnect(struct qb_ipcs_connection *c) }
static int32_t +qb_ipcs_shm_rb_open(struct qb_ipcs_connection *c,
struct qb_ipc_one_way *ow,const char *rb_name)+{
- int32_t res = 0;
- ow->u.shm.rb = qb_rb_open(rb_name,
ow->max_msg_size,QB_RB_FLAG_CREATE |QB_RB_FLAG_SHARED_PROCESS,sizeof(int32_t));- if (ow->u.shm.rb == NULL) {
res = -errno;qb_util_perror(LOG_ERR, "qb_rb_open:%s", rb_name);return res;- }
- res = qb_rb_chown(ow->u.shm.rb, c->auth.uid, c->auth.gid);
- if (res != 0) {
qb_util_perror(LOG_ERR, "qb_rb_chown:%s", rb_name);goto cleanup;- }
- res = qb_rb_chmod(ow->u.shm.rb, c->auth.mode);
- if (res != 0) {
qb_util_perror(LOG_ERR, "qb_rb_chmod:%s", rb_name);goto cleanup;- }
- return res;
+cleanup:
- qb_rb_close(ow->u.shm.rb);
- return res;
+}
+static int32_t qb_ipcs_shm_connect(struct qb_ipcs_service *s, struct qb_ipcs_connection *c, struct qb_ipc_connection_response *r) @@ -249,59 +283,27 @@ qb_ipcs_shm_connect(struct qb_ipcs_service *s, snprintf(r->event, NAME_MAX, "%s-event-%s", s->name, c->description);
- c->request.u.shm.rb = qb_rb_open(r->request,
c->request.max_msg_size,QB_RB_FLAG_CREATE |QB_RB_FLAG_SHARED_PROCESS,sizeof(int32_t));- if (c->request.u.shm.rb == NULL) {
res = -errno;qb_util_perror(LOG_ERR, "qb_rb_open:%s", r->request);goto cleanup;- }
- res = qb_rb_chown(c->request.u.shm.rb, c->euid, c->egid);
- res = qb_ipcs_shm_rb_open(c, &c->request,
r->request);
qb_util_perror(LOG_ERR, "qb_rb_chown:%s", r->request);goto cleanup; }
c->response.u.shm.rb = qb_rb_open(r->response,
c->response.max_msg_size,QB_RB_FLAG_CREATE |QB_RB_FLAG_SHARED_PROCESS, 0);if (c->response.u.shm.rb == NULL) {
res = -errno;qb_util_perror(LOG_ERR, "qb_rb_open:%s", r->response);goto cleanup_request;}
res = qb_rb_chown(c->response.u.shm.rb, c->euid, c->egid);
- res = qb_ipcs_shm_rb_open(c, &c->response,
r->response);
qb_util_perror(LOG_ERR, "qb_rb_chown:%s", r->response);goto cleanup_request; }
c->event.u.shm.rb = qb_rb_open(r->event,
c->event.max_msg_size,QB_RB_FLAG_CREATE |QB_RB_FLAG_SHARED_PROCESS, 0);if (c->event.u.shm.rb == NULL) {
res = -errno;qb_util_perror(LOG_ERR, "qb_rb_open:%s", r->event);goto cleanup_request_response;}
res = qb_rb_chown(c->event.u.shm.rb, c->euid, c->egid);
- res = qb_ipcs_shm_rb_open(c, &c->event,
r->event);
qb_util_perror(LOG_ERR, "qb_rb_chown:%s", r->event);goto cleanup_all;
goto cleanup_request_response;}
r->hdr.error = 0; return 0;
-cleanup_all:
- qb_rb_close(c->event.u.shm.rb);
cleanup_request_response: qb_rb_close(c->request.u.shm.rb);
diff --git a/lib/ipc_us.c b/lib/ipc_us.c index 2808708..73c4392 100644 --- a/lib/ipc_us.c +++ b/lib/ipc_us.c @@ -615,8 +615,9 @@ handle_new_connection(struct qb_ipcs_service *s, c->response.max_msg_size = req->max_msg_size; c->event.max_msg_size = req->max_msg_size; c->pid = ugp->pid;
- c->euid = ugp->uid;
- c->egid = ugp->gid;
- c->auth.uid = c->euid = ugp->uid;
- c->auth.gid = c->egid = ugp->gid;
- c->auth.mode = 0600; c->stats.client_pid = ugp->pid; snprintf(c->description, CONNECTION_DESCRIPTION, "%d-%d-%d", s->pid, ugp->pid,
diff --git a/lib/ipcs.c b/lib/ipcs.c index 8370c7d..68cb0a8 100644 --- a/lib/ipcs.c +++ b/lib/ipcs.c @@ -864,3 +864,14 @@ qb_ipcs_stats_get(struct qb_ipcs_service * s, } return 0; }
+void +qb_ipcs_connection_auth_set(qb_ipcs_connection_t *c, uid_t uid,
gid_t gid, mode_t mode)+{
- if (c) {
c->auth.uid = uid;c->auth.gid = gid;c->auth.mode = mode;- }
+} diff --git a/lib/ringbuffer.c b/lib/ringbuffer.c index 1daa0a9..7902dcd 100644 --- a/lib/ringbuffer.c +++ b/lib/ringbuffer.c @@ -769,3 +769,22 @@ qb_rb_chown(struct qb_ringbuffer_s * rb, uid_t owner, gid_t group) } return 0; }
+int32_t +qb_rb_chmod(qb_ringbuffer_t * rb, mode_t mode) +{
- int32_t res;
- if (rb == NULL) {
return -EINVAL;- }
- res = chmod(rb->shared_hdr->data_path, mode);
- if (res < 0) {
return -errno;- }
- res = chmod(rb->shared_hdr->hdr_path, mode);
- if (res < 0) {
return -errno;- }
- return 0;
+}
1.7.10.2
Hi Angus, Thanks a lot for introducing this! I also added the following patch, modified "examples/ipcserver.c", and it works for both QB_IPC_SHM and QB_IPC_SOCKET mode in the example.
I encountered weird behaviors for pacemaker cib though. Chmod works fine, but the group of file has never been changed. The only difference I can think of is that cib's uid "hacluster" comes from setuid() by root. But it still doesn't make sense to me that it's not allowed to change the group of a file to "hacluster"'s main group... Still checking...
diff --git a/lib/ipc_us.c b/lib/ipc_us.c index 2808708..8f11e46 100644 --- a/lib/ipc_us.c +++ b/lib/ipc_us.c @@ -949,12 +952,19 @@ qb_ipcs_us_connect(struct qb_ipcs_service *s, } (void)strlcpy(r->request, path, PATH_MAX); (void)strlcpy(c->request.u.us.shared_file_name, r->request, NAME_MAX); - res = chown(r->request, c->euid, c->egid); + res = chown(r->request, c->auth.uid, c->auth.gid); if (res != 0) { /* ignore res, this is just for the compiler warnings. */ res = 0; } + res = chmod(r->request, c->auth.mode); + if (res != 0) { + /* ignore res, this is just for the compiler warnings. + */ + res = 0; + } + shm_ptr = mmap(0, SHM_CONTROL_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd_hdr, 0);
Regards, Gao,Yan
On 06/12/12 19:56, Gao,Yan wrote:
Hi Angus, Thanks a lot for introducing this! I also added the following patch, modified "examples/ipcserver.c", and it works for both QB_IPC_SHM and QB_IPC_SOCKET mode in the example.
I encountered weird behaviors for pacemaker cib though. Chmod works fine, but the group of file has never been changed. The only difference I can think of is that cib's uid "hacluster" comes from setuid() by root. But it still doesn't make sense to me that it's not allowed to change the group of a file to "hacluster"'s main group...
So it is, "hacluster" got from setuid() by root cannot change the group of a file to "hacluster"'s main group -- "haclient", unless we also setgid to "haclient" before setuid to "hacluster", otherwise "root" must belong to "haclient" group. Another way is to change the file mode to 0666, and determine permissions in connection_accept().
Andrew, opinions?
Angus, the change works fine. Please push it. Thanks again!
Regards, Gao,Yan
diff --git a/lib/ipc_us.c b/lib/ipc_us.c index 2808708..8f11e46 100644 --- a/lib/ipc_us.c +++ b/lib/ipc_us.c @@ -949,12 +952,19 @@ qb_ipcs_us_connect(struct qb_ipcs_service *s, } (void)strlcpy(r->request, path, PATH_MAX); (void)strlcpy(c->request.u.us.shared_file_name, r->request, NAME_MAX);
- res = chown(r->request, c->euid, c->egid);
- res = chown(r->request, c->auth.uid, c->auth.gid); if (res != 0) { /* ignore res, this is just for the compiler warnings. */ res = 0; }
- res = chmod(r->request, c->auth.mode);
- if (res != 0) {
/* ignore res, this is just for the compiler warnings.*/res = 0;- }
- shm_ptr = mmap(0, SHM_CONTROL_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd_hdr, 0);
Regards, Gao,Yan
On 12/06/2012, at 10:45 PM, Gao,Yan wrote:
On 06/12/12 19:56, Gao,Yan wrote:
Hi Angus, Thanks a lot for introducing this! I also added the following patch, modified "examples/ipcserver.c", and it works for both QB_IPC_SHM and QB_IPC_SOCKET mode in the example.
I encountered weird behaviors for pacemaker cib though. Chmod works fine, but the group of file has never been changed. The only difference I can think of is that cib's uid "hacluster" comes from setuid() by root. But it still doesn't make sense to me that it's not allowed to change the group of a file to "hacluster"'s main group...
So it is, "hacluster" got from setuid() by root cannot change the group of a file to "hacluster"'s main group -- "haclient", unless we also setgid to "haclient" before setuid to "hacluster", otherwise "root" must belong to "haclient" group.
Really? That would surprise me greatly. How does chgrp manage this?
Another way is to change the file mode to 0666, and determine permissions in connection_accept().
Andrew, opinions?
Leaving the permissions wide open doesn't sound very appealing.
Angus, the change works fine. Please push it. Thanks again!
Regards, Gao,Yan
diff --git a/lib/ipc_us.c b/lib/ipc_us.c index 2808708..8f11e46 100644 --- a/lib/ipc_us.c +++ b/lib/ipc_us.c @@ -949,12 +952,19 @@ qb_ipcs_us_connect(struct qb_ipcs_service *s, } (void)strlcpy(r->request, path, PATH_MAX); (void)strlcpy(c->request.u.us.shared_file_name, r->request, NAME_MAX);
- res = chown(r->request, c->euid, c->egid);
- res = chown(r->request, c->auth.uid, c->auth.gid); if (res != 0) { /* ignore res, this is just for the compiler warnings. */ res = 0; }
- res = chmod(r->request, c->auth.mode);
- if (res != 0) {
/* ignore res, this is just for the compiler warnings.*/res = 0;- }
- shm_ptr = mmap(0, SHM_CONTROL_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd_hdr, 0);
Regards, Gao,Yan
-- Gao,Yan ygao@suse.com Software Engineer China Server Team, SUSE.
Hi Andrew,
On 06/13/12 07:18, Andrew Beekhof wrote:
On 12/06/2012, at 10:45 PM, Gao,Yan wrote:
On 06/12/12 19:56, Gao,Yan wrote:
Hi Angus, Thanks a lot for introducing this! I also added the following patch, modified "examples/ipcserver.c", and it works for both QB_IPC_SHM and QB_IPC_SOCKET mode in the example.
I encountered weird behaviors for pacemaker cib though. Chmod works fine, but the group of file has never been changed. The only difference I can think of is that cib's uid "hacluster" comes from setuid() by root. But it still doesn't make sense to me that it's not allowed to change the group of a file to "hacluster"'s main group...
So it is, "hacluster" got from setuid() by root cannot change the group of a file to "hacluster"'s main group -- "haclient", unless we also setgid to "haclient" before setuid to "hacluster", otherwise "root" must belong to "haclient" group.
Really? That would surprise me greatly.
Me too. :-\ It seems setuid() doesn't change any of the group information of the process unless setgid() _before_ that -- setgid() after setuid() is not allowed either.
How does chgrp manage this?
It invokes fchownat() system call. And it behaves in the same way as chown() for this.
Another way is to change the file mode to 0666, and determine permissions in connection_accept().
Andrew, opinions?
Leaving the permissions wide open doesn't sound very appealing.
Hmm, not very ideal indeed.
Regards, Gao,Yan
On 13/06/2012, at 11:34 AM, Gao,Yan wrote:
Hi Andrew,
On 06/13/12 07:18, Andrew Beekhof wrote:
On 12/06/2012, at 10:45 PM, Gao,Yan wrote:
On 06/12/12 19:56, Gao,Yan wrote:
Hi Angus, Thanks a lot for introducing this! I also added the following patch, modified "examples/ipcserver.c", and it works for both QB_IPC_SHM and QB_IPC_SOCKET mode in the example.
I encountered weird behaviors for pacemaker cib though. Chmod works fine, but the group of file has never been changed. The only difference I can think of is that cib's uid "hacluster" comes from setuid() by root. But it still doesn't make sense to me that it's not allowed to change the group of a file to "hacluster"'s main group...
So it is, "hacluster" got from setuid() by root cannot change the group of a file to "hacluster"'s main group -- "haclient", unless we also setgid to "haclient" before setuid to "hacluster", otherwise "root" must belong to "haclient" group.
Really? That would surprise me greatly.
Me too. :-\ It seems setuid() doesn't change any of the group information of the process unless setgid() _before_ that -- setgid() after setuid() is not allowed either.
I thought "unpriv client -> root server" and "root client -> unpriv server" both worked already. I might have fallen behind, could you restate the problem?
How does chgrp manage this?
It invokes fchownat() system call. And it behaves in the same way as chown() for this.
Can we not do this too?
Another way is to change the file mode to 0666, and determine permissions in connection_accept().
Andrew, opinions?
Leaving the permissions wide open doesn't sound very appealing.
Hmm, not very ideal indeed.
Regards, Gao,Yan -- Gao,Yan ygao@suse.com Software Engineer China Server Team, SUSE.
On 13/06/12 11:48 +1000, Andrew Beekhof wrote:
On 13/06/2012, at 11:34 AM, Gao,Yan wrote:
Hi Andrew,
On 06/13/12 07:18, Andrew Beekhof wrote:
On 12/06/2012, at 10:45 PM, Gao,Yan wrote:
On 06/12/12 19:56, Gao,Yan wrote:
Hi Angus, Thanks a lot for introducing this! I also added the following patch, modified "examples/ipcserver.c", and it works for both QB_IPC_SHM and QB_IPC_SOCKET mode in the example.
I encountered weird behaviors for pacemaker cib though. Chmod works fine, but the group of file has never been changed. The only difference I can think of is that cib's uid "hacluster" comes from setuid() by root. But it still doesn't make sense to me that it's not allowed to change the group of a file to "hacluster"'s main group...
So it is, "hacluster" got from setuid() by root cannot change the group of a file to "hacluster"'s main group -- "haclient", unless we also setgid to "haclient" before setuid to "hacluster", otherwise "root" must belong to "haclient" group.
Really? That would surprise me greatly.
Me too. :-\ It seems setuid() doesn't change any of the group information of the process unless setgid() _before_ that -- setgid() after setuid() is not allowed either.
I thought "unpriv client -> root server" and "root client -> unpriv server" both worked already. I might have fallen behind, could you restate the problem?
I think the issue here is the server is non-root-user:root-group And the client is non-root-user:non-root-group.
So server can't chown the file it created to be the uid of the client.
What we are trying to do now is: chgrp the file to a common group and chmod to 0660.
This works for me (for 2 different users with a common group). But Yan is struggling with the server as it gets the group id via setgid().
-Angus
How does chgrp manage this?
It invokes fchownat() system call. And it behaves in the same way as chown() for this.
Can we not do this too?
Another way is to change the file mode to 0666, and determine permissions in connection_accept().
Andrew, opinions?
Leaving the permissions wide open doesn't sound very appealing.
Hmm, not very ideal indeed.
Regards, Gao,Yan -- Gao,Yan ygao@suse.com Software Engineer China Server Team, SUSE.
quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
On 13/06/2012, at 12:05 PM, Angus Salkeld wrote:
On 13/06/12 11:48 +1000, Andrew Beekhof wrote:
On 13/06/2012, at 11:34 AM, Gao,Yan wrote:
Hi Andrew,
On 06/13/12 07:18, Andrew Beekhof wrote:
On 12/06/2012, at 10:45 PM, Gao,Yan wrote:
On 06/12/12 19:56, Gao,Yan wrote:
Hi Angus, Thanks a lot for introducing this! I also added the following patch, modified "examples/ipcserver.c", and it works for both QB_IPC_SHM and QB_IPC_SOCKET mode in the example.
I encountered weird behaviors for pacemaker cib though. Chmod works fine, but the group of file has never been changed. The only difference I can think of is that cib's uid "hacluster" comes from setuid() by root. But it still doesn't make sense to me that it's not allowed to change the group of a file to "hacluster"'s main group...
So it is, "hacluster" got from setuid() by root cannot change the group of a file to "hacluster"'s main group -- "haclient", unless we also setgid to "haclient" before setuid to "hacluster", otherwise "root" must belong to "haclient" group.
Really? That would surprise me greatly.
Me too. :-\ It seems setuid() doesn't change any of the group information of the process unless setgid() _before_ that -- setgid() after setuid() is not allowed either.
I thought "unpriv client -> root server" and "root client -> unpriv server" both worked already. I might have fallen behind, could you restate the problem?
I think the issue here is the server is non-root-user:root-group And the client is non-root-user:non-root-group.
So server can't chown the file it created to be the uid of the client.
What we are trying to do now is: chgrp the file to a common group and chmod to 0660.
Ok, 0660 i can handle :-)
This works for me (for 2 different users with a common group). But Yan is struggling with the server as it gets the group id via setgid().
The server will know the group name/id at startup, could we put it in a connection setting somewhere? We will always use haclient as the the common group.
-Angus
How does chgrp manage this?
It invokes fchownat() system call. And it behaves in the same way as chown() for this.
Can we not do this too?
Another way is to change the file mode to 0666, and determine permissions in connection_accept().
Andrew, opinions?
Leaving the permissions wide open doesn't sound very appealing.
Hmm, not very ideal indeed.
Regards, Gao,Yan -- Gao,Yan ygao@suse.com Software Engineer China Server Team, SUSE.
quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
On 06/13/12 10:15, Andrew Beekhof wrote:
On 13/06/2012, at 12:05 PM, Angus Salkeld wrote:
On 13/06/12 11:48 +1000, Andrew Beekhof wrote:
On 13/06/2012, at 11:34 AM, Gao,Yan wrote:
Hi Andrew,
On 06/13/12 07:18, Andrew Beekhof wrote:
On 12/06/2012, at 10:45 PM, Gao,Yan wrote:
On 06/12/12 19:56, Gao,Yan wrote: > Hi Angus, > Thanks a lot for introducing this! I also added the following patch, > modified "examples/ipcserver.c", and it works for both QB_IPC_SHM and > QB_IPC_SOCKET mode in the example. > > I encountered weird behaviors for pacemaker cib though. Chmod works > fine, but the group of file has never been changed. The only difference > I can think of is that cib's uid "hacluster" comes from setuid() by > root. But it still doesn't make sense to me that it's not allowed to > change the group of a file to "hacluster"'s main group... So it is, "hacluster" got from setuid() by root cannot change the group of a file to "hacluster"'s main group -- "haclient", unless we also setgid to "haclient" before setuid to "hacluster", otherwise "root" must belong to "haclient" group.
Really? That would surprise me greatly.
Me too. :-\ It seems setuid() doesn't change any of the group information of the process unless setgid() _before_ that -- setgid() after setuid() is not allowed either.
I thought "unpriv client -> root server" and "root client -> unpriv server" both worked already. I might have fallen behind, could you restate the problem?
I think the issue here is the server is non-root-user:root-group And the client is non-root-user:non-root-group.
So server can't chown the file it created to be the uid of the client.
What we are trying to do now is: chgrp the file to a common group and chmod to 0660.
Ok, 0660 i can handle :-)
This works for me (for 2 different users with a common group).
Works for me too, if they are in a common group, and if no one comes from setuid().
But Yan is struggling with the server as it gets the group id via setgid().
The server will know the group name/id at startup, could we put it in a connection setting somewhere? We will always use haclient as the the common group.
Yes. But the problem is if root is not in the "haclient" group also, after it setuid() to "hacluster", chown() the file to "haclient" will return EPERM.
Regards, Gao,Yan
-Angus
How does chgrp manage this?
It invokes fchownat() system call. And it behaves in the same way as chown() for this.
Can we not do this too?
Another way is to change the file mode to 0666, and determine permissions in connection_accept().
Andrew, opinions?
Leaving the permissions wide open doesn't sound very appealing.
Hmm, not very ideal indeed.
Regards, Gao,Yan -- Gao,Yan ygao@suse.com Software Engineer China Server Team, SUSE.
quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
On 06/13/12 10:40, Gao,Yan wrote:
On 06/13/12 10:15, Andrew Beekhof wrote:
On 13/06/2012, at 12:05 PM, Angus Salkeld wrote:
On 13/06/12 11:48 +1000, Andrew Beekhof wrote:
On 13/06/2012, at 11:34 AM, Gao,Yan wrote:
Hi Andrew,
On 06/13/12 07:18, Andrew Beekhof wrote:
On 12/06/2012, at 10:45 PM, Gao,Yan wrote:
> On 06/12/12 19:56, Gao,Yan wrote: >> Hi Angus, >> Thanks a lot for introducing this! I also added the following patch, >> modified "examples/ipcserver.c", and it works for both QB_IPC_SHM and >> QB_IPC_SOCKET mode in the example. >> >> I encountered weird behaviors for pacemaker cib though. Chmod works >> fine, but the group of file has never been changed. The only difference >> I can think of is that cib's uid "hacluster" comes from setuid() by >> root. But it still doesn't make sense to me that it's not allowed to >> change the group of a file to "hacluster"'s main group... > So it is, "hacluster" got from setuid() by root cannot change the group > of a file to "hacluster"'s main group -- "haclient", unless we also > setgid to "haclient" before setuid to "hacluster", otherwise "root" must > belong to "haclient" group.
Really? That would surprise me greatly.
Me too. :-\ It seems setuid() doesn't change any of the group information of the process unless setgid() _before_ that -- setgid() after setuid() is not allowed either.
I thought "unpriv client -> root server" and "root client -> unpriv server" both worked already. I might have fallen behind, could you restate the problem?
I think the issue here is the server is non-root-user:root-group And the client is non-root-user:non-root-group.
So server can't chown the file it created to be the uid of the client.
What we are trying to do now is: chgrp the file to a common group and chmod to 0660.
Ok, 0660 i can handle :-)
This works for me (for 2 different users with a common group).
Works for me too, if they are in a common group, and if no one comes from setuid().
But Yan is struggling with the server as it gets the group id via setgid().
The server will know the group name/id at startup, could we put it in a connection setting somewhere? We will always use haclient as the the common group.
Yes. But the problem is if root is not in the "haclient" group also, after it setuid() to "hacluster", chown() the file to "haclient" will return EPERM.
So there are three options in my mind: 1. Add root into "haclient" group on installation.
Or: 2. cib daemon setgid() to haclient before setuid() to hacluster
Or: 3. Mode 0666. Authenticate users in connection_accept().
Regards, Gao,Yan
How does chgrp manage this?
It invokes fchownat() system call. And it behaves in the same way as chown() for this.
Can we not do this too?
> Another way is to change the file mode to > 0666, and determine permissions in connection_accept(). > > Andrew, opinions?
Leaving the permissions wide open doesn't sound very appealing.
Hmm, not very ideal indeed.
Regards, Gao,Yan -- Gao,Yan ygao@suse.com Software Engineer China Server Team, SUSE.
quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
On 13/06/2012, at 12:54 PM, Gao,Yan wrote:
On 06/13/12 10:40, Gao,Yan wrote:
On 06/13/12 10:15, Andrew Beekhof wrote:
On 13/06/2012, at 12:05 PM, Angus Salkeld wrote:
On 13/06/12 11:48 +1000, Andrew Beekhof wrote:
On 13/06/2012, at 11:34 AM, Gao,Yan wrote:
Hi Andrew,
On 06/13/12 07:18, Andrew Beekhof wrote: > > On 12/06/2012, at 10:45 PM, Gao,Yan wrote: > >> On 06/12/12 19:56, Gao,Yan wrote: >>> Hi Angus, >>> Thanks a lot for introducing this! I also added the following patch, >>> modified "examples/ipcserver.c", and it works for both QB_IPC_SHM and >>> QB_IPC_SOCKET mode in the example. >>> >>> I encountered weird behaviors for pacemaker cib though. Chmod works >>> fine, but the group of file has never been changed. The only difference >>> I can think of is that cib's uid "hacluster" comes from setuid() by >>> root. But it still doesn't make sense to me that it's not allowed to >>> change the group of a file to "hacluster"'s main group... >> So it is, "hacluster" got from setuid() by root cannot change the group >> of a file to "hacluster"'s main group -- "haclient", unless we also >> setgid to "haclient" before setuid to "hacluster", otherwise "root" must >> belong to "haclient" group. > > Really? That would surprise me greatly. Me too. :-\ It seems setuid() doesn't change any of the group information of the process unless setgid() _before_ that -- setgid() after setuid() is not allowed either.
I thought "unpriv client -> root server" and "root client -> unpriv server" both worked already. I might have fallen behind, could you restate the problem?
I think the issue here is the server is non-root-user:root-group And the client is non-root-user:non-root-group.
So server can't chown the file it created to be the uid of the client.
What we are trying to do now is: chgrp the file to a common group and chmod to 0660.
Ok, 0660 i can handle :-)
This works for me (for 2 different users with a common group).
Works for me too, if they are in a common group, and if no one comes from setuid().
But Yan is struggling with the server as it gets the group id via setgid().
The server will know the group name/id at startup, could we put it in a connection setting somewhere? We will always use haclient as the the common group.
Yes. But the problem is if root is not in the "haclient" group also, after it setuid() to "hacluster", chown() the file to "haclient" will return EPERM.
So there are three options in my mind:
- Add root into "haclient" group on installation.
Or: 2. cib daemon setgid() to haclient before setuid() to hacluster
I like this best, but can we still connect to CPG?
Or: 3. Mode 0666. Authenticate users in connection_accept().
Regards, Gao,Yan
> How does chgrp manage this? It invokes fchownat() system call. And it behaves in the same way as chown() for this.
Can we not do this too?
> >> Another way is to change the file mode to >> 0666, and determine permissions in connection_accept(). >> >> Andrew, opinions? > > Leaving the permissions wide open doesn't sound very appealing. Hmm, not very ideal indeed.
Regards, Gao,Yan -- Gao,Yan ygao@suse.com Software Engineer China Server Team, SUSE.
quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
-- Gao,Yan ygao@suse.com Software Engineer China Server Team, SUSE. _______________________________________________ quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
On 06/13/12 10:56, Andrew Beekhof wrote:
On 13/06/2012, at 12:54 PM, Gao,Yan wrote:
On 06/13/12 10:40, Gao,Yan wrote:
On 06/13/12 10:15, Andrew Beekhof wrote:
On 13/06/2012, at 12:05 PM, Angus Salkeld wrote:
On 13/06/12 11:48 +1000, Andrew Beekhof wrote:
On 13/06/2012, at 11:34 AM, Gao,Yan wrote:
> Hi Andrew, > > On 06/13/12 07:18, Andrew Beekhof wrote: >> >> On 12/06/2012, at 10:45 PM, Gao,Yan wrote: >> >>> On 06/12/12 19:56, Gao,Yan wrote: >>>> Hi Angus, >>>> Thanks a lot for introducing this! I also added the following patch, >>>> modified "examples/ipcserver.c", and it works for both QB_IPC_SHM and >>>> QB_IPC_SOCKET mode in the example. >>>> >>>> I encountered weird behaviors for pacemaker cib though. Chmod works >>>> fine, but the group of file has never been changed. The only difference >>>> I can think of is that cib's uid "hacluster" comes from setuid() by >>>> root. But it still doesn't make sense to me that it's not allowed to >>>> change the group of a file to "hacluster"'s main group... >>> So it is, "hacluster" got from setuid() by root cannot change the group >>> of a file to "hacluster"'s main group -- "haclient", unless we also >>> setgid to "haclient" before setuid to "hacluster", otherwise "root" must >>> belong to "haclient" group. >> >> Really? That would surprise me greatly. > Me too. :-\ It seems setuid() doesn't change any of the group > information of the process unless setgid() _before_ that -- setgid() > after setuid() is not allowed either.
I thought "unpriv client -> root server" and "root client -> unpriv server" both worked already. I might have fallen behind, could you restate the problem?
I think the issue here is the server is non-root-user:root-group And the client is non-root-user:non-root-group.
So server can't chown the file it created to be the uid of the client.
What we are trying to do now is: chgrp the file to a common group and chmod to 0660.
Ok, 0660 i can handle :-)
This works for me (for 2 different users with a common group).
Works for me too, if they are in a common group, and if no one comes from setuid().
But Yan is struggling with the server as it gets the group id via setgid().
The server will know the group name/id at startup, could we put it in a connection setting somewhere? We will always use haclient as the the common group.
Yes. But the problem is if root is not in the "haclient" group also, after it setuid() to "hacluster", chown() the file to "haclient" will return EPERM.
So there are three options in my mind:
- Add root into "haclient" group on installation.
Or: 2. cib daemon setgid() to haclient before setuid() to hacluster
I like this best, but can we still connect to CPG?
Tried it a moment ago. Hmm, at least not work with corosync-1.4 based plugin:
Jun 13 11:41:18 yingying cib[24546]: info: init_ais_connection_classic: Connection to our AIS plugin (9) failed: unknown (100)
Just noticed the follow lines in "lib/ais/utils.c:165" and "mcp/pacemaker.c:306":
#if 0 /* Dont set the group for now - it prevents connection to the cluster */ if (gid && setgid(gid) < 0) { ais_perror("Could not set group to %d", gid); } #endif
Or: 3. Mode 0666. Authenticate users in connection_accept().
I was just talking to angus on the phone... can you send me the pacemaker patch you're testing? From what he describes, the existing libqb should be enough.
On 13/06/2012, at 12:54 PM, Gao,Yan wrote:
On 06/13/12 10:40, Gao,Yan wrote:
On 06/13/12 10:15, Andrew Beekhof wrote:
On 13/06/2012, at 12:05 PM, Angus Salkeld wrote:
On 13/06/12 11:48 +1000, Andrew Beekhof wrote:
On 13/06/2012, at 11:34 AM, Gao,Yan wrote:
Hi Andrew,
On 06/13/12 07:18, Andrew Beekhof wrote: > > On 12/06/2012, at 10:45 PM, Gao,Yan wrote: > >> On 06/12/12 19:56, Gao,Yan wrote: >>> Hi Angus, >>> Thanks a lot for introducing this! I also added the following patch, >>> modified "examples/ipcserver.c", and it works for both QB_IPC_SHM and >>> QB_IPC_SOCKET mode in the example. >>> >>> I encountered weird behaviors for pacemaker cib though. Chmod works >>> fine, but the group of file has never been changed. The only difference >>> I can think of is that cib's uid "hacluster" comes from setuid() by >>> root. But it still doesn't make sense to me that it's not allowed to >>> change the group of a file to "hacluster"'s main group... >> So it is, "hacluster" got from setuid() by root cannot change the group >> of a file to "hacluster"'s main group -- "haclient", unless we also >> setgid to "haclient" before setuid to "hacluster", otherwise "root" must >> belong to "haclient" group. > > Really? That would surprise me greatly. Me too. :-\ It seems setuid() doesn't change any of the group information of the process unless setgid() _before_ that -- setgid() after setuid() is not allowed either.
I thought "unpriv client -> root server" and "root client -> unpriv server" both worked already. I might have fallen behind, could you restate the problem?
I think the issue here is the server is non-root-user:root-group And the client is non-root-user:non-root-group.
So server can't chown the file it created to be the uid of the client.
What we are trying to do now is: chgrp the file to a common group and chmod to 0660.
Ok, 0660 i can handle :-)
This works for me (for 2 different users with a common group).
Works for me too, if they are in a common group, and if no one comes from setuid().
But Yan is struggling with the server as it gets the group id via setgid().
The server will know the group name/id at startup, could we put it in a connection setting somewhere? We will always use haclient as the the common group.
Yes. But the problem is if root is not in the "haclient" group also, after it setuid() to "hacluster", chown() the file to "haclient" will return EPERM.
So there are three options in my mind:
- Add root into "haclient" group on installation.
Or: 2. cib daemon setgid() to haclient before setuid() to hacluster
Or: 3. Mode 0666. Authenticate users in connection_accept().
Regards, Gao,Yan
> How does chgrp manage this? It invokes fchownat() system call. And it behaves in the same way as chown() for this.
Can we not do this too?
> >> Another way is to change the file mode to >> 0666, and determine permissions in connection_accept(). >> >> Andrew, opinions? > > Leaving the permissions wide open doesn't sound very appealing. Hmm, not very ideal indeed.
Regards, Gao,Yan -- Gao,Yan ygao@suse.com Software Engineer China Server Team, SUSE.
quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
-- Gao,Yan ygao@suse.com Software Engineer China Server Team, SUSE. _______________________________________________ quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
On 06/13/12 12:00, Andrew Beekhof wrote:
I was just talking to angus on the phone... can you send me the pacemaker patch you're testing? From what he describes, the existing libqb should be enough.
I believe libqb provides all it can do. The problem is setuid() won't get what needed. Attached the patch I'm testing.
Regards, Gao,Yan
On 13/06/2012, at 2:24 PM, Gao,Yan wrote:
On 06/13/12 12:00, Andrew Beekhof wrote:
I was just talking to angus on the phone... can you send me the pacemaker patch you're testing? From what he describes, the existing libqb should be enough.
I believe libqb provides all it can do. The problem is setuid() won't get what needed. Attached the patch I'm testing.
Where is the call to setuid()? in libqb somewhere? Where in the patch does it fail? Stacktrace?
Regards, Gao,Yan -- Gao,Yan ygao@suse.com Software Engineer China Server Team, SUSE. <pacemaker-acl-libqb.patch>_______________________________________________ quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
On 06/13/12 12:27, Andrew Beekhof wrote:
On 13/06/2012, at 2:24 PM, Gao,Yan wrote:
On 06/13/12 12:00, Andrew Beekhof wrote:
I was just talking to angus on the phone... can you send me the pacemaker patch you're testing? From what he describes, the existing libqb should be enough.
I believe libqb provides all it can do. The problem is setuid() won't get what needed. Attached the patch I'm testing.
Where is the call to setuid()? in libqb somewhere?
I mean cib, such as in "lib/ais/utils.c:177". Cib setuid() to hacluster from root.
Where in the patch does it fail? Stacktrace?
Regards, Gao,Yan
On 13/06/2012, at 2:30 PM, Gao,Yan wrote:
On 06/13/12 12:27, Andrew Beekhof wrote:
On 13/06/2012, at 2:24 PM, Gao,Yan wrote:
On 06/13/12 12:00, Andrew Beekhof wrote:
I was just talking to angus on the phone... can you send me the pacemaker patch you're testing? From what he describes, the existing libqb should be enough.
I believe libqb provides all it can do. The problem is setuid() won't get what needed. Attached the patch I'm testing.
Where is the call to setuid()? in libqb somewhere?
I mean cib, such as in "lib/ais/utils.c:177". Cib setuid() to hacluster from root.
Ok, I don't understand the question then... setuid() doesn't "get" anything. Perhaps if you clarify the error/behavior you're getting? I.e. what is failing and where.
Where in the patch does it fail? Stacktrace?
Regards, Gao,Yan -- Gao,Yan ygao@suse.com Software Engineer China Server Team, SUSE. _______________________________________________ quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
On 06/13/12 12:36, Andrew Beekhof wrote:
On 13/06/2012, at 2:30 PM, Gao,Yan wrote:
On 06/13/12 12:27, Andrew Beekhof wrote:
On 13/06/2012, at 2:24 PM, Gao,Yan wrote:
On 06/13/12 12:00, Andrew Beekhof wrote:
I was just talking to angus on the phone... can you send me the pacemaker patch you're testing? From what he describes, the existing libqb should be enough.
I believe libqb provides all it can do. The problem is setuid() won't get what needed. Attached the patch I'm testing.
Where is the call to setuid()? in libqb somewhere?
I mean cib, such as in "lib/ais/utils.c:177". Cib setuid() to hacluster from root.
Ok, I don't understand the question then... setuid() doesn't "get" anything. Perhaps if you clarify the error/behavior you're getting? I.e. what is failing and where.
/dev/shm/qb-cib_*-control* will be like:
-rw-rw---- 1 hacluster root 24 Jun 13 12:36 qb-cib_rw-control-31947-32166-15
If an ordinary user in haclient group requests to cib, he'll definitely get "permission denied":
open("/dev/shm/qb-cib_rw-control-31947-32166-15", O_RDWR) = -1 EACCES (Permission denied)
Which means the invoking cib/callbacks.c:99 qb_ipcs_connection_auth_set(c, -1, crm_grp->gr_gid, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
did change the file mode but not the group of the file.
It's due to the chown() in libqb/libqb/lib/ipc_us.c:953 res = chown(r->request, c->auth.uid, c->auth.gid);
getting "Operation not permitted (1)".
That means cib's "hacluster:root" role which comes from setuid() is not allowed change the group of file to "haclient".
Regards, Gao,Yan
On 13/06/2012, at 2:51 PM, Gao,Yan wrote:
On 06/13/12 12:36, Andrew Beekhof wrote:
On 13/06/2012, at 2:30 PM, Gao,Yan wrote:
On 06/13/12 12:27, Andrew Beekhof wrote:
On 13/06/2012, at 2:24 PM, Gao,Yan wrote:
On 06/13/12 12:00, Andrew Beekhof wrote:
I was just talking to angus on the phone... can you send me the pacemaker patch you're testing? From what he describes, the existing libqb should be enough.
I believe libqb provides all it can do. The problem is setuid() won't get what needed. Attached the patch I'm testing.
Where is the call to setuid()? in libqb somewhere?
I mean cib, such as in "lib/ais/utils.c:177". Cib setuid() to hacluster from root.
Ok, I don't understand the question then... setuid() doesn't "get" anything. Perhaps if you clarify the error/behavior you're getting? I.e. what is failing and where.
/dev/shm/qb-cib_*-control* will be like:
-rw-rw---- 1 hacluster root 24 Jun 13 12:36 qb-cib_rw-control-31947-32166-15
Isn't that the point of qb_ipcs_connection_auth_set() though, that the file has a group of "haclient" before the client tries to access that file?
If an ordinary user in haclient group requests to cib, he'll definitely get "permission denied":
open("/dev/shm/qb-cib_rw-control-31947-32166-15", O_RDWR) = -1 EACCES (Permission denied)
Which means the invoking cib/callbacks.c:99 qb_ipcs_connection_auth_set(c, -1, crm_grp->gr_gid, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
did change the file mode but not the group of the file.
It's due to the chown() in libqb/libqb/lib/ipc_us.c:953 res = chown(r->request, c->auth.uid, c->auth.gid);
getting "Operation not permitted (1)".
That means cib's "hacluster:root" role which comes from setuid() is not allowed change the group of file to "haclient".
But it should be:
-bash-4.2$ whoami hacluster -bash-4.2$ ls -al foo -rw-r--r-- 1 hacluster root 0 Jun 13 14:57 foo -bash-4.2$ chgrp haclient foo -bash-4.2$ ls -al foo -rw-r--r-- 1 hacluster haclient 0 Jun 13 14:57 foo
If you own a file, you can change the group. So what are we doing differently?
Regards, Gao,Yan -- Gao,Yan ygao@suse.com Software Engineer China Server Team, SUSE. _______________________________________________ quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
And just to tie it directly to what we're doing here:
[root@pcmk-1 ~]# su - hacluster -bash-4.2$ ls -al /dev/shm/qb-cib_shm-event-1067-1073-12-data -rw------- 1 hacluster root 524288 Jun 13 15:03 /dev/shm/qb-cib_shm-event-1067-1073-12-data -bash-4.2$ chgrp haclient /dev/shm/qb-cib_shm-event-1067-1073-12-data -bash-4.2$ ls -al /dev/shm/qb-cib_shm-event-1067-1073-12-data -rw------- 1 hacluster haclient 524288 Jun 13 15:03 /dev/shm/qb-cib_shm-event-1067-1073-12-data
So it should be possible to change the group.
On 13/06/2012, at 2:51 PM, Gao,Yan wrote:
On 06/13/12 12:36, Andrew Beekhof wrote:
On 13/06/2012, at 2:30 PM, Gao,Yan wrote:
On 06/13/12 12:27, Andrew Beekhof wrote:
On 13/06/2012, at 2:24 PM, Gao,Yan wrote:
On 06/13/12 12:00, Andrew Beekhof wrote:
I was just talking to angus on the phone... can you send me the pacemaker patch you're testing? From what he describes, the existing libqb should be enough.
I believe libqb provides all it can do. The problem is setuid() won't get what needed. Attached the patch I'm testing.
Where is the call to setuid()? in libqb somewhere?
I mean cib, such as in "lib/ais/utils.c:177". Cib setuid() to hacluster from root.
Ok, I don't understand the question then... setuid() doesn't "get" anything. Perhaps if you clarify the error/behavior you're getting? I.e. what is failing and where.
/dev/shm/qb-cib_*-control* will be like:
-rw-rw---- 1 hacluster root 24 Jun 13 12:36 qb-cib_rw-control-31947-32166-15
If an ordinary user in haclient group requests to cib, he'll definitely get "permission denied":
open("/dev/shm/qb-cib_rw-control-31947-32166-15", O_RDWR) = -1 EACCES (Permission denied)
Which means the invoking cib/callbacks.c:99 qb_ipcs_connection_auth_set(c, -1, crm_grp->gr_gid, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
did change the file mode but not the group of the file.
It's due to the chown() in libqb/libqb/lib/ipc_us.c:953 res = chown(r->request, c->auth.uid, c->auth.gid);
getting "Operation not permitted (1)".
That means cib's "hacluster:root" role which comes from setuid() is not allowed change the group of file to "haclient".
Regards, Gao,Yan -- Gao,Yan ygao@suse.com Software Engineer China Server Team, SUSE. _______________________________________________ quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
On 06/13/12 13:05, Andrew Beekhof wrote:
And just to tie it directly to what we're doing here:
[root@pcmk-1 ~]# su - hacluster -bash-4.2$ ls -al /dev/shm/qb-cib_shm-event-1067-1073-12-data -rw------- 1 hacluster root 524288 Jun 13 15:03 /dev/shm/qb-cib_shm-event-1067-1073-12-data -bash-4.2$ chgrp haclient /dev/shm/qb-cib_shm-event-1067-1073-12-data -bash-4.2$ ls -al /dev/shm/qb-cib_shm-event-1067-1073-12-data -rw------- 1 hacluster haclient 524288 Jun 13 15:03 /dev/shm/qb-cib_shm-event-1067-1073-12-data
Just checked "su". It does initgroups().
So it should be possible to change the group.
On 13/06/2012, at 2:51 PM, Gao,Yan wrote:
On 06/13/12 12:36, Andrew Beekhof wrote:
On 13/06/2012, at 2:30 PM, Gao,Yan wrote:
On 06/13/12 12:27, Andrew Beekhof wrote:
On 13/06/2012, at 2:24 PM, Gao,Yan wrote:
On 06/13/12 12:00, Andrew Beekhof wrote: > I was just talking to angus on the phone... can you send me the pacemaker patch you're testing? > From what he describes, the existing libqb should be enough. I believe libqb provides all it can do. The problem is setuid() won't get what needed. Attached the patch I'm testing.
Where is the call to setuid()? in libqb somewhere?
I mean cib, such as in "lib/ais/utils.c:177". Cib setuid() to hacluster from root.
Ok, I don't understand the question then... setuid() doesn't "get" anything. Perhaps if you clarify the error/behavior you're getting? I.e. what is failing and where.
/dev/shm/qb-cib_*-control* will be like:
-rw-rw---- 1 hacluster root 24 Jun 13 12:36 qb-cib_rw-control-31947-32166-15
If an ordinary user in haclient group requests to cib, he'll definitely get "permission denied":
open("/dev/shm/qb-cib_rw-control-31947-32166-15", O_RDWR) = -1 EACCES (Permission denied)
Which means the invoking cib/callbacks.c:99 qb_ipcs_connection_auth_set(c, -1, crm_grp->gr_gid, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
did change the file mode but not the group of the file.
It's due to the chown() in libqb/libqb/lib/ipc_us.c:953 res = chown(r->request, c->auth.uid, c->auth.gid);
getting "Operation not permitted (1)".
That means cib's "hacluster:root" role which comes from setuid() is not allowed change the group of file to "haclient".
Regards, Gao,Yan -- Gao,Yan ygao@suse.com Software Engineer China Server Team, SUSE. _______________________________________________ quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
quarterback-devel mailing list quarterback-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/quarterback-devel
On 13/06/2012, at 3:19 PM, Gao,Yan wrote:
On 06/13/12 13:05, Andrew Beekhof wrote:
And just to tie it directly to what we're doing here:
[root@pcmk-1 ~]# su - hacluster -bash-4.2$ ls -al /dev/shm/qb-cib_shm-event-1067-1073-12-data -rw------- 1 hacluster root 524288 Jun 13 15:03 /dev/shm/qb-cib_shm-event-1067-1073-12-data -bash-4.2$ chgrp haclient /dev/shm/qb-cib_shm-event-1067-1073-12-data -bash-4.2$ ls -al /dev/shm/qb-cib_shm-event-1067-1073-12-data -rw------- 1 hacluster haclient 524288 Jun 13 15:03 /dev/shm/qb-cib_shm-event-1067-1073-12-data
Just checked "su". It does initgroups().
Sounds promising... what if we have the plugin's child processes call that too?
On 06/13/12 13:21, Andrew Beekhof wrote:
On 13/06/2012, at 3:19 PM, Gao,Yan wrote:
On 06/13/12 13:05, Andrew Beekhof wrote:
And just to tie it directly to what we're doing here:
[root@pcmk-1 ~]# su - hacluster -bash-4.2$ ls -al /dev/shm/qb-cib_shm-event-1067-1073-12-data -rw------- 1 hacluster root 524288 Jun 13 15:03 /dev/shm/qb-cib_shm-event-1067-1073-12-data -bash-4.2$ chgrp haclient /dev/shm/qb-cib_shm-event-1067-1073-12-data -bash-4.2$ ls -al /dev/shm/qb-cib_shm-event-1067-1073-12-data -rw------- 1 hacluster haclient 524288 Jun 13 15:03 /dev/shm/qb-cib_shm-event-1067-1073-12-data
Just checked "su". It does initgroups().
Sounds promising... what if we have the plugin's child processes call that too?
Makes sense. I'll do that for it.
Regards, Gao,Yan
quarterback-devel@lists.fedorahosted.org
-
Andrew Beekhof -
Angus Salkeld -
Gao,Yan