Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-21 Thread Simon J. Gerraty
Warner Losh  wrote:
> > Officially this code is on the 12.0 target path, it needs
> > to be in the tree sooner where many eyes can work on it.
> >
> 
> I concur here. Let's give it until 12 to get sorted. If it's mostly sorted
> by then, we're good.
> If not we can have the discussion then.
> There's also some manifest signing stuff in the works that was recently
> approved to go in. Simon was talking about that. Maybe that will help fill
> the gaps?

I think so.

The work I've done for loader supports both X.509 and OpenPGP based
signatures, I need to tweak the library a bit so it is useful for
userland app too.

FWIW I'd meant to suggest to steve not to commit the veriexecctl tool
which I think we all agree is useless as is (never used by us).
I believe he'll back that bit out when he can get access to his keys -
he's travelling this week.

Thanks
--sjg
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-21 Thread Warner Losh
On Thu, Jun 21, 2018 at 3:10 PM, Rodney W. Grimes <
free...@pdx.rh.cn85.dnsmgr.net> wrote:

> ...
>
> > > Hi,
> > >
> > > While the code is out of HEAD, it can be posted to a github branch
> > > (or
> > > a projects/ branch if you prefer SVN) for people to try.
> > >
> > > Best regards,
> > > Conrad
> > >
> >
> > Yeah, put it on a branch where it'll get ignored for another two years.
> >
> > If this code had been committed long ago, as it probably should have
> > been, then people would have been playing with it, and by time I needed
> > it a few months ago there would have been all kinds of useful info in
> > mailing lists and blogs about how to set it up and what was good and
> > bad about it and so on. ?Iterative refinement would have been underway.
> >
> > Instead what I found was a bunch of patches and a big steep learning
> > curve with no existing information about using it in the real world.
> > With that info available, I/we ($work) would have been in a position to
> > quickly adopt it and begin contributing to the ongoing refinement.
> > Instead I had to conclude that product deadlines just didn't allow us
> > to even try to get it working from a standing start as first-adopters,
> > so we had to move in a different direction. Even though this is a
> > better solution than what we did, business practicalities will likely
> > prevent us from circling back and changing everything over to this
> > scheme in the future, so now we'll end up never contributing much to
> > this work.
> >
> > So, IMO, all this calling for things to be reverted isn't just
> > inappropriate, it's actively harmful. This is -current where
> > development happens and imperfection is expected. Hiding work in
> > patchsets and reviews and alternate branches and other shadowy places
> > because it's not perfect is just a way of ensuring it never gets any
> > better.
>
> I am with Ian on this one, we have far too much code sitting
> out of tree and rotting faster than anyone can maintain said
> code out of tree, meaning we are litterly cutting our own
> developement efforts off, not at just the foot but up closer
> to the hip.
>
> The veriexec code landed, its in tree, fix it, polish it,
> cut out the ugly bits, but for sake of sanity do not whole
> sale revert it so it can generally rot some more.
>
> Officially this code is on the 12.0 target path, it needs
> to be in the tree sooner where many eyes can work on it.
>

I concur here. Let's give it until 12 to get sorted. If it's mostly sorted
by then, we're good.
If not we can have the discussion then.
There's also some manifest signing stuff in the works that was recently
approved to go in. Simon was talking about that. Maybe that will help fill
the gaps?

Warner
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-21 Thread Rodney W. Grimes
...

> > Hi,
> > 
> > While the code is out of HEAD, it can be posted to a github branch
> > (or
> > a projects/ branch if you prefer SVN) for people to try.
> > 
> > Best regards,
> > Conrad
> > 
> 
> Yeah, put it on a branch where it'll get ignored for another two years.
> 
> If this code had been committed long ago, as it probably should have
> been, then people would have been playing with it, and by time I needed
> it a few months ago there would have been all kinds of useful info in
> mailing lists and blogs about how to set it up and what was good and
> bad about it and so on. ?Iterative refinement would have been underway.
> 
> Instead what I found was a bunch of patches and a big steep learning
> curve with no existing information about using it in the real world.
> With that info available, I/we ($work) would have been in a position to
> quickly adopt it and begin contributing to the ongoing refinement.
> Instead I had to conclude that product deadlines just didn't allow us
> to even try to get it working from a standing start as first-adopters,
> so we had to move in a different direction. Even though this is a
> better solution than what we did, business practicalities will likely
> prevent us from circling back and changing everything over to this
> scheme in the future, so now we'll end up never contributing much to
> this work.
> 
> So, IMO, all this calling for things to be reverted isn't just
> inappropriate, it's actively harmful. This is -current where
> development happens and imperfection is expected. Hiding work in
> patchsets and reviews and alternate branches and other shadowy places
> because it's not perfect is just a way of ensuring it never gets any
> better.

I am with Ian on this one, we have far too much code sitting
out of tree and rotting faster than anyone can maintain said
code out of tree, meaning we are litterly cutting our own
developement efforts off, not at just the foot but up closer
to the hip.

The veriexec code landed, its in tree, fix it, polish it,
cut out the ugly bits, but for sake of sanity do not whole
sale revert it so it can generally rot some more.

Officially this code is on the 12.0 target path, it needs
to be in the tree sooner where many eyes can work on it.
-- 
Rod Grimes rgri...@freebsd.org
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-21 Thread Ian Lepore
On Thu, 2018-06-21 at 19:02 +, Mark Linimon wrote:
> On Thu, Jun 21, 2018 at 12:33:26PM -0600, Ian Lepore wrote:
> > 
> > Hiding work in patchsets and reviews and alternate branches and
> > other
> > shadowy places because it's not perfect
> I do not consider bugzilla and phabricator to be "shadowy places";
> therefore, I reject this argument.
> 
> Although I don't have statistics, AFAICT phabricator patches have a
> better-than-even chance of going in.
> 
> But, in any case, a middle position would have been to commit this to
> a vendor branch and publish instructions on how to grab it from there
> and enable it.
> 
> I understand that -current will have regressions in it.  However, the
> pendulum has recently swung in the direction of "free-for-all".  This
> slows down (e.g.) my own work on -currernt such as testing arm boards
> and trying to fix ports there.  ATM I'm not even *attempting* to do
> the
> latter because I have little faith that any -current I bring in past
> the one I'm locked down to (r333619 May 16 UTC 2018) will do anything
> but burn my time trying to track down regressions.
> 
> tl:dr; I have enough work to do without trying to fix other people's
> stuff.  If that's harsh, so be it.
> 
> mcl
> 

I guess you missed the part about all of this new code being optional
and compiled-in only if you add the new options to your kernel config?

-- Ian

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-21 Thread Mark Linimon
On Thu, Jun 21, 2018 at 12:33:26PM -0600, Ian Lepore wrote:
> Hiding work in patchsets and reviews and alternate branches and other
> shadowy places because it's not perfect

I do not consider bugzilla and phabricator to be "shadowy places";
therefore, I reject this argument.

Although I don't have statistics, AFAICT phabricator patches have a
better-than-even chance of going in.

But, in any case, a middle position would have been to commit this to
a vendor branch and publish instructions on how to grab it from there
and enable it.

I understand that -current will have regressions in it.  However, the
pendulum has recently swung in the direction of "free-for-all".  This
slows down (e.g.) my own work on -currernt such as testing arm boards
and trying to fix ports there.  ATM I'm not even *attempting* to do the
latter because I have little faith that any -current I bring in past
the one I'm locked down to (r333619 May 16 UTC 2018) will do anything
but burn my time trying to track down regressions.

tl:dr; I have enough work to do without trying to fix other people's
stuff.  If that's harsh, so be it.

