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 E8A255A004F for ; Thu, 20 Jun 2024 16:22:55 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1718893374; 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=ew7juJxB4U+e9YjqC5VSHNHpr+cNDXYlKiWT9XgTnXc=; b=b2zDzlSF85Ow0OS1JIcMqrTUpnPNGxrJg+Kup/WX0WQf+3zh19rePX/EzijevSCMfDHFcF TXmEM5VCgVDRblrAgvIKpcla7PvxNmlqYD7bWiHlYIF0GKdK5Cu22qLUO/E24XkwJkIHrf fp//tHhUkqJxc3YSY4mHysLhSlBoHGM= Received: from mail-ot1-f70.google.com (mail-ot1-f70.google.com [209.85.210.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-695-G8-qrzWvMZ2VLPbyGNuvOw-1; Thu, 20 Jun 2024 10:22:53 -0400 X-MC-Unique: G8-qrzWvMZ2VLPbyGNuvOw-1 Received: by mail-ot1-f70.google.com with SMTP id 46e09a7af769-6fc0266675dso1138657a34.2 for ; Thu, 20 Jun 2024 07:22:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718893370; x=1719498170; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=ew7juJxB4U+e9YjqC5VSHNHpr+cNDXYlKiWT9XgTnXc=; b=q48Q2eBbNJ1RRU/lVqp7DV0H8+krB1s4WvYLaraH6natcy0BwXWCluhZlJ5cIzbBaI Twz7pBbYtA3q3gq98zCcwHPcx/M4f9UfDEAwettaNpH040NJ6mJxRDWzlssRzhy/W7gU 4jETS2MrWoWUHNiLvWzfqB3TH3Ip05NLkcHaH2A9a0K0kpI02csfcGniopMFtvTEhC4c Jmx/b5AkzO5xr4b0Znx1WFI7tLUM7corhTkDDxy9LARzb6deUBvUAK2L+1conTNScL66 pf23EXT1G7wcDUMu2IGT2jKEh7xKlJNPc/bu1DGQj3ONATz3EBdp+jQxdbqm89/49u3N VC8g== X-Forwarded-Encrypted: i=1; AJvYcCXef9scPF2JzvpCB7SAAM6NIRKhVrP4uUYB/q66MaRUp3BXk1bzgjdYLVF9Sk5JZB8gt7DC2/DCYqWnX7geDMik7ND/ X-Gm-Message-State: AOJu0YyI1pEFPrhrou730YJET1r+DjCQiQwDhRMbBJHt0t1KXAi1rkac yjgZYJhiVsi0nDEzg7w3hAgqTeeqRfqZqAEVB+veFKfG3nv9gRnblEioPjBX/jcuItTt52lQ/cM Kj75oh5N/U/dE2NetNXp8vlJ9qbt/1j/XWPjS5j2UCc7T0G0YRw== X-Received: by 2002:a05:6830:606:b0:6f9:87ea:2159 with SMTP id 46e09a7af769-7007171e6f5mr5911985a34.0.1718893369778; Thu, 20 Jun 2024 07:22:49 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEdzGsgmNVj6ioDUleSXMqExPiZy+8eZIGCTKWKFTHPsQ/9dk/atgVTU4PDgR1kbSv1xzrjng== X-Received: by 2002:a05:6830:606:b0:6f9:87ea:2159 with SMTP id 46e09a7af769-7007171e6f5mr5911952a34.0.1718893369211; Thu, 20 Jun 2024 07:22:49 -0700 (PDT) Received: from maya.cloud.tilaa.com (maya.cloud.tilaa.com. [164.138.29.33]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-6b4fcf9ab77sm25881276d6.7.2024.06.20.07.22.48 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 20 Jun 2024 07:22:48 -0700 (PDT) Date: Thu, 20 Jun 2024 16:22:12 +0200 From: Stefano Brivio To: "Richard W.M. Jones" Subject: Re: [PATCH 7/8] conf, passt, tap: Open socket and PID files before switching UID/GID Message-ID: <20240620162212.09ccc2f9@elisabeth> In-Reply-To: <20240620124730.GC1450@redhat.com> References: <20240522205911.261325-1-sbrivio@redhat.com> <20240522205911.261325-8-sbrivio@redhat.com> <20240620113054.GB1450@redhat.com> <20240620141253.7e6d3855@elisabeth> <20240620124730.GC1450@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.41; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: Q5TX3GCUWKAZR2OPZCQGHJH63S44ETA6 X-Message-ID-Hash: Q5TX3GCUWKAZR2OPZCQGHJH63S44ETA6 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: David Gibson , passt-dev@passt.top, Minxi Hou 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: On Thu, 20 Jun 2024 13:47:31 +0100 "Richard W.M. Jones" wrote: > On Thu, Jun 20, 2024 at 02:12:53PM +0200, Stefano Brivio wrote: > > On Thu, 20 Jun 2024 12:30:54 +0100 > > "Richard W.M. Jones" wrote: > > > > > On Wed, May 29, 2024 at 12:35:24PM +1000, David Gibson wrote: > > > > On Wed, May 22, 2024 at 10:59:10PM +0200, Stefano Brivio wrote: > > > > > Otherwise, if the user runs us as root, and gives us paths that are > > > > > only accessible by root, we'll fail to open them, which might in turn > > > > > encourage users to change permissions or ownerships: definitely a bad > > > > > idea in terms of security. > > > > > > > > > > Reported-by: Minxi Hou > > > > > Reported-by: Richard W.M. Jones > > > > > Signed-off-by: Stefano Brivio > > > > > > > > Looking at this I did notice a pre-existing, well, maybe not bug > > > > exactly, but possibly surprising behaviour, which makes me a > > > > bit more nervous now that we can invoke it as root. > > > > > > > > tap_sock_unix_open() will silently truncate the given socket path to > > > > the maximum length for a Unix socket. Which means we could bind(), > > > > but also unlink() a path that's not exactly the same as the one the > > > > one the user requested. I don't immediately see a way to exploit > > > > that, but it's the sort of thing that makes me nervous. I think we > > > > should instead outright fail if the given socket path is too long. > > > > > > Yes, agreed. > > > > > > It seems as if the latest passt code still does this. Do you want me > > > to open a bug about it? > > > > Yes, please, that, or a patch :) > > While I was testing this, I found we do seem to check it: > > https://passt.top/passt/tree/conf.c#n1446 Oh, I thought David was referring to the loop in tap_sock_unix_open(), where we try paths in the form "/tmp/passt_%i.socket". But even there, we can't exceed UNIX_PATH_MAX. One minor issue remains, though: in conf(), we refuse paths that are longer than UNIX_SOCK_MAX (100). That's the maximum index for the "/tmp/passt_%i.socket", it happens to be a sane value, but we should use UNIX_PATH_MAX (108) instead. I'll fix it, but wait for David's feedback first. -- Stefano