* [PATCH 0/3] libceph: #8979 fix (wip-auth-8979)
@ 2014-09-10 7:54 Ilya Dryomov
2014-09-10 7:54 ` [PATCH 1/3] libceph: gracefully handle large reply messages from the mon Ilya Dryomov
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Ilya Dryomov @ 2014-09-10 7:54 UTC (permalink / raw)
To: ceph-devel
Hello,
This is a fix for #8979 ("GPF kernel panics - auth?"), based on Sage's
wip-8979 branch.
Thanks,
Ilya
Ilya Dryomov (2):
libceph: add process_one_ticket() helper
libceph: do not hard code max auth ticket len
Sage Weil (1):
libceph: gracefully handle large reply messages from the mon
net/ceph/auth_x.c | 256 ++++++++++++++++++++++++++-----------------------
net/ceph/mon_client.c | 8 ++
2 files changed, 143 insertions(+), 121 deletions(-)
--
1.7.10.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] libceph: gracefully handle large reply messages from the mon
2014-09-10 7:54 [PATCH 0/3] libceph: #8979 fix (wip-auth-8979) Ilya Dryomov
@ 2014-09-10 7:54 ` Ilya Dryomov
2014-09-10 12:48 ` Sage Weil
2014-09-10 7:54 ` [PATCH 2/3] libceph: add process_one_ticket() helper Ilya Dryomov
2014-09-10 7:54 ` [PATCH 3/3] libceph: do not hard code max auth ticket len Ilya Dryomov
2 siblings, 1 reply; 7+ messages in thread
From: Ilya Dryomov @ 2014-09-10 7:54 UTC (permalink / raw)
To: ceph-devel; +Cc: Sage Weil
From: Sage Weil <sage@redhat.com>
We preallocate a few of the message types we get back from the mon. If we
get a larger message than we are expecting, fall back to trying to allocate
a new one instead of blindly using the one we have.
CC: stable@vger.kernel.org
Signed-off-by: Sage Weil <sage@redhat.com>
---
net/ceph/mon_client.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/ceph/mon_client.c b/net/ceph/mon_client.c
index 067d3af2eaf6..61fcfc304f68 100644
--- a/net/ceph/mon_client.c
+++ b/net/ceph/mon_client.c
@@ -1181,7 +1181,15 @@ static struct ceph_msg *mon_alloc_msg(struct ceph_connection *con,
if (!m) {
pr_info("alloc_msg unknown type %d\n", type);
*skip = 1;
+ } else if (front_len > m->front_alloc_len) {
+ pr_warning("mon_alloc_msg front %d > prealloc %d (%u#%llu)\n",
+ front_len, m->front_alloc_len,
+ (unsigned int)con->peer_name.type,
+ le64_to_cpu(con->peer_name.num));
+ ceph_msg_put(m);
+ m = ceph_msg_new(type, front_len, GFP_NOFS, false);
}
+
return m;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] libceph: add process_one_ticket() helper
2014-09-10 7:54 [PATCH 0/3] libceph: #8979 fix (wip-auth-8979) Ilya Dryomov
2014-09-10 7:54 ` [PATCH 1/3] libceph: gracefully handle large reply messages from the mon Ilya Dryomov
@ 2014-09-10 7:54 ` Ilya Dryomov
2014-09-10 12:52 ` Sage Weil
2014-09-10 7:54 ` [PATCH 3/3] libceph: do not hard code max auth ticket len Ilya Dryomov
2 siblings, 1 reply; 7+ messages in thread
From: Ilya Dryomov @ 2014-09-10 7:54 UTC (permalink / raw)
To: ceph-devel
Add a helper for processing individual cephx auth tickets. Needed for
the next commit, that deals with allocating ticket buffers. (Most of
the diff here is whitespace - view with git diff -b).
Cc: stable@vger.kernel.org
Signed-off-by: Ilya Dryomov <ilya.dryomov@inktank.com>
---
net/ceph/auth_x.c | 228 +++++++++++++++++++++++++++++------------------------
1 file changed, 124 insertions(+), 104 deletions(-)
diff --git a/net/ceph/auth_x.c b/net/ceph/auth_x.c
index 96238ba95f2b..0eb146dce1aa 100644
--- a/net/ceph/auth_x.c
+++ b/net/ceph/auth_x.c
@@ -129,17 +129,131 @@ static void remove_ticket_handler(struct ceph_auth_client *ac,
kfree(th);
}
+static int process_one_ticket(struct ceph_auth_client *ac,
+ struct ceph_crypto_key *secret,
+ void **p, void *end,
+ void *dbuf, void *ticket_buf)
+{
+ struct ceph_x_info *xi = ac->private;
+ int type;
+ u8 tkt_struct_v, blob_struct_v;
+ struct ceph_x_ticket_handler *th;
+ void *dp, *dend;
+ int dlen;
+ char is_enc;
+ struct timespec validity;
+ struct ceph_crypto_key old_key;
+ void *tp, *tpend;
+ struct ceph_timespec new_validity;
+ struct ceph_crypto_key new_session_key;
+ struct ceph_buffer *new_ticket_blob;
+ unsigned long new_expires, new_renew_after;
+ u64 new_secret_id;
+ int ret;
+
+ ceph_decode_need(p, end, sizeof(u32) + 1, bad);
+
+ type = ceph_decode_32(p);
+ dout(" ticket type %d %s\n", type, ceph_entity_type_name(type));
+
+ tkt_struct_v = ceph_decode_8(p);
+ if (tkt_struct_v != 1)
+ goto bad;
+
+ th = get_ticket_handler(ac, type);
+ if (IS_ERR(th)) {
+ ret = PTR_ERR(th);
+ goto out;
+ }
+
+ /* blob for me */
+ dlen = ceph_x_decrypt(secret, p, end, dbuf,
+ TEMP_TICKET_BUF_LEN);
+ if (dlen <= 0) {
+ ret = dlen;
+ goto out;
+ }
+ dout(" decrypted %d bytes\n", dlen);
+ dp = dbuf;
+ dend = dp + dlen;
+
+ tkt_struct_v = ceph_decode_8(&dp);
+ if (tkt_struct_v != 1)
+ goto bad;
+
+ memcpy(&old_key, &th->session_key, sizeof(old_key));
+ ret = ceph_crypto_key_decode(&new_session_key, &dp, dend);
+ if (ret)
+ goto out;
+
+ ceph_decode_copy(&dp, &new_validity, sizeof(new_validity));
+ ceph_decode_timespec(&validity, &new_validity);
+ new_expires = get_seconds() + validity.tv_sec;
+ new_renew_after = new_expires - (validity.tv_sec / 4);
+ dout(" expires=%lu renew_after=%lu\n", new_expires,
+ new_renew_after);
+
+ /* ticket blob for service */
+ ceph_decode_8_safe(p, end, is_enc, bad);
+ tp = ticket_buf;
+ if (is_enc) {
+ /* encrypted */
+ dout(" encrypted ticket\n");
+ dlen = ceph_x_decrypt(&old_key, p, end, ticket_buf,
+ TEMP_TICKET_BUF_LEN);
+ if (dlen < 0) {
+ ret = dlen;
+ goto out;
+ }
+ dlen = ceph_decode_32(&tp);
+ } else {
+ /* unencrypted */
+ ceph_decode_32_safe(p, end, dlen, bad);
+ ceph_decode_need(p, end, dlen, bad);
+ ceph_decode_copy(p, ticket_buf, dlen);
+ }
+ tpend = tp + dlen;
+ dout(" ticket blob is %d bytes\n", dlen);
+ ceph_decode_need(&tp, tpend, 1 + sizeof(u64), bad);
+ blob_struct_v = ceph_decode_8(&tp);
+ new_secret_id = ceph_decode_64(&tp);
+ ret = ceph_decode_buffer(&new_ticket_blob, &tp, tpend);
+ if (ret)
+ goto out;
+
+ /* all is well, update our ticket */
+ ceph_crypto_key_destroy(&th->session_key);
+ if (th->ticket_blob)
+ ceph_buffer_put(th->ticket_blob);
+ th->session_key = new_session_key;
+ th->ticket_blob = new_ticket_blob;
+ th->validity = new_validity;
+ th->secret_id = new_secret_id;
+ th->expires = new_expires;
+ th->renew_after = new_renew_after;
+ dout(" got ticket service %d (%s) secret_id %lld len %d\n",
+ type, ceph_entity_type_name(type), th->secret_id,
+ (int)th->ticket_blob->vec.iov_len);
+ xi->have_keys |= th->service;
+
+out:
+ return ret;
+
+bad:
+ ret = -EINVAL;
+ goto out;
+}
+
static int ceph_x_proc_ticket_reply(struct ceph_auth_client *ac,
struct ceph_crypto_key *secret,
void *buf, void *end)
{
- struct ceph_x_info *xi = ac->private;
- int num;
void *p = buf;
- int ret;
char *dbuf;
char *ticket_buf;
u8 reply_struct_v;
+ u32 num;
+ int ret;
dbuf = kmalloc(TEMP_TICKET_BUF_LEN, GFP_NOFS);
if (!dbuf)
@@ -150,112 +264,18 @@ static int ceph_x_proc_ticket_reply(struct ceph_auth_client *ac,
if (!ticket_buf)
goto out_dbuf;
- ceph_decode_need(&p, end, 1 + sizeof(u32), bad);
- reply_struct_v = ceph_decode_8(&p);
+ ceph_decode_8_safe(&p, end, reply_struct_v, bad);
if (reply_struct_v != 1)
- goto bad;
- num = ceph_decode_32(&p);
- dout("%d tickets\n", num);
- while (num--) {
- int type;
- u8 tkt_struct_v, blob_struct_v;
- struct ceph_x_ticket_handler *th;
- void *dp, *dend;
- int dlen;
- char is_enc;
- struct timespec validity;
- struct ceph_crypto_key old_key;
- void *tp, *tpend;
- struct ceph_timespec new_validity;
- struct ceph_crypto_key new_session_key;
- struct ceph_buffer *new_ticket_blob;
- unsigned long new_expires, new_renew_after;
- u64 new_secret_id;
-
- ceph_decode_need(&p, end, sizeof(u32) + 1, bad);
-
- type = ceph_decode_32(&p);
- dout(" ticket type %d %s\n", type, ceph_entity_type_name(type));
-
- tkt_struct_v = ceph_decode_8(&p);
- if (tkt_struct_v != 1)
- goto bad;
-
- th = get_ticket_handler(ac, type);
- if (IS_ERR(th)) {
- ret = PTR_ERR(th);
- goto out;
- }
-
- /* blob for me */
- dlen = ceph_x_decrypt(secret, &p, end, dbuf,
- TEMP_TICKET_BUF_LEN);
- if (dlen <= 0) {
- ret = dlen;
- goto out;
- }
- dout(" decrypted %d bytes\n", dlen);
- dend = dbuf + dlen;
- dp = dbuf;
-
- tkt_struct_v = ceph_decode_8(&dp);
- if (tkt_struct_v != 1)
- goto bad;
+ return -EINVAL;
- memcpy(&old_key, &th->session_key, sizeof(old_key));
- ret = ceph_crypto_key_decode(&new_session_key, &dp, dend);
- if (ret)
- goto out;
+ ceph_decode_32_safe(&p, end, num, bad);
+ dout("%d tickets\n", num);
- ceph_decode_copy(&dp, &new_validity, sizeof(new_validity));
- ceph_decode_timespec(&validity, &new_validity);
- new_expires = get_seconds() + validity.tv_sec;
- new_renew_after = new_expires - (validity.tv_sec / 4);
- dout(" expires=%lu renew_after=%lu\n", new_expires,
- new_renew_after);
-
- /* ticket blob for service */
- ceph_decode_8_safe(&p, end, is_enc, bad);
- tp = ticket_buf;
- if (is_enc) {
- /* encrypted */
- dout(" encrypted ticket\n");
- dlen = ceph_x_decrypt(&old_key, &p, end, ticket_buf,
- TEMP_TICKET_BUF_LEN);
- if (dlen < 0) {
- ret = dlen;
- goto out;
- }
- dlen = ceph_decode_32(&tp);
- } else {
- /* unencrypted */
- ceph_decode_32_safe(&p, end, dlen, bad);
- ceph_decode_need(&p, end, dlen, bad);
- ceph_decode_copy(&p, ticket_buf, dlen);
- }
- tpend = tp + dlen;
- dout(" ticket blob is %d bytes\n", dlen);
- ceph_decode_need(&tp, tpend, 1 + sizeof(u64), bad);
- blob_struct_v = ceph_decode_8(&tp);
- new_secret_id = ceph_decode_64(&tp);
- ret = ceph_decode_buffer(&new_ticket_blob, &tp, tpend);
+ while (num--) {
+ ret = process_one_ticket(ac, secret, &p, end,
+ dbuf, ticket_buf);
if (ret)
goto out;
-
- /* all is well, update our ticket */
- ceph_crypto_key_destroy(&th->session_key);
- if (th->ticket_blob)
- ceph_buffer_put(th->ticket_blob);
- th->session_key = new_session_key;
- th->ticket_blob = new_ticket_blob;
- th->validity = new_validity;
- th->secret_id = new_secret_id;
- th->expires = new_expires;
- th->renew_after = new_renew_after;
- dout(" got ticket service %d (%s) secret_id %lld len %d\n",
- type, ceph_entity_type_name(type), th->secret_id,
- (int)th->ticket_blob->vec.iov_len);
- xi->have_keys |= th->service;
}
ret = 0;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] libceph: do not hard code max auth ticket len
2014-09-10 7:54 [PATCH 0/3] libceph: #8979 fix (wip-auth-8979) Ilya Dryomov
2014-09-10 7:54 ` [PATCH 1/3] libceph: gracefully handle large reply messages from the mon Ilya Dryomov
2014-09-10 7:54 ` [PATCH 2/3] libceph: add process_one_ticket() helper Ilya Dryomov
@ 2014-09-10 7:54 ` Ilya Dryomov
2014-09-10 12:55 ` Sage Weil
2 siblings, 1 reply; 7+ messages in thread
From: Ilya Dryomov @ 2014-09-10 7:54 UTC (permalink / raw)
To: ceph-devel
We hard code cephx auth ticket buffer size to 256 bytes. This isn't
enough for any moderate setups and, in case tickets themselves are not
encrypted, leads to buffer overflows (ceph_x_decrypt() errors out, but
ceph_decode_copy() doesn't - it's just a memcpy() wrapper). Since the
buffer is allocated dynamically anyway, allocated it a bit later, at
the point where we know how much is going to be needed.
Fixes: http://tracker.ceph.com/issues/8979
Cc: stable@vger.kernel.org
Signed-off-by: Ilya Dryomov <ilya.dryomov@inktank.com>
---
net/ceph/auth_x.c | 64 ++++++++++++++++++++++++-----------------------------
1 file changed, 29 insertions(+), 35 deletions(-)
diff --git a/net/ceph/auth_x.c b/net/ceph/auth_x.c
index 0eb146dce1aa..de6662b14e1f 100644
--- a/net/ceph/auth_x.c
+++ b/net/ceph/auth_x.c
@@ -13,8 +13,6 @@
#include "auth_x.h"
#include "auth_x_protocol.h"
-#define TEMP_TICKET_BUF_LEN 256
-
static void ceph_x_validate_tickets(struct ceph_auth_client *ac, int *pneed);
static int ceph_x_is_authenticated(struct ceph_auth_client *ac)
@@ -64,7 +62,7 @@ static int ceph_x_encrypt(struct ceph_crypto_key *secret,
}
static int ceph_x_decrypt(struct ceph_crypto_key *secret,
- void **p, void *end, void *obuf, size_t olen)
+ void **p, void *end, void **obuf, size_t olen)
{
struct ceph_x_encrypt_header head;
size_t head_len = sizeof(head);
@@ -75,8 +73,14 @@ static int ceph_x_decrypt(struct ceph_crypto_key *secret,
return -EINVAL;
dout("ceph_x_decrypt len %d\n", len);
- ret = ceph_decrypt2(secret, &head, &head_len, obuf, &olen,
- *p, len);
+ if (*obuf == NULL) {
+ *obuf = kmalloc(len, GFP_NOFS);
+ if (!*obuf)
+ return -ENOMEM;
+ olen = len;
+ }
+
+ ret = ceph_decrypt2(secret, &head, &head_len, *obuf, &olen, *p, len);
if (ret)
return ret;
if (head.struct_v != 1 || le64_to_cpu(head.magic) != CEPHX_ENC_MAGIC)
@@ -131,18 +135,19 @@ static void remove_ticket_handler(struct ceph_auth_client *ac,
static int process_one_ticket(struct ceph_auth_client *ac,
struct ceph_crypto_key *secret,
- void **p, void *end,
- void *dbuf, void *ticket_buf)
+ void **p, void *end)
{
struct ceph_x_info *xi = ac->private;
int type;
u8 tkt_struct_v, blob_struct_v;
struct ceph_x_ticket_handler *th;
+ void *dbuf = NULL;
void *dp, *dend;
int dlen;
char is_enc;
struct timespec validity;
struct ceph_crypto_key old_key;
+ void *ticket_buf = NULL;
void *tp, *tpend;
struct ceph_timespec new_validity;
struct ceph_crypto_key new_session_key;
@@ -167,8 +172,7 @@ static int process_one_ticket(struct ceph_auth_client *ac,
}
/* blob for me */
- dlen = ceph_x_decrypt(secret, p, end, dbuf,
- TEMP_TICKET_BUF_LEN);
+ dlen = ceph_x_decrypt(secret, p, end, &dbuf, 0);
if (dlen <= 0) {
ret = dlen;
goto out;
@@ -195,20 +199,25 @@ static int process_one_ticket(struct ceph_auth_client *ac,
/* ticket blob for service */
ceph_decode_8_safe(p, end, is_enc, bad);
- tp = ticket_buf;
if (is_enc) {
/* encrypted */
dout(" encrypted ticket\n");
- dlen = ceph_x_decrypt(&old_key, p, end, ticket_buf,
- TEMP_TICKET_BUF_LEN);
+ dlen = ceph_x_decrypt(&old_key, p, end, &ticket_buf, 0);
if (dlen < 0) {
ret = dlen;
goto out;
}
+ tp = ticket_buf;
dlen = ceph_decode_32(&tp);
} else {
/* unencrypted */
ceph_decode_32_safe(p, end, dlen, bad);
+ ticket_buf = kmalloc(dlen, GFP_NOFS);
+ if (!ticket_buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ tp = ticket_buf;
ceph_decode_need(p, end, dlen, bad);
ceph_decode_copy(p, ticket_buf, dlen);
}
@@ -237,6 +246,8 @@ static int process_one_ticket(struct ceph_auth_client *ac,
xi->have_keys |= th->service;
out:
+ kfree(ticket_buf);
+ kfree(dbuf);
return ret;
bad:
@@ -249,21 +260,10 @@ static int ceph_x_proc_ticket_reply(struct ceph_auth_client *ac,
void *buf, void *end)
{
void *p = buf;
- char *dbuf;
- char *ticket_buf;
u8 reply_struct_v;
u32 num;
int ret;
- dbuf = kmalloc(TEMP_TICKET_BUF_LEN, GFP_NOFS);
- if (!dbuf)
- return -ENOMEM;
-
- ret = -ENOMEM;
- ticket_buf = kmalloc(TEMP_TICKET_BUF_LEN, GFP_NOFS);
- if (!ticket_buf)
- goto out_dbuf;
-
ceph_decode_8_safe(&p, end, reply_struct_v, bad);
if (reply_struct_v != 1)
return -EINVAL;
@@ -272,22 +272,15 @@ static int ceph_x_proc_ticket_reply(struct ceph_auth_client *ac,
dout("%d tickets\n", num);
while (num--) {
- ret = process_one_ticket(ac, secret, &p, end,
- dbuf, ticket_buf);
+ ret = process_one_ticket(ac, secret, &p, end);
if (ret)
- goto out;
+ return ret;
}
- ret = 0;
-out:
- kfree(ticket_buf);
-out_dbuf:
- kfree(dbuf);
- return ret;
+ return 0;
bad:
- ret = -EINVAL;
- goto out;
+ return -EINVAL;
}
static int ceph_x_build_authorizer(struct ceph_auth_client *ac,
@@ -603,13 +596,14 @@ static int ceph_x_verify_authorizer_reply(struct ceph_auth_client *ac,
struct ceph_x_ticket_handler *th;
int ret = 0;
struct ceph_x_authorize_reply reply;
+ void *preply = &reply;
void *p = au->reply_buf;
void *end = p + sizeof(au->reply_buf);
th = get_ticket_handler(ac, au->service);
if (IS_ERR(th))
return PTR_ERR(th);
- ret = ceph_x_decrypt(&th->session_key, &p, end, &reply, sizeof(reply));
+ ret = ceph_x_decrypt(&th->session_key, &p, end, &preply, sizeof(reply));
if (ret < 0)
return ret;
if (ret != sizeof(reply))
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] libceph: gracefully handle large reply messages from the mon
2014-09-10 7:54 ` [PATCH 1/3] libceph: gracefully handle large reply messages from the mon Ilya Dryomov
@ 2014-09-10 12:48 ` Sage Weil
0 siblings, 0 replies; 7+ messages in thread
From: Sage Weil @ 2014-09-10 12:48 UTC (permalink / raw)
To: Ilya Dryomov; +Cc: ceph-devel
Reviewed-by:
On Wed, 10 Sep 2014, Ilya Dryomov wrote:
> From: Sage Weil <sage@redhat.com>
>
> We preallocate a few of the message types we get back from the mon. If we
> get a larger message than we are expecting, fall back to trying to allocate
> a new one instead of blindly using the one we have.
>
> CC: stable@vger.kernel.org
> Signed-off-by: Sage Weil <sage@redhat.com>
> ---
> net/ceph/mon_client.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/net/ceph/mon_client.c b/net/ceph/mon_client.c
> index 067d3af2eaf6..61fcfc304f68 100644
> --- a/net/ceph/mon_client.c
> +++ b/net/ceph/mon_client.c
> @@ -1181,7 +1181,15 @@ static struct ceph_msg *mon_alloc_msg(struct ceph_connection *con,
> if (!m) {
> pr_info("alloc_msg unknown type %d\n", type);
> *skip = 1;
> + } else if (front_len > m->front_alloc_len) {
> + pr_warning("mon_alloc_msg front %d > prealloc %d (%u#%llu)\n",
> + front_len, m->front_alloc_len,
> + (unsigned int)con->peer_name.type,
> + le64_to_cpu(con->peer_name.num));
> + ceph_msg_put(m);
> + m = ceph_msg_new(type, front_len, GFP_NOFS, false);
> }
> +
> return m;
> }
>
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] libceph: add process_one_ticket() helper
2014-09-10 7:54 ` [PATCH 2/3] libceph: add process_one_ticket() helper Ilya Dryomov
@ 2014-09-10 12:52 ` Sage Weil
0 siblings, 0 replies; 7+ messages in thread
From: Sage Weil @ 2014-09-10 12:52 UTC (permalink / raw)
To: Ilya Dryomov; +Cc: ceph-devel
Reviewed-by:
On Wed, 10 Sep 2014, Ilya Dryomov wrote:
> Add a helper for processing individual cephx auth tickets. Needed for
> the next commit, that deals with allocating ticket buffers. (Most of
> the diff here is whitespace - view with git diff -b).
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Ilya Dryomov <ilya.dryomov@inktank.com>
> ---
> net/ceph/auth_x.c | 228 +++++++++++++++++++++++++++++------------------------
> 1 file changed, 124 insertions(+), 104 deletions(-)
>
> diff --git a/net/ceph/auth_x.c b/net/ceph/auth_x.c
> index 96238ba95f2b..0eb146dce1aa 100644
> --- a/net/ceph/auth_x.c
> +++ b/net/ceph/auth_x.c
> @@ -129,17 +129,131 @@ static void remove_ticket_handler(struct ceph_auth_client *ac,
> kfree(th);
> }
>
> +static int process_one_ticket(struct ceph_auth_client *ac,
> + struct ceph_crypto_key *secret,
> + void **p, void *end,
> + void *dbuf, void *ticket_buf)
> +{
> + struct ceph_x_info *xi = ac->private;
> + int type;
> + u8 tkt_struct_v, blob_struct_v;
> + struct ceph_x_ticket_handler *th;
> + void *dp, *dend;
> + int dlen;
> + char is_enc;
> + struct timespec validity;
> + struct ceph_crypto_key old_key;
> + void *tp, *tpend;
> + struct ceph_timespec new_validity;
> + struct ceph_crypto_key new_session_key;
> + struct ceph_buffer *new_ticket_blob;
> + unsigned long new_expires, new_renew_after;
> + u64 new_secret_id;
> + int ret;
> +
> + ceph_decode_need(p, end, sizeof(u32) + 1, bad);
> +
> + type = ceph_decode_32(p);
> + dout(" ticket type %d %s\n", type, ceph_entity_type_name(type));
> +
> + tkt_struct_v = ceph_decode_8(p);
> + if (tkt_struct_v != 1)
> + goto bad;
> +
> + th = get_ticket_handler(ac, type);
> + if (IS_ERR(th)) {
> + ret = PTR_ERR(th);
> + goto out;
> + }
> +
> + /* blob for me */
> + dlen = ceph_x_decrypt(secret, p, end, dbuf,
> + TEMP_TICKET_BUF_LEN);
> + if (dlen <= 0) {
> + ret = dlen;
> + goto out;
> + }
> + dout(" decrypted %d bytes\n", dlen);
> + dp = dbuf;
> + dend = dp + dlen;
> +
> + tkt_struct_v = ceph_decode_8(&dp);
> + if (tkt_struct_v != 1)
> + goto bad;
> +
> + memcpy(&old_key, &th->session_key, sizeof(old_key));
> + ret = ceph_crypto_key_decode(&new_session_key, &dp, dend);
> + if (ret)
> + goto out;
> +
> + ceph_decode_copy(&dp, &new_validity, sizeof(new_validity));
> + ceph_decode_timespec(&validity, &new_validity);
> + new_expires = get_seconds() + validity.tv_sec;
> + new_renew_after = new_expires - (validity.tv_sec / 4);
> + dout(" expires=%lu renew_after=%lu\n", new_expires,
> + new_renew_after);
> +
> + /* ticket blob for service */
> + ceph_decode_8_safe(p, end, is_enc, bad);
> + tp = ticket_buf;
> + if (is_enc) {
> + /* encrypted */
> + dout(" encrypted ticket\n");
> + dlen = ceph_x_decrypt(&old_key, p, end, ticket_buf,
> + TEMP_TICKET_BUF_LEN);
> + if (dlen < 0) {
> + ret = dlen;
> + goto out;
> + }
> + dlen = ceph_decode_32(&tp);
> + } else {
> + /* unencrypted */
> + ceph_decode_32_safe(p, end, dlen, bad);
> + ceph_decode_need(p, end, dlen, bad);
> + ceph_decode_copy(p, ticket_buf, dlen);
> + }
> + tpend = tp + dlen;
> + dout(" ticket blob is %d bytes\n", dlen);
> + ceph_decode_need(&tp, tpend, 1 + sizeof(u64), bad);
> + blob_struct_v = ceph_decode_8(&tp);
> + new_secret_id = ceph_decode_64(&tp);
> + ret = ceph_decode_buffer(&new_ticket_blob, &tp, tpend);
> + if (ret)
> + goto out;
> +
> + /* all is well, update our ticket */
> + ceph_crypto_key_destroy(&th->session_key);
> + if (th->ticket_blob)
> + ceph_buffer_put(th->ticket_blob);
> + th->session_key = new_session_key;
> + th->ticket_blob = new_ticket_blob;
> + th->validity = new_validity;
> + th->secret_id = new_secret_id;
> + th->expires = new_expires;
> + th->renew_after = new_renew_after;
> + dout(" got ticket service %d (%s) secret_id %lld len %d\n",
> + type, ceph_entity_type_name(type), th->secret_id,
> + (int)th->ticket_blob->vec.iov_len);
> + xi->have_keys |= th->service;
> +
> +out:
> + return ret;
> +
> +bad:
> + ret = -EINVAL;
> + goto out;
> +}
> +
> static int ceph_x_proc_ticket_reply(struct ceph_auth_client *ac,
> struct ceph_crypto_key *secret,
> void *buf, void *end)
> {
> - struct ceph_x_info *xi = ac->private;
> - int num;
> void *p = buf;
> - int ret;
> char *dbuf;
> char *ticket_buf;
> u8 reply_struct_v;
> + u32 num;
> + int ret;
>
> dbuf = kmalloc(TEMP_TICKET_BUF_LEN, GFP_NOFS);
> if (!dbuf)
> @@ -150,112 +264,18 @@ static int ceph_x_proc_ticket_reply(struct ceph_auth_client *ac,
> if (!ticket_buf)
> goto out_dbuf;
>
> - ceph_decode_need(&p, end, 1 + sizeof(u32), bad);
> - reply_struct_v = ceph_decode_8(&p);
> + ceph_decode_8_safe(&p, end, reply_struct_v, bad);
> if (reply_struct_v != 1)
> - goto bad;
> - num = ceph_decode_32(&p);
> - dout("%d tickets\n", num);
> - while (num--) {
> - int type;
> - u8 tkt_struct_v, blob_struct_v;
> - struct ceph_x_ticket_handler *th;
> - void *dp, *dend;
> - int dlen;
> - char is_enc;
> - struct timespec validity;
> - struct ceph_crypto_key old_key;
> - void *tp, *tpend;
> - struct ceph_timespec new_validity;
> - struct ceph_crypto_key new_session_key;
> - struct ceph_buffer *new_ticket_blob;
> - unsigned long new_expires, new_renew_after;
> - u64 new_secret_id;
> -
> - ceph_decode_need(&p, end, sizeof(u32) + 1, bad);
> -
> - type = ceph_decode_32(&p);
> - dout(" ticket type %d %s\n", type, ceph_entity_type_name(type));
> -
> - tkt_struct_v = ceph_decode_8(&p);
> - if (tkt_struct_v != 1)
> - goto bad;
> -
> - th = get_ticket_handler(ac, type);
> - if (IS_ERR(th)) {
> - ret = PTR_ERR(th);
> - goto out;
> - }
> -
> - /* blob for me */
> - dlen = ceph_x_decrypt(secret, &p, end, dbuf,
> - TEMP_TICKET_BUF_LEN);
> - if (dlen <= 0) {
> - ret = dlen;
> - goto out;
> - }
> - dout(" decrypted %d bytes\n", dlen);
> - dend = dbuf + dlen;
> - dp = dbuf;
> -
> - tkt_struct_v = ceph_decode_8(&dp);
> - if (tkt_struct_v != 1)
> - goto bad;
> + return -EINVAL;
>
> - memcpy(&old_key, &th->session_key, sizeof(old_key));
> - ret = ceph_crypto_key_decode(&new_session_key, &dp, dend);
> - if (ret)
> - goto out;
> + ceph_decode_32_safe(&p, end, num, bad);
> + dout("%d tickets\n", num);
>
> - ceph_decode_copy(&dp, &new_validity, sizeof(new_validity));
> - ceph_decode_timespec(&validity, &new_validity);
> - new_expires = get_seconds() + validity.tv_sec;
> - new_renew_after = new_expires - (validity.tv_sec / 4);
> - dout(" expires=%lu renew_after=%lu\n", new_expires,
> - new_renew_after);
> -
> - /* ticket blob for service */
> - ceph_decode_8_safe(&p, end, is_enc, bad);
> - tp = ticket_buf;
> - if (is_enc) {
> - /* encrypted */
> - dout(" encrypted ticket\n");
> - dlen = ceph_x_decrypt(&old_key, &p, end, ticket_buf,
> - TEMP_TICKET_BUF_LEN);
> - if (dlen < 0) {
> - ret = dlen;
> - goto out;
> - }
> - dlen = ceph_decode_32(&tp);
> - } else {
> - /* unencrypted */
> - ceph_decode_32_safe(&p, end, dlen, bad);
> - ceph_decode_need(&p, end, dlen, bad);
> - ceph_decode_copy(&p, ticket_buf, dlen);
> - }
> - tpend = tp + dlen;
> - dout(" ticket blob is %d bytes\n", dlen);
> - ceph_decode_need(&tp, tpend, 1 + sizeof(u64), bad);
> - blob_struct_v = ceph_decode_8(&tp);
> - new_secret_id = ceph_decode_64(&tp);
> - ret = ceph_decode_buffer(&new_ticket_blob, &tp, tpend);
> + while (num--) {
> + ret = process_one_ticket(ac, secret, &p, end,
> + dbuf, ticket_buf);
> if (ret)
> goto out;
> -
> - /* all is well, update our ticket */
> - ceph_crypto_key_destroy(&th->session_key);
> - if (th->ticket_blob)
> - ceph_buffer_put(th->ticket_blob);
> - th->session_key = new_session_key;
> - th->ticket_blob = new_ticket_blob;
> - th->validity = new_validity;
> - th->secret_id = new_secret_id;
> - th->expires = new_expires;
> - th->renew_after = new_renew_after;
> - dout(" got ticket service %d (%s) secret_id %lld len %d\n",
> - type, ceph_entity_type_name(type), th->secret_id,
> - (int)th->ticket_blob->vec.iov_len);
> - xi->have_keys |= th->service;
> }
>
> ret = 0;
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] libceph: do not hard code max auth ticket len
2014-09-10 7:54 ` [PATCH 3/3] libceph: do not hard code max auth ticket len Ilya Dryomov
@ 2014-09-10 12:55 ` Sage Weil
0 siblings, 0 replies; 7+ messages in thread
From: Sage Weil @ 2014-09-10 12:55 UTC (permalink / raw)
To: Ilya Dryomov; +Cc: ceph-devel
On Wed, 10 Sep 2014, Ilya Dryomov wrote:
> We hard code cephx auth ticket buffer size to 256 bytes. This isn't
> enough for any moderate setups and, in case tickets themselves are not
> encrypted, leads to buffer overflows (ceph_x_decrypt() errors out, but
> ceph_decode_copy() doesn't - it's just a memcpy() wrapper). Since the
> buffer is allocated dynamically anyway, allocated it a bit later, at
> the point where we know how much is going to be needed.
>
> Fixes: http://tracker.ceph.com/issues/8979
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Ilya Dryomov <ilya.dryomov@inktank.com>
Reviewed-by: Sage Weil <sage@redhat.com>
> ---
> net/ceph/auth_x.c | 64 ++++++++++++++++++++++++-----------------------------
> 1 file changed, 29 insertions(+), 35 deletions(-)
>
> diff --git a/net/ceph/auth_x.c b/net/ceph/auth_x.c
> index 0eb146dce1aa..de6662b14e1f 100644
> --- a/net/ceph/auth_x.c
> +++ b/net/ceph/auth_x.c
> @@ -13,8 +13,6 @@
> #include "auth_x.h"
> #include "auth_x_protocol.h"
>
> -#define TEMP_TICKET_BUF_LEN 256
> -
> static void ceph_x_validate_tickets(struct ceph_auth_client *ac, int *pneed);
>
> static int ceph_x_is_authenticated(struct ceph_auth_client *ac)
> @@ -64,7 +62,7 @@ static int ceph_x_encrypt(struct ceph_crypto_key *secret,
> }
>
> static int ceph_x_decrypt(struct ceph_crypto_key *secret,
> - void **p, void *end, void *obuf, size_t olen)
> + void **p, void *end, void **obuf, size_t olen)
> {
> struct ceph_x_encrypt_header head;
> size_t head_len = sizeof(head);
> @@ -75,8 +73,14 @@ static int ceph_x_decrypt(struct ceph_crypto_key *secret,
> return -EINVAL;
>
> dout("ceph_x_decrypt len %d\n", len);
> - ret = ceph_decrypt2(secret, &head, &head_len, obuf, &olen,
> - *p, len);
> + if (*obuf == NULL) {
> + *obuf = kmalloc(len, GFP_NOFS);
> + if (!*obuf)
> + return -ENOMEM;
> + olen = len;
> + }
> +
> + ret = ceph_decrypt2(secret, &head, &head_len, *obuf, &olen, *p, len);
> if (ret)
> return ret;
> if (head.struct_v != 1 || le64_to_cpu(head.magic) != CEPHX_ENC_MAGIC)
> @@ -131,18 +135,19 @@ static void remove_ticket_handler(struct ceph_auth_client *ac,
>
> static int process_one_ticket(struct ceph_auth_client *ac,
> struct ceph_crypto_key *secret,
> - void **p, void *end,
> - void *dbuf, void *ticket_buf)
> + void **p, void *end)
> {
> struct ceph_x_info *xi = ac->private;
> int type;
> u8 tkt_struct_v, blob_struct_v;
> struct ceph_x_ticket_handler *th;
> + void *dbuf = NULL;
> void *dp, *dend;
> int dlen;
> char is_enc;
> struct timespec validity;
> struct ceph_crypto_key old_key;
> + void *ticket_buf = NULL;
> void *tp, *tpend;
> struct ceph_timespec new_validity;
> struct ceph_crypto_key new_session_key;
> @@ -167,8 +172,7 @@ static int process_one_ticket(struct ceph_auth_client *ac,
> }
>
> /* blob for me */
> - dlen = ceph_x_decrypt(secret, p, end, dbuf,
> - TEMP_TICKET_BUF_LEN);
> + dlen = ceph_x_decrypt(secret, p, end, &dbuf, 0);
> if (dlen <= 0) {
> ret = dlen;
> goto out;
> @@ -195,20 +199,25 @@ static int process_one_ticket(struct ceph_auth_client *ac,
>
> /* ticket blob for service */
> ceph_decode_8_safe(p, end, is_enc, bad);
> - tp = ticket_buf;
> if (is_enc) {
> /* encrypted */
> dout(" encrypted ticket\n");
> - dlen = ceph_x_decrypt(&old_key, p, end, ticket_buf,
> - TEMP_TICKET_BUF_LEN);
> + dlen = ceph_x_decrypt(&old_key, p, end, &ticket_buf, 0);
> if (dlen < 0) {
> ret = dlen;
> goto out;
> }
> + tp = ticket_buf;
> dlen = ceph_decode_32(&tp);
> } else {
> /* unencrypted */
> ceph_decode_32_safe(p, end, dlen, bad);
> + ticket_buf = kmalloc(dlen, GFP_NOFS);
> + if (!ticket_buf) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + tp = ticket_buf;
> ceph_decode_need(p, end, dlen, bad);
> ceph_decode_copy(p, ticket_buf, dlen);
> }
> @@ -237,6 +246,8 @@ static int process_one_ticket(struct ceph_auth_client *ac,
> xi->have_keys |= th->service;
>
> out:
> + kfree(ticket_buf);
> + kfree(dbuf);
> return ret;
>
> bad:
> @@ -249,21 +260,10 @@ static int ceph_x_proc_ticket_reply(struct ceph_auth_client *ac,
> void *buf, void *end)
> {
> void *p = buf;
> - char *dbuf;
> - char *ticket_buf;
> u8 reply_struct_v;
> u32 num;
> int ret;
>
> - dbuf = kmalloc(TEMP_TICKET_BUF_LEN, GFP_NOFS);
> - if (!dbuf)
> - return -ENOMEM;
> -
> - ret = -ENOMEM;
> - ticket_buf = kmalloc(TEMP_TICKET_BUF_LEN, GFP_NOFS);
> - if (!ticket_buf)
> - goto out_dbuf;
> -
> ceph_decode_8_safe(&p, end, reply_struct_v, bad);
> if (reply_struct_v != 1)
> return -EINVAL;
> @@ -272,22 +272,15 @@ static int ceph_x_proc_ticket_reply(struct ceph_auth_client *ac,
> dout("%d tickets\n", num);
>
> while (num--) {
> - ret = process_one_ticket(ac, secret, &p, end,
> - dbuf, ticket_buf);
> + ret = process_one_ticket(ac, secret, &p, end);
> if (ret)
> - goto out;
> + return ret;
> }
>
> - ret = 0;
> -out:
> - kfree(ticket_buf);
> -out_dbuf:
> - kfree(dbuf);
> - return ret;
> + return 0;
>
> bad:
> - ret = -EINVAL;
> - goto out;
> + return -EINVAL;
> }
>
> static int ceph_x_build_authorizer(struct ceph_auth_client *ac,
> @@ -603,13 +596,14 @@ static int ceph_x_verify_authorizer_reply(struct ceph_auth_client *ac,
> struct ceph_x_ticket_handler *th;
> int ret = 0;
> struct ceph_x_authorize_reply reply;
> + void *preply = &reply;
> void *p = au->reply_buf;
> void *end = p + sizeof(au->reply_buf);
>
> th = get_ticket_handler(ac, au->service);
> if (IS_ERR(th))
> return PTR_ERR(th);
> - ret = ceph_x_decrypt(&th->session_key, &p, end, &reply, sizeof(reply));
> + ret = ceph_x_decrypt(&th->session_key, &p, end, &preply, sizeof(reply));
> if (ret < 0)
> return ret;
> if (ret != sizeof(reply))
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-09-10 12:55 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-10 7:54 [PATCH 0/3] libceph: #8979 fix (wip-auth-8979) Ilya Dryomov
2014-09-10 7:54 ` [PATCH 1/3] libceph: gracefully handle large reply messages from the mon Ilya Dryomov
2014-09-10 12:48 ` Sage Weil
2014-09-10 7:54 ` [PATCH 2/3] libceph: add process_one_ticket() helper Ilya Dryomov
2014-09-10 12:52 ` Sage Weil
2014-09-10 7:54 ` [PATCH 3/3] libceph: do not hard code max auth ticket len Ilya Dryomov
2014-09-10 12:55 ` Sage Weil
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.