mcl
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-21 Thread Ian Lepore
On Thu, 2018-06-21 at 11:13 -0700, Conrad Meyer wrote:
> On Thu, Jun 21, 2018 at 9:51 AM, Stephen Kiernan  om> wrote:
> > 
> > On Wed, Jun 20, 2018 at 10:36 PM, Eitan Adler  > > wrote:
> > > 
> > > 
> > > On 19 June 2018 at 20:08, Eitan Adler 
> > > wrote:
> > > > 
> > > > On 19 June 2018 at 18:08, Stephen J. Kiernan  > > > g> wrote:
> > > > > 
> > > > > Added: head/sbin/veriexecctl/Makefile
> > > > > 
> > > > > =
> > > > > =
> > > > > --- /dev/null   00:00:00 1970   (empty, because file is newly
> > > > > added)
> > > > > +++ head/sbin/veriexecctl/Makefile  Wed Jun 20 01:08:54
> > > > > 2018
> > > > > (r335402)
> > > > > @@ -0,0 +1,11 @@
> > > > > +# $FreeBSD$
> > > > > +
> > > > > +PROG=  veriexecctl
> > > > > +MAN=   veriexecctl.8
> > > > > +SRCS=  veriexecctl_parse.y veriexecctl_conf.l veriexecctl.c
> > > > > +
> > > > > +WARNS?=3
> > > > Why are we introducing new code with lower-than-6 warnings ?
> > > In all the commotion about the more important issues this fell
> > > through.  Also its argument parsing appears to not be using
> > > getopt[_long] ?
> > 
> > I replied to this 2 days ago with:
> > "veriexecctl came from NetBSD originally and that is what they had,
> > but I believe it should be able to be bumped up."
> > 
> > However, there has been some discussion about just not putting in
> > veriexecctl for now and wait for some work that Simon Gerraty has
> > been
> > doing, using some of the work for the verified loader, instead.
> > However, it
> > would also mean that in the meantime, there would be nothing
> > available
> > to be able to people to try out veriexec to provide some feedback
> > until
> > that utility was completed and committed.
> Hi,
> 
> While the code is out of HEAD, it can be posted to a github branch
> (or
> a projects/ branch if you prefer SVN) for people to try.
> 
> Best regards,
> Conrad
> 

Yeah, put it on a branch where it'll get ignored for another two years.

If this code had been committed long ago, as it probably should have
been, then people would have been playing with it, and by time I needed
it a few months ago there would have been all kinds of useful info in
mailing lists and blogs about how to set it up and what was good and
bad about it and so on.  Iterative refinement would have been underway.

Instead what I found was a bunch of patches and a big steep learning
curve with no existing information about using it in the real world.
With that info available, I/we ($work) would have been in a position to
quickly adopt it and begin contributing to the ongoing refinement.
Instead I had to conclude that product deadlines just didn't allow us
to even try to get it working from a standing start as first-adopters,
so we had to move in a different direction. Even though this is a
better solution than what we did, business practicalities will likely
prevent us from circling back and changing everything over to this
scheme in the future, so now we'll end up never contributing much to
this work.

So, IMO, all this calling for things to be reverted isn't just
inappropriate, it's actively harmful. This is -current where
development happens and imperfection is expected. Hiding work in
patchsets and reviews and alternate branches and other shadowy places
because it's not perfect is just a way of ensuring it never gets any
better.

-- Ian
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-21 Thread Conrad Meyer
On Thu, Jun 21, 2018 at 9:51 AM, Stephen Kiernan  wrote:
> On Wed, Jun 20, 2018 at 10:36 PM, Eitan Adler  wrote:
>>
>> On 19 June 2018 at 20:08, Eitan Adler  wrote:
>> > On 19 June 2018 at 18:08, Stephen J. Kiernan  wrote:
>> >> Added: head/sbin/veriexecctl/Makefile
>> >>
>> >> ==
>> >> --- /dev/null   00:00:00 1970   (empty, because file is newly added)
>> >> +++ head/sbin/veriexecctl/Makefile  Wed Jun 20 01:08:54 2018
>> >> (r335402)
>> >> @@ -0,0 +1,11 @@
>> >> +# $FreeBSD$
>> >> +
>> >> +PROG=  veriexecctl
>> >> +MAN=   veriexecctl.8
>> >> +SRCS=  veriexecctl_parse.y veriexecctl_conf.l veriexecctl.c
>> >> +
>> >> +WARNS?=3
>> >
>> > Why are we introducing new code with lower-than-6 warnings ?
>>
>> In all the commotion about the more important issues this fell
>> through.  Also its argument parsing appears to not be using
>> getopt[_long] ?
>
>
> I replied to this 2 days ago with:
> "veriexecctl came from NetBSD originally and that is what they had,
> but I believe it should be able to be bumped up."
>
> However, there has been some discussion about just not putting in
> veriexecctl for now and wait for some work that Simon Gerraty has been
> doing, using some of the work for the verified loader, instead. However, it
> would also mean that in the meantime, there would be nothing available
> to be able to people to try out veriexec to provide some feedback until
> that utility was completed and committed.

Hi,

While the code is out of HEAD, it can be posted to a github branch (or
a projects/ branch if you prefer SVN) for people to try.

Best regards,
Conrad
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-21 Thread Stephen Kiernan
On Wed, Jun 20, 2018 at 10:36 PM, Eitan Adler  wrote:

> On 19 June 2018 at 20:08, Eitan Adler  wrote:
> > On 19 June 2018 at 18:08, Stephen J. Kiernan  wrote:
> >> Added: head/sbin/veriexecctl/Makefile
> >> 
> ==
> >> --- /dev/null   00:00:00 1970   (empty, because file is newly added)
> >> +++ head/sbin/veriexecctl/Makefile  Wed Jun 20 01:08:54 2018
> (r335402)
> >> @@ -0,0 +1,11 @@
> >> +# $FreeBSD$
> >> +
> >> +PROG=  veriexecctl
> >> +MAN=   veriexecctl.8
> >> +SRCS=  veriexecctl_parse.y veriexecctl_conf.l veriexecctl.c
> >> +
> >> +WARNS?=3
> >
> > Why are we introducing new code with lower-than-6 warnings ?
>
> In all the commotion about the more important issues this fell
> through.  Also its argument parsing appears to not be using
> getopt[_long] ?
>

I replied to this 2 days ago with:
"veriexecctl came from NetBSD originally and that is what they had,
but I believe it should be able to be bumped up."

However, there has been some discussion about just not putting in
veriexecctl for now and wait for some work that Simon Gerraty has been
doing, using some of the work for the verified loader, instead. However, it
would also mean that in the meantime, there would be nothing available
to be able to people to try out veriexec to provide some feedback until
that utility was completed and committed.

-Steve
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-21 Thread Eitan Adler
On 19 June 2018 at 20:08, Eitan Adler  wrote:
> On 19 June 2018 at 18:08, Stephen J. Kiernan  wrote:
>> Added: head/sbin/veriexecctl/Makefile
>> ==
>> --- /dev/null   00:00:00 1970   (empty, because file is newly added)
>> +++ head/sbin/veriexecctl/Makefile  Wed Jun 20 01:08:54 2018
>> (r335402)
>> @@ -0,0 +1,11 @@
>> +# $FreeBSD$
>> +
>> +PROG=  veriexecctl
>> +MAN=   veriexecctl.8
>> +SRCS=  veriexecctl_parse.y veriexecctl_conf.l veriexecctl.c
>> +
>> +WARNS?=3
>
> Why are we introducing new code with lower-than-6 warnings ?

