From patchwork Wed Sep 10 07:54:31 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Dryomov X-Patchwork-Id: 4874981 Return-Path: X-Original-To: patchwork-ceph-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id E5CBDC0338 for ; Wed, 10 Sep 2014 07:54:47 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id E377C201BF for ; Wed, 10 Sep 2014 07:54:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B46AB2018B for ; Wed, 10 Sep 2014 07:54:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751721AbaIJHyo (ORCPT ); Wed, 10 Sep 2014 03:54:44 -0400 Received: from mail-lb0-f171.google.com ([209.85.217.171]:36165 "EHLO mail-lb0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751283AbaIJHyn (ORCPT ); Wed, 10 Sep 2014 03:54:43 -0400 Received: by mail-lb0-f171.google.com with SMTP id 10so11201596lbg.16 for ; Wed, 10 Sep 2014 00:54:41 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=ouU03u/cuqC21M6DSwqVXESvs7hwdsImHHIlqYz7s7w=; b=Lq/vMC7YLhgjcxhYGNc5TdoRlT+6y2mKdSlaV7WiprMpFwdE6GH1TrfL6ALRf4IW8z jNHyWuwBE5mv7hmhgrYg/LFoiTgiypv4BaoB+4+AsKCCOmh+tt9z/XCm/iQtrOtJUzn6 tlZ6wigFXHIjZry1JiWCWWpjkQX3lg1AfWJ4G3HMuhRtlSvqQLMhb/5QVcAlWi9PXqYP 11ww2MEaTj44q3IPYexn8L/k75au7k47rfJG3lsa4x1/Ge7tr61KTUhlEv+dwB9Qa2tP vy4PaO2mjX3QMk6gRdTPKO8ERPy8Ga0bQGL1ryfbXgI4OFpGoKNTi4tsHikc3E9oH3Yv bzag== X-Gm-Message-State: ALoCoQmNEBxgsIwXyCAFKnBXWH0JEowAimFz594cGYt8sfpMFjAHBPSZ5jcnX0sgPIB7s/cxmiTq X-Received: by 10.112.98.140 with SMTP id ei12mr38803137lbb.6.1410335681791; Wed, 10 Sep 2014 00:54:41 -0700 (PDT) Received: from localhost ([109.110.67.183]) by mx.google.com with ESMTPSA id f6sm5035650lam.49.2014.09.10.00.54.40 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Wed, 10 Sep 2014 00:54:41 -0700 (PDT) From: Ilya Dryomov To: ceph-devel@vger.kernel.org Subject: [PATCH 3/3] libceph: do not hard code max auth ticket len Date: Wed, 10 Sep 2014 11:54:31 +0400 Message-Id: <1410335671-4581-4-git-send-email-ilya.dryomov@inktank.com> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1410335671-4581-1-git-send-email-ilya.dryomov@inktank.com> References: <1410335671-4581-1-git-send-email-ilya.dryomov@inktank.com> Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org X-Spam-Status: No, score=-9.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 Reviewed-by: Sage Weil --- 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))