From nobody Sun May 19 18:11:17 2024 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Vj80B4czQz5KskN for ; Sun, 19 May 2024 18:11:22 +0000 (UTC) (envelope-from shawn.webb@hardenedbsd.org) Received: from mail-io1-xd34.google.com (mail-io1-xd34.google.com [IPv6:2607:f8b0:4864:20::d34]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Vj8096Sppz4ymT for ; Sun, 19 May 2024 18:11:21 +0000 (UTC) (envelope-from shawn.webb@hardenedbsd.org) Authentication-Results: mx1.freebsd.org; none Received: by mail-io1-xd34.google.com with SMTP id ca18e2360f4ac-7e1e06c9a10so107556539f.3 for ; Sun, 19 May 2024 11:11:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=hardenedbsd.org; s=google; t=1716142279; x=1716747079; darn=freebsd.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=pV7ecV2+Xq8l2PD6zPE3eYsW8FDemf2JtdJLrXKns48=; b=KzmOrTJHl5d8rycpYa7KlRh4ydKDffM76RCClmpc7AYFBCjE1ZFuh9YNDT8acDwEXZ DDp7SLOrMbTWyvmbtOw92zdIb0DLKQSPwMJmvPibt30R4kg0/VFoEt5IpfVrgWEV/gY/ LeGod8KPhIM8bUGHZG2U6P78JgHT3GoxNZBZrv8xCYlXT3+XVwo67h9kJDmQpigyPQ1k dQSRiVzTSVDr1nStlLCKCKahn4jyTPQrg3RiiU+hjMeb5kksS+WQHt2LXUkD9vVJkk4g JelkWNJo/JmOQBnvH00j5Nic9ACoE9guVt0lgvQ8Gkkm/bdTXi8av/yi6gFgSqJZc25/ +e5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716142279; x=1716747079; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=pV7ecV2+Xq8l2PD6zPE3eYsW8FDemf2JtdJLrXKns48=; b=FJJY68ULpbMI8H+EMO9Bm3zsTnCrz27U/HmJJXLgrr5/73H3r231TWr/+5JYWbDLax sP50nJ4rKfilhw36XCIPfqCBPUfw7zFXlyUyzIW8Z8J0kpFIfKEp6wTFxgNS0t2QbUA8 LUWA59lPEOrwWAPmMzEbWGK5h0rNiQ2P2Le+K4IQXc6dYyj4EaggPc47bdMYcoDRdN/k iRNUJlbjNkaNdVzGFf8t376NkXZTEjhhDJ+u1qB9nKdmkFOlmZt/gShoDfPnUr4iajSS 3GfW3p1rIE20gvgcwOGgEqDx8joju4CFmY7wQCPyeLHlkNj4nMj/6xgGUlzVftOBjDCL sS5Q== X-Forwarded-Encrypted: i=1; AJvYcCVmrWQXsJ7RN7xvcJ/u+qX5bPtTJHWUtdSYRtY80k+XEtBtdz6mDYB9cGqDipnc4mfySmmuhmPCLTvGmjTQIf9b65tDMfcZXRKrVpEt1vD6GQ== X-Gm-Message-State: AOJu0YzQ4OvK9wri4L8r5I27kxcGoWqXNh+2rppHlJmocOpmChVj2wQF N8JyMr2Q1rVM5jilhWiGr61HS25ZDDRQalUzlDCz4Y6oHS40phkKVtSpu7lWiSY= X-Google-Smtp-Source: AGHT+IG/sFIOG6ehNgitRRhIqCe6pONnwSGBbDuN3obHFSMf2PnHPbr87zGQ193d24WaOG1K9tYwyQ== X-Received: by 2002:a5d:948e:0:b0:7d9:6351:4ef3 with SMTP id ca18e2360f4ac-7e1b51bb73amr2647964839f.5.1716142279369; Sun, 19 May 2024 11:11:19 -0700 (PDT) Received: from mutt-hbsd ([184.99.37.29]) by smtp.gmail.com with ESMTPSA id 8926c6da1cb9f-489376dc822sm5821808173.131.2024.05.19.11.11.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 19 May 2024 11:11:18 -0700 (PDT) Date: Sun, 19 May 2024 18:11:17 +0000 From: Shawn Webb To: Pedro Giffuni Cc: Kyle Evans , "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Subject: Re: git: be04fec42638 - main - Import _FORTIFY_SOURCE implementation from NetBSD Message-ID: X-Operating-System: FreeBSD mutt-hbsd 15.0-CURRENT-HBSD FreeBSD 15.0-CURRENT-HBSD X-PGP-Key: https://git.hardenedbsd.org/hardenedbsd/pubkeys/-/blob/master/Shawn_Webb/03A4CBEBB82EA5A67D9F3853FF2E67A277F8E1FA.pub.asc References: <02326b5e-a1fe-4411-a869-d21f9a76130c@email.android.com> <999469960.1638478.1716080957814@mail.yahoo.com> <6276b721-6c7b-41cd-9d1b-4169e86ec5e9@FreeBSD.org> <1413980952.1357400.1716093599901@mail.yahoo.com> <216ca4a4-c0b8-4d72-bc6f-95e82a6b77da@FreeBSD.org> <418178403.1722964.1716130042183@mail.yahoo.com> List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="rvc6baalkatqcyi5" Content-Disposition: inline In-Reply-To: <418178403.1722964.1716130042183@mail.yahoo.com> X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US] X-Rspamd-Queue-Id: 4Vj8096Sppz4ymT --rvc6baalkatqcyi5 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable I did not know a battle was even being fought. :-) I'd like to give a little hypothetical as to why I think these features are complementary to each other. The primary goal of SafeStack is to protect the control flow. This means putting spillable data in a separate stack, named the unsafe stack. Spilling one buffer into another is (usually) not as bad as spilling onto the return address pointer on the stack. SafeStack does not attempt to prevent spillage. The primary goal of the SSP canary is also to protect control flow. However, in some cases, the check may be too late. If a function pointer sits adjacent to a spilled buffer, and the function pointer is called after the buffer is overflowed, the attacker gains arbitrary code execution. One feature critical to _FORTIFY_SOURCE is better bounds checking, both at compile time and run time. Let's say, hypothetically, that two buffers are created and sit adjacent to each other. SafeStack put these two buffers on the unsafe stack. The second buffer contains authentication and authorization data. By overflowing into the second buffer, the attacker can gain new privileges. If the buffer contains authentication data, perhaps the attacker can (from this point forward) steal the session from another user. If the buffer contains authorization data, perhaps the attacker can (from this point forward) perform privileged operations. This is where _FORTIFY_SOURCE comes in. If _FORTIFY_SOURCE can prevent the overflow from happening, the attacker cannot steal sessions or perform privileged operations. I intentionally skipped over SSP in this hypothetical. In this particular case, trying to protect the return address pointer saved on the stack is moot. SSP is still useful, but not for this particular hypothetical. I hope this makes sense. Thanks, --=20 Shawn Webb Cofounder / Security Engineer HardenedBSD Tor-ified Signal: +1 303-901-1600 / shawn_webb_opsec.50 https://git.hardenedbsd.org/hardenedbsd/pubkeys/-/raw/master/Shawn_Webb/03A= 4CBEBB82EA5A67D9F3853FF2E67A277F8E1FA.pub.asc On Sun, May 19, 2024 at 02:47:22PM +0000, Pedro Giffuni wrote: > Hmm... well. > In all honesty I understand I am doomed to lose this battle :).FORTIFY_SO= URCE is in linux and in Apple and that weights enough that it had to find i= t's way to FreeBSD sooner or later. Plus I am just not much involved in Fre= eBSD or OSs anymore so I don't feel like stopping other people from doing d= evelopment. > best of lucks! > Pedro. > ps. Just for the reference, the Google guys did develop a document on how= they implemented this for clang on bionic:https://goo.gl/8HS2dW >=20 > On Sunday, May 19, 2024 at 12:13:00 AM GMT-5, Kyle Evans wrote: =20 > =20 > On 5/18/24 23:39, Pedro Giffuni wrote: > > FWIW .. and let me be clear I haven't worked on this in ages and I am= =20 > > not planning to retake this either... > >=20 > > clang just couldn't do the static=A0 fortify_source checks=A0 due to th= e way=20 > > llvm uses an intermediate representation; the size just couldn't be=20 > > handled in the preprocessor. Google did spend some time adding extra=20 > > attributes to clang to improve the debugging and you can see that=20 > > implemented in bionic libc but that was it. musl didn't even try. > >=20 >=20 > Admittedly, I have no idea what you're talking about here; none of this= =20 > implementation requires any knowledge of anything at preproc time.=20 > __builtin_object_size() does the right thing, and the typically=20 > performance critical string/memory ops use __builtin___foo_chk() that do= =20 > successfully get optimized away in the common case to the underlying=20 > foo() call.=A0 This all works very well with clang, I haven't tested it= =20 > under GCC but, as you've noted, would assume that it works at least as we= ll. >=20 > > fortify_source does replace some key libc functions with memory checkin= g=20 > > alternatives and that turns out to be annoying when debugging. In a way= =20 > > it breaks that principle C programmers once had, where developers are= =20 > > expected to know what they are doing, and if the error is caught at=20 > > runtime by the stack protector anyways it ends up being redundant. > > > One more thing about the static checks. Most of the linux distributio= ns > > out there indeed have built their software packages with GCC and=20 > > fortify_source >=3D2. As a consequence, when we ran an exp-run on the= =20 > > ports tree (with GCC), fortify_source didn't find anything: it was=20 > > basically a waste of time. > >=20 > > Another reason for not setting it by default is performance. And here I= =20 > > answer Shawn's comment on why not enable stack-protector-all and=20 > > safestack and fortify_source at the same time: running unnecessary=20 > > checks over and over again wastes energy and can have some performance= =20 > > hit. The later may seem negligible in modern processors, but why do the= m=20 > > if they bring no benefit? (No need to answer ... just left as food for= =20 > > thought) > >=20 > > Pedro. > >=20 > > On Saturday, May 18, 2024 at 09:08:52 PM GMT-5, Kyle Evans=20 > > wrote: > >=20 > >=20 > >=20 > >=20 > > On 5/18/24 20:09, Pedro Giffuni wrote: > >=A0 > (sorry for top posting .. my mailer just sucks) > >=A0 > Hi; > >=A0 > > >=A0 > I used to like the limited static checking FORTIFY_SOURCE provides= and > >=A0 > when I ran it over FreeBSD it did find a couple of minor issues. I= t only > >=A0 > works for GCC though. > >=A0 > > >=20 > > I don't think this is particularly true anymore; I haven't found a case > > yet where __builtin_object_size(3) doesn't give me the correct size > > while GCC did.=A0 I'd welcome counter-examples here, though -- we have > > funding to both finish the project (widen the _FORTIFY_SOURCE net to > > more of libc/libsys) and add tests to demonstrate that it's both > > functional and correct.=A0 It would be useful to also document > > deficiencies in the tests. > >=20 > >=A0 > I guess it doesn't really hurt to have FORTIFY_SOURCE around and N= etBSD > >=A0 > had the least intrusive implementation the last time I checked but= I > >=A0 > would certainly request it should never be activated by default, > >=A0 > specially with clang. The GCC version has seen more development on= glibc > >=A0 > but I still think its a dead end. > >=A0 > > >=20 > > I don't see a compelling reason to avoid enabling it by default; see > > above, the functionality that we need in clang appears to be just fine > > (and, iirc, was also fine when I checked at the beginning of working on > > this in 2021) and it provides useful > >=20 > >=A0 > What I would like to see working on FreeBSD is Safestack as a > >=A0 > replacement for the stack protector, which we were so very slow to= adopt > >=A0 > even when it was originally developed in FreeBSD. I think other pr= ojects > >=A0 > based on FreeBSD (Chimera and hardenedBSD) have been using it but I > >=A0 > don't know the details. > >=A0 > > >=20 > > No comment there, though I think Shawn Webb / HardenedBSD had been > > playing around with SafeStack (and might have enabled it? I haven't > > actually looked in a while now). > >=20 > >=A0 > This is just all my $0.02 > >=A0 > > >=A0 > Pedro. > >=20 > > Thanks, > >=20 > > Kyle Evans > >=20 > >=A0 > > >=A0 > On Saturday, May 18, 2024 at 05:54:42 PM GMT-5, Kyle Evans > >=A0 > > wrote: > >=A0 > > >=A0 > > >=A0 > > >=A0 > > >=A0 > On May 18, 2024 13:42, Pedro Giffuni > > wrote: > >=A0 > > >=A0 >=A0 =A0 Oh no .. please not... > >=A0 > > >=A0 >=A0 =A0 We went into that in a GSoC: > >=A0 > > >=A0 >=20 > > https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions= =20 > > > > >=A0 > > >=A0 > > >=A0 >=A0 =A0 Ultimately it proved to be useless since stack-protector-st= rong. > >=A0 > > >=A0 > > >=A0 > Respectfully, I disagree with your conclusion here: > >=A0 > > >=A0 > 1.) _FORTIFY_SOURCE provides more granular detection of overflow; I > >=A0 > don't have to overflow all the way into the canary at the end of t= he > >=A0 > frame to be detected, so my minor bug now can be caught before som= ething > >=A0 > causes the stack frame to be rearranged and turn it into a security > >=A0 > issue later > >=A0 > > >=A0 > 2.) __builtin_object_size doesn't work on heap objects, but it act= ually > >=A0 > can work on subobjects from a heap allocation (e.g., &foo->name), = so the > >=A0 > coverage extends beyond the stack into starting to detect other ki= nds of > >=A0 > overflow > >=A0 > > >=A0 > While the security value over stack-protector-strong may be margin= al (I > >=A0 > won't debate this specifically), the feature still has value in ge= neral. > >=A0 > > >=A0 > Thanks, > >=A0 > > >=A0 > Kyle Evans > >=A0=20 > =20 --rvc6baalkatqcyi5 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEA6TL67gupaZ9nzhT/y5nonf44foFAmZKQL4ACgkQ/y5nonf4 4fqYPg//X6q7NdewhCWT8sE2tvZSp2rCCMqnTNYN3OqzwVXQeuhuqcypJ3zL8bfS QMCUSvqVxH+YWt9Fg1vyVICW1Ly12nVhm8yhbIzHrZP3OeEJXGNu/sDqwNxDRYza aKlw3ac74dkNJpikpeZn7mMT0iGJpsranm9SyIwshJwxxvF4YT5/Ax4ovxsQspga zIy/LX5Pne3ZSTTKLwxLMns3neD2s9bn3qA9qW3AOaFZcJIt2lPouKzduW/FI4bs 8eVEH7aH/8OjFbWAiE9ZgjWK2rrclNc19HSnE0jw0LxoivmUMNL4DMY4BTAS9BOL RtUR48QbSHVs9GQeRulZYcu2l7hizN7oQMaA6eZcSj1Syy+gWlHa12bEoj5oTCvl JSDPPQv8vMr6x8vmccfhIMVPRZz31gxlNZ5UQDecxBF5CTaNH24nXK8ntYoRxkp+ gzaeuaGjw8Q2Rmltoju8+CrWPdrobP5WdAGznKazNVul2tjLj/F61d3FMxMMwvV7 M+jpOMc6JUdE5BpOXXZiCv/yLKa2RE0FT0xooJM/FlpinodZC3Fopbhd8RmjO1CD giOJYw6cgpZrECLAaVkdX5k/Kom7/35jBTFZs+pqYqV9Iw9L282+2JfNM7g3kPMl 6WEq8COklrpCRPBQu3YfQJ1hnd9gc97UKmAY5BrCsJqt9TGrbwg= =lS9x -----END PGP SIGNATURE----- --rvc6baalkatqcyi5--