In all the commotion about the more important issues this fell
through.  Also its argument parsing appears to not be using
getopt[_long] ?


-- 
Eitan Adler
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Stephen Kiernan
(Apologies to cem@ for the duplicate in his inbox, Gmail decided to not
reply-all in my reply.)

On Wed, Jun 20, 2018 at 1:07 PM, Conrad Meyer  wrote:

> Hi Jon,
>
> On Wed, Jun 20, 2018 at 11:58 AM, Jonathan Anderson
>  wrote:
> > On 20 Jun 2018, at 15:32, Jonathan T. Looney wrote:
> >> My reasoning
> >> was fairly simple: when a review has been open for over a year with no
> >> action, it seems like the submitter should be able to commit it without
> >> waiting for more review, if they are confident in their change. I stand
> by
> >> that (and, in fact, would substitute something shorter for "over a
> year").
>
> For this as a question of general process, I think "it depends."  For
> minor fixes?  Of course!  Less than one year.  For minor improvements
> or enhancements?  Also yes, also probably less than 1yr.  For large
> features like this, I think the answer is more nuanced.
>
> Sometimes being in a review for a year means the reviewers are
> overloaded and don't have time.  In that case, maybe they shouldn't
> block progress.  Sometimes being in review for a year just means the
> right reviewers haven't been found.  I think that was more the case
> here.
>
> I would suggest anyone submitting a large feature and not getting
> feedback in a reasonable timeframe to solicit feedback from a wider
> audience much sooner than one year, and also indicate intention to
> merge the unreviewed change with some time window (1-2 weeks?) on
> giving feedback or at least asking for more time to review.
>

That was the problem. I soliticted feedback many times over at least 3
years.
The first set was in my GitHub branch and I had discussed with various
people starting at BSDCan 3 or 4 years ago.

I had even sent an early version of the changes to rwatson (whom I had
previously worked with at TIS/NAI/McAfee) and was provided some
feedback based on a cursory look over of the code, which was integrated
into the branch.

At MeetBSD 2016, I also discussed the code with some people and that
was when it was suggested that I create an account of Phabricator and
put the code up for review. Some members of core and/or the Foundation
had suggested who to talk to get reviews and I had also spoken with
rwatson again to see if he had any suggestions.

Once the code had landed in review, I also talked with people during
vBSDcon last year in an effort to try to find out who else should put eyes
on it. I even talked about it during one of the BoF sessions and pointed
out both my GitHub branch and the set of reviews on Phabricator, which
is when there started to get some additional feedback and/or watchers
on the review.

Also, after I had received my commit bit and the reviews were still
collecting
dust in Phabricator, on numberous occassions I solicited feedback or
suggestions on who to contact on the IRC channel.

I'm guessing that people had not looked at the reviews before suggesting
who to talk to or they did not realize the areas of interest that the
changes
might cover.

I think this experience illustrates the need for some better way for someone
contributing to be able to find out who the correct parties are that need
to be
contacted.

It was 3+ year struggle for me to find people to put eyes on the code and it
was not just a fire it off and wait forever thing. One can only listen to
crickets
in response for requests for help for so long.

-Steve
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Simon J. Gerraty
Conrad Meyer  wrote:
> The signing of manifests does not exist in the patch series committed.

Nor will it, the singing of manifests is a build thing.

But as I mentioned earlier I think the loader verification code can be
leveraged for a verifying userland veriexec tool similar to that in
Junos.

> (If NetBSD does something broken, that is not an excuse to copy it.)

How about we backout the veriexecctl tool - which is the only bit your
comments apply to - and which we do not use.

All the signing etc discussion is orthogonal to the kernel part.

> > A veriexec loader that leverages signed manifests requires some signing
> > infra.  That's a big topic all by itself.
> 
> It *may* require that.  However, even without that, admins could
> reasonably manage their own PKI in some fashion, independent of

There still needs to be a tool that they can use.
The work I've done on the loader should make this simple since it
provides for OpenPGP signatures as well as X.509 based certs.

> FreeBSD's infra.  But it requires the support code to verify
> signatures, as in the "verify" part of veriexec, which is wholly
> absent.

Yes.  As above, reverting the veriexecctl tool would be fine,
I'll commit a proper verifying tool along with the loader bits.
I have to do some tweaking of one of my libs first.

> Again — this is a discussion for arch or phabricator, with the series
> reverted first.

For code that's off by default why is reverting a requirement?

> many other glaring performance problems.  If you care about MAC
> performance in a secure algorithm in 2018, perhaps look at any of
> these great options:
> 
> * SHA-3 (Keccak)
> * Blake2-b
> * Poly1305-{AES,Salsa,ChaCha}

The framework allows folk to add any hashes they like.
For us, anything which is not NIST approved is of little interest.
Obviously many people have the luxury of not haveing to bow to NIST,
so again the framework provides.

> FreeBSD has had this code for 0 years.  It's a novel feature here.
> There is no reason to introduce SHA-1 in novel security features in
> 2018.

As mentioned earlier (in this thread? hard to say), no reason it needs
to be enabled by default.

FreeBSD.org if they are going to sign the packages they ship, need to
make a decision about the hashes they want to support.

> And no, upstreaming the signature verification code is completely
> orthogonal to implementing signing infrastructure.

Not really since one dictates the other.
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Simon J. Gerraty
Xin LI  wrote:
> I do agree with others that SHA-1 support should not be included

It can certainly be disabled by default.

> (unless I have missed something, but I think firmware integrity check
> counts as a "Digital signature" verification, according to SP 800-131A

A "Digital signature" verification is an accepted form of firmware
integrity check, but a simple hash (inlcuding SHA-1) is also acceptible.
We of course perform both - and the Digital signature does *not* use
SHA-1, it has been deprecated for that purpose for some years now.

> "9 Hash algorithms", SHA-1 verification should only be used for legacy
> usage, which does not apply on FreeBSD because this is new feature).

I've managed to get out of having to memorize all those SP's, so will
check with one of the pour souls who still does - as to whether we are
claiming "legacy" status...

> But even that, given the code only impacts systems that have it
> explicitly compiled in, it's reasonable to give the committer more
> time to make further improvements rather than reverting it as a whole
> as this would give the code more exposure.

Indeed - thanks
--sjg
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Conrad Meyer
Hi Jon,

On Wed, Jun 20, 2018 at 11:58 AM, Jonathan Anderson
 wrote:
> On 20 Jun 2018, at 15:32, Jonathan T. Looney wrote:
>> My reasoning
>> was fairly simple: when a review has been open for over a year with no
>> action, it seems like the submitter should be able to commit it without
>> waiting for more review, if they are confident in their change. I stand by
>> that (and, in fact, would substitute something shorter for "over a year").

For this as a question of general process, I think "it depends."  For
minor fixes?  Of course!  Less than one year.  For minor improvements
or enhancements?  Also yes, also probably less than 1yr.  For large
features like this, I think the answer is more nuanced.

Sometimes being in a review for a year means the reviewers are
overloaded and don't have time.  In that case, maybe they shouldn't
block progress.  Sometimes being in review for a year just means the
right reviewers haven't been found.  I think that was more the case
here.

I would suggest anyone submitting a large feature and not getting
feedback in a reasonable timeframe to solicit feedback from a wider
audience much sooner than one year, and also indicate intention to
merge the unreviewed change with some time window (1-2 weeks?) on
giving feedback or at least asking for more time to review.

> So: is there a strong reason why a backout-and-re-commit approach is
> superior to iterating on the code in-tree?

Yeah:

> Any disputed change must be backed out pending resolution of the dispute
> if requested by a maintainer. Security related changes may override a
> maintainer's wishes at the Security Officer's discretion.
>
> This may be hard to swallow in times of conflict (when each side is
> convinced that they are in the right, of course) but a version control
> system makes it unnecessary to have an ongoing dispute raging when it is
> far easier to simply reverse the disputed change, get everyone calmed
> down again and then try to figure out what is the best way to proceed.
> If the change turns out to be the best thing after all, it can be easily
> brought back. If it turns out not to be, then the users did not have to
> live with the bogus change in the tree while everyone was busily
> debating its merits. People very rarely call for back-outs in the
> repository since discussion generally exposes bad or controversial
> changes before the commit even happens, but on such rare occasions the
> back-out should be done without argument so that we can get immediately
> on to the topic of figuring out whether it was bogus or not.

In particular, SHA1 was never the primary reason the backout was
requested, just one design smell in the aggregate.  This change
introduces a security feature that doesn't provide the security
guarantees it claims to.  That should be a major red flag.

All the best,
Conrad
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Jonathan Anderson

On 20 Jun 2018, at 15:32, Jonathan T. Looney wrote:

On Wed, Jun 20, 2018 at 9:49 AM Stephen Kiernan 


wrote:
And I was working on those sets of changes, when work and family 
didn't
steal away time. I was told that some discussion happened at BSDCan 
this
year in such that veriexec should go in as-is so it would be there, 
which

is why

the commit happened (given the review was approved to land back in

January.)

I will readily admit that I was probably the source of this. My 
reasoning

was fairly simple: when a review has been open for over a year with no
action, it seems like the submitter should be able to commit it 
without
waiting for more review, if they are confident in their change. I 
stand by
that (and, in fact, would substitute something shorter for "over a 
year").


([...] I wasn't intending to push Steve to commit
this before he was ready.)


I suspect I was part of that push, and while it's provoked some 
discussion I still think it was the right course of action. Simon and I 
chatted about these changes at BSDCan (starting with my apologies for 
not doing the asked-for review two years ago!) and I suggested that, 
since the changes had been accepted in Phabricator in January and nobody 
had raised any objections since then (see: D85{61,62,75}), it would 
probably be better to commit and iterate than to continue waiting for an 
unspecified number of additional reviews that might never come. Now that 
the code has landed it's sparked some lively discussion about potential 
improvements, but that's a much better situation than the silence Steve 
heard for a long time in Phabricator.


So: is there a strong reason why a backout-and-re-commit approach is 
superior to iterating on the code in-tree? SHA-1 doesn't give me the 
warm fuzzies either, but I don't see why the whole framework has to be 
backed out rather than just receive incremental improvements, e.g., 
disabling or removing SHA-1 while keeping it easy for downstream 
consumers to re-enable/re-add whatever algorithms their customers want.



Jon
--
jonat...@freebsd.org
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Xin LI
On Wed, Jun 20, 2018 at 10:58 AM Jonathan T. Looney  wrote:
>
> On Tue, Jun 19, 2018 at 8:34 PM Conrad Meyer  wrote:
>>
>> Please revert this patchset.  It's not ready.
>
>
> I'm not sure I understand the need to revert the patches. They may need some 
> refinement, but they also do provide some functionality upon which you can 
> build the tooling that Simon discussed.
>
> Unless I missed something, this feature only impacts the system when it is 
> specifically compiled in. In cases like that, I think its reasonable to give 
> the committer some time to refine them in place prior to the code 
> slush/freeze, at which point we can decide what to do.

+1 for all points.

I do agree with others that SHA-1 support should not be included
(unless I have missed something, but I think firmware integrity check
counts as a "Digital signature" verification, according to SP 800-131A
"9 Hash algorithms", SHA-1 verification should only be used for legacy
usage, which does not apply on FreeBSD because this is new feature).
But even that, given the code only impacts systems that have it
explicitly compiled in, it's reasonable to give the committer more
time to make further improvements rather than reverting it as a whole
as this would give the code more exposure.

Cheers,
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Jonathan T. Looney
On Wed, Jun 20, 2018 at 9:49 AM Stephen Kiernan 
wrote:
> And I was working on those sets of changes, when work and family didn't
> steal away time. I was told that some discussion happened at BSDCan this
> year in such that veriexec should go in as-is so it would be there, which
is why
> the commit happened (given the review was approved to land back in
January.)

I will readily admit that I was probably the source of this. My reasoning
was fairly simple: when a review has been open for over a year with no
action, it seems like the submitter should be able to commit it without
waiting for more review, if they are confident in their change. I stand by
that (and, in fact, would substitute something shorter for "over a year").

(Of course, if the submitter has other reasons for delaying the commit,
that's their prerogative, and I wasn't intending to push Steve to commit
this before he was ready.)

Jonathan
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Jonathan T. Looney
On Tue, Jun 19, 2018 at 8:34 PM Conrad Meyer  wrote:

> Please revert this patchset.  It's not ready.
>

I'm not sure I understand the need to revert the patches. They may need
some refinement, but they also do provide some functionality upon which you
can build the tooling that Simon discussed.

Unless I missed something, this feature only impacts the system when it is
specifically compiled in. In cases like that, I think its reasonable to
give the committer some time to refine them in place prior to the code
slush/freeze, at which point we can decide what to do.

Jonathan
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Conrad Meyer
Hi Simon,

Jonathan points out some of my comments were more acerbic than
necessary.  I apologize for that.  I'd like to try to rephrase them in
a more clear way.

On Wed, Jun 20, 2018 at 8:43 AM, Conrad Meyer  wrote:
> On Tue, Jun 19, 2018 at 11:21 PM, Simon J. Gerraty  wrote:
>> As I mentioned in my talk at BSDCan,
>
> (FWIW, I was not at your talk, and it is not a justification for bad
> design or implementation anyway.)

I said before and I'll repeat: I think this design is pretty close to
a reasonable security feature.  I think it currently has a number of
serious — but addressable — flaws, some of which I have tried to
outline.

> ...
> Why is this either necessary or helpful to be in the FreeBSD tree
> as-is?  I don't think it is, and you should revert it.  Please.  I
> don't know if there's a maintainer timeout on this kind of thing, but,
> you are forewarned.

Sorry, this was a poor choice of words.

I mean to say something like: I asked for a revert in an earlier
email, and this reply did not address the primary reason for the
revert, so I am still asking for a revert.  I can do it myself, but I
would like to give the committer the opportunity to do it themselves.

(In private, Stephen has let me know he will do so when he gets back
to his FreeBSD machines, so there's no need for that anyway.)

All the best,
Conrad
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Stephen Kiernan
On Wed, Jun 20, 2018 at 9:30 AM, Conrad Meyer  wrote:
>
> Please look at the actual code size and layout of the sha1 support
> module and tell me that is a burden for Juniper to maintain in their
> downstream tree, rather than just getting angry about the suggestion
> we don't introduce novel, insecurity cryptographic designs.
>

I have no problem removing the SHA1 and RIPEMD implementations.
It's a minor change and very little code for others to have to maintain if
needed.
That was the intention of fingerprint module implementation, to try to make
it easy
to add/remote different algorithms. It could even potentially be done as a
port,
if people are keen to having to pre-load a module to get the support (I
know that
that's a bit of a grey area without a verified loader and secureboot or
similar
functionality to protect integrity.)

I think some of the issue was this code has been looking for eyes to give a
good
look for over 2 years (even before the review was posted, it was available
in my
GitHub branch for at least 1 year prior, if not longer.)

As for some of the other issues, note my comment in the review
https://reviews.freebsd.org/D8554:
"Note I have some updates that I have been working on to handle the
meta-data
store better in SMP environments. So there will be updates to these reviews,
hopefully in the near future, time permitting."

And I was working on those sets of changes, when work and family didn't
steal away time. I was told that some discussion happened at BSDCan this
year in such that veriexec should go in as-is so it would be there, which
is why
the commit happened (given the review was approved to land back in January.)

I suppose I should have just kept with my original intention to fix the
issues and
update the review(s).

Hopefully now it will mean it will get the right eyes on it. I don't
believe I need
to dig up all the e-mail threads and chat logs for IRC where I asked for
help and
was given pointers to folks to contact and we ended up here.

It's a better use of everyone's time to just cool down, back things out,
get new
reviews updated and provide constructive feedback.

So far this experience (I am not pointing at you here) has been a mixed set
of
constructive comments and outright flaming. The latter of which is never
going
to help get the right results and could be one of the reasons that a number
of
folks give up contributing to FreeBSD.

-Steve
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Ian Lepore
On Wed, 2018-06-20 at 09:30 -0700, Conrad Meyer wrote:
> Ian,
> 
> On Wed, Jun 20, 2018 at 8:58 AM, Ian Lepore  wrote:
> > 
> > And I request exactly the opposite: reject the complaining of people
> > who think all the world is a 256-core 5ghz server
> First: no need to be so rude to other committers.  The hyperbole
> doesn't help anyone and doesn't help communicate your point clearly.
> It's just verbal diarrhea.  Please try to be excellent to each other.
> 
> > 
> > and leave in the
> > option to use faster algorithms on real-world hardware used by real-
> > world vendors who need some option other than "rev your hardware every
> > 18 months to keep up with the software or get out of the business."
> Second: You have very much misread my complaints and very much
> misunderstand the cryptographic algorithm landscape if that is your
> conclusion.
> 
> See my earlier email,
> https://lists.freebsd.org/pipermail/svn-src-all/2018-June/166107.html
> ; in particular I would quote:
> 
> > 
> > Performance is absolutely not a reason to use a known weak hash
> > algorithm in 2018, especially when the feature as-committed has so
> > many other glaring performance problems.  If you care about MAC
> > performance in a secure algorithm in 2018, perhaps look at any of
> > these great options:
> > 
> > * Blake2-b
> > * Poly1305-{AES,Salsa,ChaCha}
> > 
> > They have highly efficient software implementations *that smoke SHA-2
> > and don't have SHA-1's known weakness*.  Blake2 is even in-tree
> > already.
> (Removing Keccak, which I had forgotten has crap performance in
> software — mea culpa.)
> 
> > 
> > Stronger algorithm options, yes. Even making stronger options the
> > default, yes.
> "More secure than SHA1" and "faster than SHA1" are *not* mutually exclusive.
> 
> > 
> > But removing viable options which are endorsed by the
> > people who actually set the standards, no.
> No one actually endorses SHA1 in new designs.  No one endorses RIPEMD at all.
> 
> Please look at the actual code size and layout of the sha1 support
> module and tell me that is a burden for Juniper to maintain in their
> downstream tree, rather than just getting angry about the suggestion
> we don't introduce novel, insecurity cryptographic designs.
> 
> Thank you,
> Conrad
> 

I have zero interest in arguing with you (or anybody) about this. I've
expessed my opinion on the subject and have nothing more to say.

-- Ian
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Conrad Meyer
Ian,

On Wed, Jun 20, 2018 at 8:58 AM, Ian Lepore  wrote:
> And I request exactly the opposite: reject the complaining of people
> who think all the world is a 256-core 5ghz server

First: no need to be so rude to other committers.  The hyperbole
doesn't help anyone and doesn't help communicate your point clearly.
It's just verbal diarrhea.  Please try to be excellent to each other.

> and leave in the
> option to use faster algorithms on real-world hardware used by real-
> world vendors who need some option other than "rev your hardware every
> 18 months to keep up with the software or get out of the business."

Second: You have very much misread my complaints and very much
misunderstand the cryptographic algorithm landscape if that is your
conclusion.

See my earlier email,
https://lists.freebsd.org/pipermail/svn-src-all/2018-June/166107.html
; in particular I would quote:

> Performance is absolutely not a reason to use a known weak hash
> algorithm in 2018, especially when the feature as-committed has so
> many other glaring performance problems.  If you care about MAC
> performance in a secure algorithm in 2018, perhaps look at any of
> these great options:
>
> * Blake2-b
> * Poly1305-{AES,Salsa,ChaCha}
>
> They have highly efficient software implementations *that smoke SHA-2
> and don't have SHA-1's known weakness*.  Blake2 is even in-tree
> already.

(Removing Keccak, which I had forgotten has crap performance in
software — mea culpa.)

> Stronger algorithm options, yes. Even making stronger options the
> default, yes.

"More secure than SHA1" and "faster than SHA1" are *not* mutually exclusive.

> But removing viable options which are endorsed by the
> people who actually set the standards, no.

No one actually endorses SHA1 in new designs.  No one endorses RIPEMD at all.

Please look at the actual code size and layout of the sha1 support
module and tell me that is a burden for Juniper to maintain in their
downstream tree, rather than just getting angry about the suggestion
we don't introduce novel, insecurity cryptographic designs.

Thank you,
Conrad
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Conrad Meyer
I want to preface this by saying: this discussion should be happening
in either arch@ or phabricator, after the patch series was temporarily
reverted pending necessary improvements.  I have asked for the series
to be reverted and am still waiting on that.

I am happy to promise to respond promptly to updates on this
particular series so you are not waiting for two years again.

I think it's quite close to something reasonably general, but as-is,
it is worse than useless — it promises a security feature but _does
not deliver it_, which is the "emperor's new clothes" of security.

On Tue, Jun 19, 2018 at 11:21 PM, Simon J. Gerraty  wrote:
> Conrad Meyer  wrote:
>> First and foremost: nothing is actually signed, anywhere.  The
>
> The signing of manifests is external.  The veriexecctl tool is I assume
> a straight copy of what's in NetBSD (I've not looked at it in at least a
> decade).

The signing of manifests does not exist in the patch series committed.

(If NetBSD does something broken, that is not an excuse to copy it.)

> A veriexec loader that leverages signed manifests requires some signing
> infra.  That's a big topic all by itself.

It *may* require that.  However, even without that, admins could
reasonably manage their own PKI in some fashion, independent of
FreeBSD's infra.  But it requires the support code to verify
signatures, as in the "verify" part of veriexec, which is wholly
absent.

> As I mentioned in my talk at BSDCan,

(FWIW, I was not at your talk, and it is not a justification for bad
design or implementation anyway.)

> the signing server we use is open
> source and handles pretty much anything OpenSSL can, as well as OpenPGP
> (and others).

It doesn't really matter if there's a signing server, because nothing
in the patch series *verifies* signatures.

> Tweaking the veriexec loader to only process manifests after
> verification is not hard

Then I even more do not understand why it was not done prior to commit.

> - one of the first things I did when pulling
> veriexec into Junos almost 15 years ago.
>
>> As a corollary to the above, the name "signature file" is used
>> repeatedly in the code, which is misleading.  The file contains hashes
>> (digests), not signatures (MACs).  The file itself is unsigned.
>> Nothing about this has signatures.
>
> NetBSD refers to the hashes as fingerprints - AFAIK that terminology is
> retained.

Fingerprints is fine, "signatures" is not.  "Signatures file" is used
to refer to the manifest file, which only contains fingerprints, in
multiple locations in the code.

> If the term signature is used to refer to anything other than the signed
> manifests that should be fixed.

Should have been fixed prior to commit, yes.

>> There's absolutely no reason to use sha1 or ripemd in new designs.
>> These should be removed.
>
> Sorry I disagree - not with ripem (we never supported that or any of the
> non-NIST approved hashes), but sha1 is still approved by NIST for
> firmware integrity checks - which is what this is, and sha1 is cheaper
> than sha256.

Again — this is a discussion for arch or phabricator, with the series
reverted first.

I reckon you're wrong.  If you're unwilling to trust me, I believe you
should get and accept input from a 3rd party vaguely familiar with
cryptography (maybe cperciva@ or gordon@ or delphij@) before
introducing SHA1 or Ripe-MD in a novel integrity-protection design.

(Some modern Intel and AMD CPUs have intrinsics support for SHA-2, and
on those machines SHA-2 256 is about the same performance as SHA-1.)

Performance is absolutely not a reason to use a known weak hash
algorithm in 2018, especially when the feature as-committed has so
many other glaring performance problems.  If you care about MAC
performance in a secure algorithm in 2018, perhaps look at any of
these great options:

* SHA-3 (Keccak)
* Blake2-b
* Poly1305-{AES,Salsa,ChaCha}

All have highly efficient software implementations that smoke SHA-2
and don't have SHA-1's known weakness.  Blake2 is even in-tree
already.

> As I mentioned in my talk we've included support for sha256 for 10+
> years,

FreeBSD has had this code for 0 years.  It's a novel feature here.
There is no reason to introduce SHA-1 in novel security features in
2018.

> but do not plan to drop sha1 until NIST deprecate it for that
> purpose since boot time is a very sensitive subject for us.

See above.  There are lots of secure hashes faster than SHA-1 without
its known weaknesses.

>> The patchset is littered with style issues.  One fairly obvious issue
>> is mixed indentation styles — some files vary between space and tab
>> indentation from line to line.
>
> You can probably blame me for some of that.
> I only recently found a style9.el that does a half decent job of
> formatting per style(9).

To commit to the tree, you are supposed to be able to get it right, or
at least close to right.

>> Please revert this patchset.  It's not ready.

>> - Maybe use HMACs instead of raw hashes
>
> Why?


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Ian Lepore
On Wed, 2018-06-20 at 08:45 -0700, Conrad Meyer wrote:
> You can keep these poor security modes in your downstream product if
> you want, but don't put them in the tree.
> 

And I request exactly the opposite: reject the complaining of people
who think all the world is a 256-core 5ghz server and leave in the
option to use faster algorithms on real-world hardware used by real-
world vendors who need some option other than "rev your hardware every
18 months to keep up with the software or get out of the business."

Stronger algorithm options, yes. Even making stronger options the
default, yes. But removing viable options which are endorsed by the
people who actually set the standards, no.

- Ian

> On Wed, Jun 20, 2018 at 8:28 AM, Simon J. Gerraty 
> wrote:
> > 
> > Benjamin Kaduk  wrote:
> > > 
> > > With all due respect, NIST is hardly the sole authority on this
> > > topic.
> > True, unless of course you sell to US govt.
> > 
> > > 
> > > With my IETF Security Area Director hat on, any greenfield
> > > proposal coming
> > > in
> > > to the IESG that included sha1 support would get extremely strong
> > > pushback,
> > > and I don't expect that "reducing boot time" would be seen as
> > > sufficiently
> > > compelling.
> > Well that's unfortunate, because reality (and sales teams) can be a
> > pain.   The number of customers who would trade boot time for
> > improved
> > security is depressingly small.
> > 
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Conrad Meyer
You can keep these poor security modes in your downstream product if
you want, but don't put them in the tree.

On Wed, Jun 20, 2018 at 8:28 AM, Simon J. Gerraty  wrote:
> Benjamin Kaduk  wrote:
>> With all due respect, NIST is hardly the sole authority on this topic.
>
> True, unless of course you sell to US govt.
>
>> With my IETF Security Area Director hat on, any greenfield proposal coming
>> in
>> to the IESG that included sha1 support would get extremely strong pushback,
>> and I don't expect that "reducing boot time" would be seen as sufficiently
>> compelling.
>
> Well that's unfortunate, because reality (and sales teams) can be a
> pain.   The number of customers who would trade boot time for improved
> security is depressingly small.
>
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Simon J. Gerraty
Cy Schubert  wrote:
> > The signing of manifests is external.  The veriexecctl tool is I assume
> > a straight copy of what's in NetBSD (I've not looked at it in at least a
> > decade).
> 
> If this is correct, should it not be imported into the vendor branches 
> first?
> 
> What are the criteria to import through the vendor branches v.s. direct 
> import into HEAD? Do I fail to understand a missing piece of 
> information or is there an inconsistency?

AFAIK the key is whether there is an upstream project that will be
tracked, which is not the case here.
The ctl tool is the only bit that bears any relationship to the NetBSD
code - because we never used it.

Once I commit the loader stuff, we can replace the above with something
more useful - can leverage the same library to verify manifest
signatures.
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Simon J. Gerraty
Benjamin Kaduk  wrote:
> With all due respect, NIST is hardly the sole authority on this topic.

True, unless of course you sell to US govt.

> With my IETF Security Area Director hat on, any greenfield proposal coming
> in
> to the IESG that included sha1 support would get extremely strong pushback,
> and I don't expect that "reducing boot time" would be seen as sufficiently
> compelling.

Well that's unfortunate, because reality (and sales teams) can be a
pain.   The number of customers who would trade boot time for improved
security is depressingly small.

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Stephen Kiernan
On Wed, Jun 20, 2018, 6:42 AM Cy Schubert  wrote:

> In message <96021.1529475...@kaos.jnpr.net>, "Simon J. Gerraty" writes:
> > Conrad Meyer  wrote:
> > > First and foremost: nothing is actually signed, anywhere.  The
> >
> > The signing of manifests is external.  The veriexecctl tool is I assume
> > a straight copy of what's in NetBSD (I've not looked at it in at least a
> > decade).
>
> If this is correct, should it not be imported into the vendor branches
> first?
>

