From patchwork Thu Jun 18 15:50:39 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Carlos O'Donell X-Patchwork-Id: 486360 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4EBA2140271 for ; Fri, 19 Jun 2015 01:50:56 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=sourceware.org header.i=@sourceware.org header.b=TgxIGY4m; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:subject :content-type:content-transfer-encoding; q=dns; s=default; b=xT6 C4Jl72CbFosF9X2bcMpvTNkYLPK9Emo7H34eGVcwjC3Bq9q4KT4PWsGF/zjczdYN vx0AIQc/8+SeJaW1MR4AkERw4fjD+wQOrs3RyJhTKrZIDPxIkY3PqVxxTssjVlu5 46305nKSP/BHNx1v/brJF6jd6J2P8VHYZTW+/3RY= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:subject :content-type:content-transfer-encoding; s=default; bh=pUUH0zYxg qNtPYIpqfrt1CKbLA4=; b=TgxIGY4mU1EqWxGMQ7lj9XrWW0aohr1L3aAs0LtRZ HXRfN6cVqiOapwn5O6DFKCwG3eSP27EQSwytep7uTzcXjKk4o14TmYsMwxycAPWK 5KNXSBr2soYqVtKltLuxhnZZ1WPVNAiPZuLzd7ts3TKzzEhlCjrIviLDyCoogz5L KI= Received: (qmail 113342 invoked by alias); 18 Jun 2015 15:50:48 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 113268 invoked by uid 89); 18 Jun 2015 15:50:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Message-ID: <5582E8CF.3030509@redhat.com> Date: Thu, 18 Jun 2015 11:50:39 -0400 From: "Carlos O'Donell" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: GNU C Library Subject: [PATCH] Fix ruserok scalability with large ~/.rhosts file. The ruserok API does hosts checks first while it walks the user's ~/.rhosts file. This results in lots of DNS queries that could have been skipped if we short-circuit test the user portion first to see if would have had a failed match. This supports configurations where rlogin is used on internal secure networks with large numbers of users and machines. The Red Hat QE team did extensive testing on various rlogin combinations to validate this change, and in fact we found a defect in the first version which is fixed in this version. Unfortunately without installed tree + container testing we can't add an easy test case for this. We need to setup one or two systems in order to verify, and that's what we did. We'll get to this eventually though. I have also updated the linux kernel man page to describe the configuration syntax in more detail: http://git.kernel.org/cgit/docs/man-pages/man-pages.git/commit/?id=427cee53f06a4be5bfd808191ecc5624d3f0240b (with some follow up commits) Tested on x86-64, i686, ppc64, ppc64le, aarch64, s390, and s390x with no regressions. OK? 2015-06-18 Carlos O'Donell * inet/rcmd.c (__validuser2_sa): Check user first to short-circuit additional host check. diff --git a/inet/rcmd.c b/inet/rcmd.c index 98b3735..91623b0 100644 --- a/inet/rcmd.c +++ b/inet/rcmd.c @@ -809,29 +809,38 @@ __validuser2_sa(hostf, ra, ralen, luser, ruser, rhost) *p = '\0'; /* terminate username (+host?) */ /* buf -> host(?) ; user -> username(?) */ + if (*buf == '\0') + break; + if (*user == '\0') + user = luser; + + /* First check the user part. This is an optimization, since + one should always check the host first in order to detect + negative host checks (which we check for later). */ + ucheck = __icheckuser (user, ruser); + + /* Either we found the user, or we didn't and this is a + negative host check. We must do the negative host lookup + in order to preserve the semantics of stopping on this line + before processing others. */ + if (ucheck != 0 || *buf == '-') { + + /* Next check host part */ + hcheck = __checkhost_sa (ra, ralen, buf, rhost); + + /* Negative '-host user(?)' match? */ + if (hcheck < 0) + break; - /* First check host part */ - hcheck = __checkhost_sa (ra, ralen, buf, rhost); - - if (hcheck < 0) - break; - - if (hcheck) { - /* Then check user part */ - if (! (*user)) - user = luser; - - ucheck = __icheckuser (user, ruser); - - /* Positive 'host user' match? */ - if (ucheck > 0) { + /* Positive 'host user' match? */ + if (hcheck > 0 && ucheck > 0) { retval = 0; break; } - /* Negative 'host -user' match? */ - if (ucheck < 0) - break; + /* Negative 'host -user' match? */ + if (hcheck > 0 && ucheck < 0) + break; /* Neither, go on looking for match */ }