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.129.124]) by passt.top (Postfix) with ESMTP id DBE445A005E for ; Wed, 15 Feb 2023 08:51:04 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1676447463; 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=wDSxiyJEmT6IB1vwoHTNlYnWVa0Pi0dfitMfhmPKuW4=; b=FKW7mZwrAtdykNjF5wxCLTCT7Tugf3/rQh0q3W5PFuHZQiaeyHAijF/x7ne96o9uLqs0x5 sPaRKMPa1TnsO+CesVUfc9bS529jFlwFFKMvWrmV4aPBPYCw9cwHPfQa87S1EqQh5bPASJ cdSwhrimIanHyPkLnfvYGi0W+RV1TVs= Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-591-GUV_JLAWNVCobHUJoYrLUQ-1; Wed, 15 Feb 2023 02:51:01 -0500 X-MC-Unique: GUV_JLAWNVCobHUJoYrLUQ-1 Received: by mail-qv1-f70.google.com with SMTP id c10-20020a05621401ea00b004c72d0e92bcso10092887qvu.12 for ; Tue, 14 Feb 2023 23:51:01 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:organization:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=wDSxiyJEmT6IB1vwoHTNlYnWVa0Pi0dfitMfhmPKuW4=; b=EjovrR6nWbQtUESrql3ZWQLiaFgwSuuEioAieAC8wBLAD8o/0Zk0y0lTQv+1u5PpLb O2+7jZwUdlW9bKXEySNGBe/gnrWCIXhx98Htw4cceV8o1yQy7tS3sfxfSH1uDf+T1t1l uYzwV1UtFSK/9O4CNudfFrmMj2ydBekSE2PcrCsy16mPslYvmOpOQeXNxpoPYHkXMJAW x9AUGjtHiWsiDCM3cw4Xe4XXfspXr+01uvPsVWilDkrTU8h32jhBGmMKr06p8ukLYkgY 9Oam+KGQXuKsdWVSJ5aKnouK4UJVN3ExXMdd0FC9HXV/0cJzLBzf4Dz6KV4g3sKz6zpR T58g== X-Gm-Message-State: AO0yUKXd+KMfFJj+Grm76AYS9wkPFDv7OI0sfRpPGHY4t2Ht8fliZcaW X8EK2NZsRddRtfVO/7fMdaMEqGG/MmeqJoRd7WpJrO0t8Vkx3jHiop55LJNeg0yCfmuYt3eZYup ya+oXU4b7LciX X-Received: by 2002:ad4:5b8f:0:b0:56e:b251:a8e1 with SMTP id 15-20020ad45b8f000000b0056eb251a8e1mr2691296qvp.4.1676447461081; Tue, 14 Feb 2023 23:51:01 -0800 (PST) X-Google-Smtp-Source: AK7set+X6tv8BMIVs5OD6sY8NqBIY/xNHqyyK4Z7ToOMcTknoLypQfUTlptGuGsEFTB4q+IFTL0oKA== X-Received: by 2002:ad4:5b8f:0:b0:56e:b251:a8e1 with SMTP id 15-20020ad45b8f000000b0056eb251a8e1mr2691274qvp.4.1676447460744; Tue, 14 Feb 2023 23:51:00 -0800 (PST) Received: from ?IPV6:2600:8805:3a00:3:3b4f:6d3c:92c4:a5c7? ([2600:8805:3a00:3:3b4f:6d3c:92c4:a5c7]) by smtp.gmail.com with ESMTPSA id b4-20020a378004000000b0073b4a797702sm4504042qkd.40.2023.02.14.23.51.00 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 14 Feb 2023 23:51:00 -0800 (PST) Message-ID: <90dbb5f3-7b3f-893c-ca32-a7653eb486c6@redhat.com> Date: Wed, 15 Feb 2023 02:50:59 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Subject: Re: [PATCH 4/4] qemu_passt: Don't let passt fork off To: Libvirt , passt-dev@passt.top References: <5abfc412e4692a38e980c8dc600e1bfbd03ddcfd.1676374699.git.mprivozn@redhat.com> <20230214140253.49bbc13a@elisabeth> From: Laine Stump Organization: Red Hat In-Reply-To: <20230214140253.49bbc13a@elisabeth> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Message-ID-Hash: 5423CSJK4DDZQGZYE3RKKHLNHZMII3W7 X-Message-ID-Hash: 5423CSJK4DDZQGZYE3RKKHLNHZMII3W7 X-MailFrom: laine@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: Stefano Brivio , Michal Privoznik X-Mailman-Version: 3.3.3 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: On 2/14/23 8:02 AM, Stefano Brivio wrote: > On Tue, 14 Feb 2023 12:51:22 +0100 > Michal Privoznik wrote: > >> When passt starts it tries to do some security measures to >> restrict itself. For instance, it creates its own namespaces, >> umounts basically everything, drops capabilities, forks off to >> further restrict itself (the child is where all interesting work >> takes place now). This is sound, except it's causing two >> problems: >> >> 1) the PID file FD, which we leak into the passt process, gets >> closed (and thus our virPidFile*() helpers see unlocked PID >> file, which makes them think the process is gone), > > I didn't realise this was the case, but giving passt write (unless I'm > missing something) access to a file created by libvirtd doesn't look > desirable to me. > >> 2) the PID file no longer reflects true PID of the process. >> >> Worse, the child calls setsid() so we can't even kill the whole >> process group. I mean, we can but it won't be any good. I think that (incorrect PID in the pidfile) is happening because Michal is using the original version of my patches that were pushed - I had mimicked the behavior of slirp, where libvirt deamonizes the new process. If that process then daemonizes itself, we have some sort of "double daemon"; libvirt has saved off the pid of what it thinks is going to be the final process, but then that process further forks and exits from the process whose pid libvirt saved. But because passt was cleaning up after itself I hadn't noticed the discrepancy in pids when testing. Without going into all the details of the pidfile and locking and etc, I just want to say that if we can fork/exec dnsmasq and let it daemonize itself and create its own pidfile, then certainly we can do the same thing for passt. (and if there's a fundamental problem, then it's a fundamental problem for dnsmasq as well). >> >> Fortunately, passt has '--foreground' argument, which causes it >> to undergo the same security measures but without forking off the >> child. But if we do --foreground in combination with calling virCommandDaemonize(), then won't we still have the problem that libvirt won't know whether or not passt has failed to start (not unless we want to put in a bunch of gorp to continue grabbing stderr while watching to see if the passt process has exited, etc. It's going to take some convincing for me to think we should run passt with --foreground rather than letting it daemonize itself. > > They're not the same -- unfortunately they can't be, because, on Linux, > you can't change the PID of an existing process, so there's no way to > enter a new PID namespace without clone(). > > If passt remains in the same PID namespace, it's still able to see PIDs > of other processes, which is not desirable from a security perspective. > > Again from a security perspective, this is probably a small impact, so > I guess it's fine if there's no other way around it. But I see a lot of > ways around it... >