It's not a straight copy. But much of it is from the NetBSD version.


> What are the criteria to import through the vendor branches v.s. direct
> import into HEAD? Do I fail to understand a missing piece of
> information or is there an inconsistency?
>

I asked about it at BSDCan 3 years ago and pointed people my github branch
(before review was created) to get opinions on what to do about it (vendor
or directly put it in sbin) and the consensus was to submit it the way I
did.

-Steve
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Simon J. Gerraty
Simon J. Gerraty  wrote:
> > - Maybe sign the source-of-trust file
> 
> We do.  As noted above, we cannot upstream that until FreeBSD has
> suitable signing infra.

It occurred to me, that since I'll be upstreaming a library that
uses BearSSL to do the necessary manifest verification for the loader,
the same could be leveraged for the veriexec loader.

That may impact where the two libs should go - will discuss with imp.
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Cy Schubert
In message <96021.1529475...@kaos.jnpr.net>, "Simon J. Gerraty" writes:
> Conrad Meyer  wrote:
> > First and foremost: nothing is actually signed, anywhere.  The
>
> The signing of manifests is external.  The veriexecctl tool is I assume
> a straight copy of what's in NetBSD (I've not looked at it in at least a
> decade).

If this is correct, should it not be imported into the vendor branches 
first?

What are the criteria to import through the vendor branches v.s. direct 
import into HEAD? Do I fail to understand a missing piece of 
information or is there an inconsistency?


