From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTP id 379555A0262 for ; Tue, 6 Jun 2023 22:58:59 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1686085138; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=wIN1j+FveZATZmllbMThnlAe97lGdjMmmTJc9iZGcfc=; b=iEbdMe9SlQsNjPhRHP7aFyiyP4fHTxk147jwLZ0c9wogoaqjk3S0tJ/OC2hlxTwkl51UW9 c82W/Fu2YBPVtJBrNhEz/JUyfsQFhu7V7g9z+nURc+lZ4ID2GXH+UC7rHGBiMEixH2WqtM TWrIDTspaRh0+618K9OzbXIIzUk72BE= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-532-Hcw8cvDRN7yvSp4-R3Zo4Q-1; Tue, 06 Jun 2023 16:58:56 -0400 X-MC-Unique: Hcw8cvDRN7yvSp4-R3Zo4Q-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 84CD5280D582 for ; Tue, 6 Jun 2023 20:58:56 +0000 (UTC) Received: from elisabeth (unknown [10.39.208.3]) by smtp.corp.redhat.com (Postfix) with ESMTPS id CC7BA9E8B; Tue, 6 Jun 2023 20:58:55 +0000 (UTC) Date: Tue, 6 Jun 2023 22:58:36 +0200 From: Stefano Brivio To: Michal Privoznik Subject: Re: [PATCH 0/2] Introduce --log-fd option Message-ID: <20230606225836.63aecebe@elisabeth> In-Reply-To: References: Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.5 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: MYUPQTSC74O5B3YWHXBF6W4YD5VOZAPD X-Message-ID-Hash: MYUPQTSC74O5B3YWHXBF6W4YD5VOZAPD X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Hi, On Tue, 6 Jun 2023 13:41:28 +0200 Michal Privoznik wrote: > The idea behind this is so that libvirt can create the log file, set all > DAC/SELinux labels and just pass pre-opened FD to passt. > > You can view my WIP patches for libvirt here: > > https://gitlab.com/MichalPrivoznik/libvirt/-/commit/5bbe40087d6888a7c4031a2224731523e117c072 Thanks for the implementation... and also for taking care of the details! I'm not really enthusiastic about the security implications of this approach, but _if it's the only reasonable way to solve this_, I won't certainly stand in the way of progress. The series looks mostly good to me, I have only a few nits, reported in the single replies. This adds a further interfacing mode between passt and the parent process, which makes me a bit uncomfortable in general. Specifically, if the parent process runs as root, this gives a rogue passt process a way to write potentially unlimited amounts of data to essentially any place (minus _some_ checks implemented by Linux Security Modules). And a rogue passt process doesn't necessarily imply a rogue parent, so this is additional surface. Oh, and by the way of LSMs, we kind of bypass stuff like this. For a non-rogue passt process, if the parent runs as root, I don't see any additional attack scenario -- the parent could do anything it wants anyway. But if the parent runs as regular user, there are a few additional ways to cause passt to misbehave by passing in a file descriptor that doesn't correspond to a file, or that's opened by other processes, without any mediation by the filesystem (which is generally speaking not under control of unprivileged users). I'd rather much prefer the more common approach of defaulting, or suggesting to the user, to write to a temporary filesystem, available under most distributions under /run or /var/run. Is that really unfeasible? I'm thinking that libvirt already needs a specific directory for passt to use (socket and PID files). What if logFile, other than an absolute path, supported a relative path? This logic might perhaps apply to other helpers or external programs too. By the way, passt ships with AppArmor and SELinux example policies, which are also included in packages. They would need at least a quick review, probably some edits, and some basic tests. Thinking of those, a relative path would also simplify things. -- Stefano