From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id E8C9C3857415 for ; Wed, 18 Aug 2021 22:55:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E8C9C3857415 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-338-mk6zfkVoNY2abDVdYVUULg-1; Wed, 18 Aug 2021 18:55:44 -0400 X-MC-Unique: mk6zfkVoNY2abDVdYVUULg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A02F91F2D9 for ; Wed, 18 Aug 2021 22:55:43 +0000 (UTC) Received: from redhat.com (ovpn-112-63.phx2.redhat.com [10.3.112.63]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 83B13369A; Wed, 18 Aug 2021 22:55:43 +0000 (UTC) Received: from fche by redhat.com with local (Exim 4.94.2) (envelope-from ) id 1mGUTF-00029b-VF; Wed, 18 Aug 2021 18:55:42 -0400 Date: Wed, 18 Aug 2021 18:55:41 -0400 From: "Frank Ch. Eigler" To: Di Chen Cc: elfutils-devel@sourceware.org Subject: Re: [PATCH] debuginfod: PR27917 - protect against federation loops Message-ID: <20210818225541.GE6064@redhat.com> References: MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.12.0 (2019-05-25) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: elfutils-devel@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Elfutils-devel mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 Aug 2021 22:55:47 -0000 Hi - > This patch aims to reduce the risk by adding an option to debuginfod > that functions kind of like an IP packet's TTL: a limit on the > length of XFF: header that debuginfod is willing to process. If > X-Forwarded-For: exceeds N hops, it will not delegate a local lookup > miss to upstream debuginfods. [...] Thank you very much! > Commit ab38d167c40c99 causes federation loops for non-existent > resources to result in multiple temporary livelocks, each lasting > for $DEBUGINFOD_TIMEOUT seconds. [...] (FWIW, the term "livelock" is not quite right here, try just "deadlock".) The patch looks functional, and thank you also for including the docs and test case. Thorough enough! > @@ -1862,6 +1869,12 @@ handle_buildid (MHD_Connection* conn, > // We couldn't find it in the database. Last ditch effort > // is to defer to other debuginfo servers. > > + // if X-Forwarded-For: exceeds N hops, > + // do not delegate a local lookup miss to upstream debuginfods. > + if (disable_query_server) > + throw reportable_exception(MHD_HTTP_NOT_FOUND, "not found, > --forwared-ttl-limit reached \ > +and will not query the upstream servers"); One part I don't understand is why you added the code to check for XFF length into handler_cb(), and then passed the disable_query_server result flag to this function. Was there some reason not to perform the XFF comma-counting right here? - FChE