-- 
Cheers,
Cy Schubert 
FreeBSD UNIX: Web:  http://www.FreeBSD.org

The need of the many outweighs the greed of the few.


___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Benjamin Kaduk
On Wed, Jun 20, 2018 at 1:21 AM, Simon J. Gerraty  wrote:

> Conrad Meyer  wrote:
>
> > There's absolutely no reason to use sha1 or ripemd in new designs.
> > These should be removed.
>
> Sorry I disagree - not with ripem (we never supported that or any of the
> non-NIST approved hashes), but sha1 is still approved by NIST for
> firmware integrity checks - which is what this is, and sha1 is cheaper
> than sha256.
>
> As I mentioned in my talk we've included support for sha256 for 10+
> years, but do not plan to drop sha1 until NIST deprecate it for that
> purpose since boot time is a very sensitive subject for us.
>


With all due respect, NIST is hardly the sole authority on this topic.
Over in the IETF, we have the SUIT working group that is even considering
hash-based signatures for firmware updates for post-quantum resistance
(so that devices can be shipped now that have 20-year lifecycles can have
some expectation of retaining the ability to securely take updates over that
lifecycle, admittedly).

With my IETF Security Area Director hat on, any greenfield proposal coming
in
to the IESG that included sha1 support would get extremely strong pushback,
and I don't expect that "reducing boot time" would be seen as sufficiently
compelling.

-Ben
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Stephen Kiernan
On Tue, Jun 19, 2018 at 11:21 PM, Simon J. Gerraty  wrote:

> Conrad Meyer  wrote:
>
> > As a corollary to the above, the name "signature file" is used
> > repeatedly in the code, which is misleading.  The file contains hashes
> > (digests), not signatures (MACs).  The file itself is unsigned.
> > Nothing about this has signatures.
>

I think you mean "signature".
I belive the only place that says "signature file" is the veriexecctl.
And that was in the original sources from NetBSD.

For example, see the currentl veriexecctl in NetBSD and it still uses the
terminology "signature file".

http://cvsweb.netbsd.org/bsdweb.cgi/src/sbin/veriexecctl/veriexecctl.c?rev=1.40

But yes, I agree that it's the wrong term that they're using there.


> NetBSD refers to the hashes as fingerprints - AFAIK that terminology is
> retained.
>
> If the term signature is used to refer to anything other than the signed
> manifests that should be fixed.
>

That was in the veriexec that was the basis for the MAC conversion. I know
I had corrected some before, but probably missed the fact that it was used
in some other places. Easy to happen when you've seen the same code for
a number of years.
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-20 Thread Simon J. Gerraty
Conrad Meyer  wrote:
> First and foremost: nothing is actually signed, anywhere.  The

The signing of manifests is external.  The veriexecctl tool is I assume
a straight copy of what's in NetBSD (I've not looked at it in at least a
decade).

A veriexec loader that leverages signed manifests requires some signing
infra.  That's a big topic all by itself.

As I mentioned in my talk at BSDCan, the signing server we use is open
source and handles pretty much anything OpenSSL can, as well as OpenPGP
(and others).
I also made a point of suggesting that the packages for base system
include signed manifests.

Tweaking the veriexec loader to only process manifests after
verification is not hard - one of the first things I did when pulling
veriexec into Junos almost 15 years ago.

> As a corollary to the above, the name "signature file" is used
> repeatedly in the code, which is misleading.  The file contains hashes
> (digests), not signatures (MACs).  The file itself is unsigned.
> Nothing about this has signatures.

NetBSD refers to the hashes as fingerprints - AFAIK that terminology is
retained.

If the term signature is used to refer to anything other than the signed
manifests that should be fixed.

> There's absolutely no reason to use sha1 or ripemd in new designs.
> These should be removed.

Sorry I disagree - not with ripem (we never supported that or any of the
non-NIST approved hashes), but sha1 is still approved by NIST for
firmware integrity checks - which is what this is, and sha1 is cheaper
than sha256.

As I mentioned in my talk we've included support for sha256 for 10+
years, but do not plan to drop sha1 until NIST deprecate it for that
purpose since boot time is a very sensitive subject for us.

> The patchset is littered with style issues.  One fairly obvious issue
> is mixed indentation styles — some files vary between space and tab
> indentation from line to line.

You can probably blame me for some of that.
I only recently found a style9.el that does a half decent job of
formatting per style(9).

> Please revert this patchset.  It's not ready.
> 
> Some suggestions for a second attempt:
> 
> - Maybe use HMACs instead of raw hashes

Why?

> - Maybe sign the source-of-trust file

We do.  As noted above, we cannot upstream that until FreeBSD has
suitable signing infra.

> - Fix the style issues
> - Fix the compiler warnings at 6
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-19 Thread Cy Schubert
In message 
, Conrad Meyer writes:
> On Tue, Jun 19, 2018 at 6:08 PM, Stephen J. Kiernan  wrot
> e:
> > Author: stevek
> > Date: Wed Jun 20 01:08:54 2018
> > New Revision: 335402
> > URL: https://svnweb.freebsd.org/changeset/base/335402
> >
> > Log:
> >   This application (veriexecctl) handles reading a fingerprints file
>
> Hi,
>
> This patchset needed design and code review prior to commit.  It
> appears to have serious problems.
>
> First and foremost: nothing is actually signed, anywhere.  The
> veriexecctl tool parses and tells the kernel to trust a file input.
> But if we don't trust other files on the filesystem, why do we trust
> that one?  There is no embedded signature mechanism proving the hash
> list file is trustworthy.
>
> As a corollary to the above, the name "signature file" is used
> repeatedly in the code, which is misleading.  The file contains hashes
> (digests), not signatures (MACs).  The file itself is unsigned.
> Nothing about this has signatures.
>
> There's absolutely no reason to use sha1 or ripemd in new designs.
> These should be removed.
>
> The patchset is littered with style issues.  One fairly obvious issue
> is mixed indentation styles — some files vary between space and tab
> indentation from line to line.
>
> Please revert this patchset.  It's not ready.
>
> Some suggestions for a second attempt:
>
> - Maybe use HMACs instead of raw hashes
> - Maybe sign the source-of-trust file
> - Fix the style issues
> - Fix the compiler warnings at 6
  - i386 format issues, build failures in multiple places 


-- 
Cheers,
Cy Schubert 
FreeBSD UNIX: Web:  http://www.FreeBSD.org

The need of the many outweighs the greed of the few.


___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-19 Thread Conrad Meyer
I forgot to mention that the kernel code also introduces severe
performance problems due to really pessimal data structures, small IO
sizes, and problematic locking.

Again: please revert and proceed through a round or two of design review.

Thank you,
Conrad

On Tue, Jun 19, 2018 at 8:33 PM, Conrad Meyer  wrote:
> On Tue, Jun 19, 2018 at 6:08 PM, Stephen J. Kiernan  
> wrote:
>> Author: stevek
>> Date: Wed Jun 20 01:08:54 2018
>> New Revision: 335402
>> URL: https://svnweb.freebsd.org/changeset/base/335402
>>
>> Log:
>>   This application (veriexecctl) handles reading a fingerprints file
>
> Hi,
>
> This patchset needed design and code review prior to commit.  It
> appears to have serious problems.
>
> First and foremost: nothing is actually signed, anywhere.  The
> veriexecctl tool parses and tells the kernel to trust a file input.
> But if we don't trust other files on the filesystem, why do we trust
> that one?  There is no embedded signature mechanism proving the hash
> list file is trustworthy.
>
> As a corollary to the above, the name "signature file" is used
> repeatedly in the code, which is misleading.  The file contains hashes
> (digests), not signatures (MACs).  The file itself is unsigned.
> Nothing about this has signatures.
>
> There's absolutely no reason to use sha1 or ripemd in new designs.
> These should be removed.
>
> The patchset is littered with style issues.  One fairly obvious issue
> is mixed indentation styles — some files vary between space and tab
> indentation from line to line.
>
> Please revert this patchset.  It's not ready.
>
> Some suggestions for a second attempt:
>
> - Maybe use HMACs instead of raw hashes
> - Maybe sign the source-of-trust file
> - Fix the style issues
> - Fix the compiler warnings at 6
>
> Thank you,
> Conrad
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-19 Thread Conrad Meyer
On Tue, Jun 19, 2018 at 6:08 PM, Stephen J. Kiernan  wrote:
> Author: stevek
> Date: Wed Jun 20 01:08:54 2018
> New Revision: 335402
> URL: https://svnweb.freebsd.org/changeset/base/335402
>
> Log:
>   This application (veriexecctl) handles reading a fingerprints file

Hi,

This patchset needed design and code review prior to commit.  It
appears to have serious problems.

First and foremost: nothing is actually signed, anywhere.  The
veriexecctl tool parses and tells the kernel to trust a file input.
But if we don't trust other files on the filesystem, why do we trust
that one?  There is no embedded signature mechanism proving the hash
list file is trustworthy.

As a corollary to the above, the name "signature file" is used
repeatedly in the code, which is misleading.  The file contains hashes
(digests), not signatures (MACs).  The file itself is unsigned.
Nothing about this has signatures.

There's absolutely no reason to use sha1 or ripemd in new designs.
These should be removed.

The patchset is littered with style issues.  One fairly obvious issue
is mixed indentation styles — some files vary between space and tab
indentation from line to line.

Please revert this patchset.  It's not ready.

Some suggestions for a second attempt:

- Maybe use HMACs instead of raw hashes
- Maybe sign the source-of-trust file
- Fix the style issues
- Fix the compiler warnings at 6

Thank you,
Conrad
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r335402 - head/sbin/veriexecctl

2018-06-19 Thread Eitan Adler
On 19 June 2018 at 18:08, Stephen J. Kiernan  wrote:
> Added: head/sbin/veriexecctl/Makefile
> ==
> --- /dev/null   00:00:00 1970   (empty, because file is newly added)
> +++ head/sbin/veriexecctl/Makefile  Wed Jun 20 01:08:54 2018
> (r335402)
> @@ -0,0 +1,11 @@
> +# $FreeBSD$
> +
> +PROG=  veriexecctl
> +MAN=   veriexecctl.8
> +SRCS=  veriexecctl_parse.y veriexecctl_conf.l veriexecctl.c
> +
> +WARNS?=3

Why are we introducing new code with lower-than-6 warnings ?

-- 
Eitan Adler
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


svn commit: r335402 - head/sbin/veriexecctl

2018-06-19 Thread Stephen J. Kiernan
Author: stevek
Date: Wed Jun 20 01:08:54 2018
New Revision: 335402
URL: https://svnweb.freebsd.org/changeset/base/335402

Log:
  This application (veriexecctl) handles reading a fingerprints file
  containing paths, fingerprints, and optional option flags which in turn
  get pushed into the MAC/veriexec meta-data store via the veriexec device.
  
  The format of the fingerprints file is as follows:
  path type fingerprint options
  
  The type of fingerprint supported depends on what MAC/veriexec fingerprint
  modules have been loaded into the system. The veriexecctl application is
  able to determine which ones are available by consulting the
  security.mac.veriexec.algorithms sysctl.
  
  The following options are currently supported in MAC/veriexec and by the
  veriexecctl application:
  
  indirect
If this option is set then the executable cannot be invoked directly, it
can only be used as an interpreter in shell scripts.
  file
Indicates that the fingerprint is associated with a file, not an
executable. Files have their fingerprints verified during open(2) and are
automatically made read only. This option may be used to verify shared
libraries have not been tampered with.
  no_ptrace
If this option is set then the executable cannot be traced with the
ptrace(2) process tracing and debugging call.
  trusted
If this option is set then the executable is allowed to write to the
mem(4) devices. By default, when verified execution is enforced, no
process is allowed to write to the mem(4) devices.
  
  The options are not case sensitive.
  
  Reviewed by:  jtl, wblock
  Obtained from:Juniper Networks, Inc.
  Differential Revision:https://reviews.freebsd.org/D8575

Added:
  head/sbin/veriexecctl/
  head/sbin/veriexecctl/Makefile   (contents, props changed)
  head/sbin/veriexecctl/veriexecctl.8   (contents, props changed)
  head/sbin/veriexecctl/veriexecctl.c   (contents, props changed)
  head/sbin/veriexecctl/veriexecctl_conf.l   (contents, props changed)
  head/sbin/veriexecctl/veriexecctl_parse.y   (contents, props changed)

Added: head/sbin/veriexecctl/Makefile
==
--- /dev/null   00:00:00 1970   (empty, because file is newly added)
+++ head/sbin/veriexecctl/Makefile  Wed Jun 20 01:08:54 2018
(r335402)
@@ -0,0 +1,11 @@
+# $FreeBSD$
+
+PROG=  veriexecctl
+MAN=   veriexecctl.8
+SRCS=  veriexecctl_parse.y veriexecctl_conf.l veriexecctl.c
+
+WARNS?=3
+
+LIBADD=l
+
+.include 

Added: head/sbin/veriexecctl/veriexecctl.8
==
--- /dev/null   00:00:00 1970   (empty, because file is newly added)
+++ head/sbin/veriexecctl/veriexecctl.8 Wed Jun 20 01:08:54 2018
(r335402)
@@ -0,0 +1,121 @@
+.\" $NetBSD: veriexecctl.8,v 1.5 2004/03/06 23:40:13 wiz Exp $
+.\"
+.\" Copyright (c) 1999
+.\"Brett Lymn - bl...@baea.com.au, brett_l...@yahoo.com.au
+.\"
+.\" This code is donated to The NetBSD Foundation by the author.
+.\"
+.\" Redistribution and use in source and binary forms, with or without
+.\" modification, are permitted provided that the following conditions
+.\" are met:
+.\" 1. Redistributions of source code must retain the above copyright
+.\"notice, this list of conditions and the following disclaimer.
+.\" 2. Redistributions in binary form must reproduce the above copyright
+.\"notice, this list of conditions and the following disclaimer in the
+.\"documentation and/or other materials provided with the distribution.
+.\" 3. The name of the Author may not be used to endorse or promote
+.\"products derived from this software without specific prior written
+.\"permission.
+.\"
+.\" THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND
+.\" ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+.\" IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+.\" ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR BE LIABLE
+.\" FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+.\" DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+.\" OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+.\" HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+.\" LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+.\" SUCH DAMAGE.
+.\"
+.\" $FreeBSD$
+.\"
+.Dd June 19, 2018
+.Dt VERIEXECCTL 8
+.Os
+.Sh NAME
+.Nm veriexecctl
+.Nd load verified exec fingerprints
+.Sh SYNOPSIS
+.Nm
+.Ar fingerprints
+.Sh DESCRIPTION
+The
+.Nm
+command loads an in-kernel metadata store of the fingerprints
+given in the
+.Ar fingerprints
+file.
+The kernel can then calculate the fingerprint of programs at time of
+execution or files that are opened with
+.Dv O_VERIFY
+and verify