Page MenuHomeFreeBSD
Differential D3933

Add /boot/entropy at install time, and be more careful with permissions
ClosedPublic
Actions

Authored by dteske on Oct 18 2015, 8:10 PM.
Tags
None
Referenced Files
F131721983: D3933.id9492.diff
Fri, Oct 10, 3:40 PM
F131706788: D3933.diff
Fri, Oct 10, 12:35 PM
Unknown Object (File)
Thu, Oct 9, 4:30 AM
Unknown Object (File)
Tue, Oct 7, 6:15 PM
Unknown Object (File)
Thu, Sep 25, 10:44 PM
Unknown Object (File)
Thu, Sep 25, 9:16 PM
Unknown Object (File)
Fri, Sep 19, 7:40 AM
Unknown Object (File)
Fri, Sep 19, 12:31 AM
View All Files
Subscribers
imp
op

Details

Reviewers
jmg
markm
delphij
Group Reviewers
secteam
Summary

Now that /boot/entropy is loaded by loader at boot time, we should generate this file also at install time.

We were also not being explicit about permissions on these files, so make sure that if umask or other things changes, that these will have correct permissions. We should probably also updated the rc.d scripts to do the same incase they are created by hand.

In the case /entropy is created by hand w/ incorrect permissions, no script fixes this.

Related: https://github.com/HardenedBSD/hardenedBSD/issues/167

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 808
Build 808: arc lint + arc unit

Event Timeline

jmg updated this revision to Diff 9488.Oct 18 2015, 8:10 PM
jmg retitled this revision from to Add /boot/entropy at install time, and be more careful with permissions.
jmg updated this object.
jmg edited the test plan for this revision. (Show Details)
jmg added reviewers: delphij, secteam, markm, dteske.
jmg set the repository for this revision to rS FreeBSD src repository - subversion.
Herald added a subscriber: imp. · View Herald TranscriptOct 18 2015, 8:10 PM
markm accepted this revision.Oct 18 2015, 8:41 PM
markm edited edge metadata.

This was on my TODO list. I wish I'd known it was this simple!

usr.sbin/bsdinstall/scripts/entropy
29

The double quotes are superfluous, but not a particularly big deal.

This revision is now accepted and ready to land.Oct 18 2015, 8:41 PM
jmg added a subscriber: op.Oct 19 2015, 12:19 AM

I had thought that we weren't writing out even /entropy files, but @op pointed me to this file.

usr.sbin/bsdinstall/scripts/entropy
29

only if BSDINSTALL_CHROOT is guaranteed to never have spaces in it, and as this is passed in externally, I prefer not to make that assumption.

jmg added a comment.Oct 19 2015, 12:23 AM

Well, I attempted to test this, and I don't believe that this file is being called at the end of install.

I d/l'd FreeBSD-11.0-CURRENT-amd64-20151016-r289420-disc1.iso, and put the new entropy file in the iso manually via:
strings -td FreeBSD-11.0-CURRENT-amd64-20151016-r289420-disc1.iso | grep 'BSDINSTALL_CHROOT/entropy'
echo $((659639709/2048))

Then:
dd if=FreeBSD-11.0-CURRENT-amd64-20151016-r289420-disc1.iso bs=2k iseek=322089 count=1 | hexdump -C | less
to verify that I had the correct file,

and then:
dd if=/tmp/entropy conv=notrunc of=FreeBSD-11.0-CURRENT-amd64-20151016-r289420-disc1.iso bs=2k oseek=322089 count=1

to overwrite the file...

I then used bhyve to do an install:
truncate -s 4g 11snap.img
sh /usr/share/examples/bhyve/vmrun.sh -m 4g -d 11snap.img -I FreeBSD-11.0-CURRENT-amd64-20151016-r289420-disc1.iso 11snap

Then selected all the defaults for an install, and when it was done and rebooted, I booted single user mode, and inspected the fs, and found that neither /entropy nor /boot/entropy were created.

jmg updated this revision to Diff 9492.Oct 19 2015, 1:13 AM
jmg edited edge metadata.
  • add /boot/entropy to install generation.. Also, be extra careful
  • add quotes around the $i for more space protection..

I have done a basic test, and this looks good, I'll commit shortly..

This revision now requires review to proceed.Oct 19 2015, 1:13 AM
jmg added a comment.Oct 19 2015, 1:14 AM

though using dd to put it in an image works, it doesn't change the file size, so I needed to delete some comments, and add white space... once I did that, I was able to verify that the script ran fine...

markm accepted this revision.Oct 19 2015, 7:01 AM
markm edited edge metadata.

I'm happy.

This revision is now accepted and ready to land.Oct 19 2015, 7:01 AM
op added a comment.Oct 19 2015, 9:39 AM
In D3933#81686, @markm wrote:

This was on my TODO list. I wish I'd known it was this simple!

Could you please share your TODO list somewhere?

op added a comment.Oct 19 2015, 9:42 AM
In D3933#81711, @jmg wrote:
  • add /boot/entropy to install generation.. Also, be extra careful
  • add quotes around the $i for more space protection..

I have done a basic test, and this looks good, I'll commit shortly..

We have one more issue in relation to this bug: https://github.com/HardenedBSD/hardenedBSD/commit/73e53f0af947e9ad77653f44ef99a2d66394789c

Possible the current candidate supersede this change. I will test them today.

delphij requested changes to this revision.Oct 29 2015, 4:26 PM
delphij edited edge metadata.

Perhaps we can use umask instead?

Also do we really need bsdinstall entropy when creating a jail? (Note that /etc/rc.d/random have nojail in it, so it's not actually being used there).

usr.sbin/bsdinstall/scripts/entropy
32

Instead of this can we just do umask 077 before the dd and umask 022 after it?

This revision now requires changes to proceed.Oct 29 2015, 4:26 PM
jmg added a comment.Nov 4 2015, 12:00 AM

for jails we don't need to create the file, but I don't see any harm in having this file created... I looked briefly at the jail script, and I don't see a good way to turn it off in the jail case...

usr.sbin/bsdinstall/scripts/entropy
32

hmm.. We should just do umask 077 and not restore after.. I just confirmed w/ dteske that these scripts are executed, not sourced.

jmg updated this revision to Diff 9929.Nov 4 2015, 12:04 AM
jmg edited edge metadata.
  • add /boot/entropy to install generation.. Also, be extra careful
  • add quotes around the $i for more space protection..
  • use umask per delphij...
jmg marked 4 inline comments as done.Nov 4 2015, 12:05 AM

mark various parts done

dteske commandeered this revision.Nov 4 2015, 12:41 AM
dteske edited reviewers, added: jmg; removed: dteske.

Take

dteske updated this revision to Diff 9931.Nov 4 2015, 12:42 AM
dteske edited edge metadata.

Remove unnecessary sub-shell (bsdinstall verbs are exec'd;
no need to protect environment from umask effects).

dteske updated this revision to Diff 9932.Nov 4 2015, 12:44 AM

Refactor

jmg commandeered this revision.Nov 4 2015, 12:49 AM
jmg edited reviewers, added: dteske; removed: jmg.

take back so I can update and confirm my change is same..

jmg updated this revision to Diff 9933.Nov 4 2015, 12:50 AM
  • add /boot/entropy to install generation.. Also, be extra careful
  • add quotes around the $i for more space protection..
  • use umask per delphij...
  • dteske says the parens are not needed.. remove them..
jmg updated this revision to Diff 9934.Nov 4 2015, 12:55 AM
  • add /boot/entropy to install generation.. Also, be extra careful
  • add quotes around the $i for more space protection..
  • use umask per delphij...
  • dteske says the parens are not needed.. remove them..
  • pull in dteske's version..
dteske commandeered this revision.Nov 4 2015, 4:52 AM
dteske edited reviewers, added: jmg; removed: dteske.

So sorry to commandeer again; but I have to fix a mistake in my previous code which was written hastily during the conference

dteske updated this revision to Diff 9936.Nov 4 2015, 4:54 AM

Fix prior refactoring

dteske added a comment.Nov 4 2015, 4:55 AM

Please re-commandeer. Sorry for prior misgiving (refactoring was ``off'' for chown(8) at end of for-loop).

delphij accepted this revision.Nov 4 2015, 4:56 AM
delphij edited edge metadata.

Looks good to me.

This revision is now accepted and ready to land.Nov 4 2015, 4:56 AM
markm accepted this revision.Nov 4 2015, 9:17 AM
markm edited edge metadata.

I'm still Happy!

op added a comment.Nov 20 2015, 4:06 PM

Any update on this change?

delphij added a comment.Nov 20 2015, 4:10 PM

Hrm I thought Devin have already integrated this weeks ago but a quick glance suggests it's not. Devin, looks like all issues was resolved already?

dteske added a comment.Nov 20 2015, 5:01 PM

See prior comment; I asked jmg to re-commandeer for the commit. I only commandeered it so I could update it -- had no intention of truly owning it and wanted jmg to drive it.

But... it looks like jmg is relinquishing to me by way of implicit ETIMEOUT on taking-back.
I'll go ahead and drive this in (most likely by EOD today).

delphij added a comment.Dec 20 2015, 10:29 AM
In D3933#89226, @dteske wrote:

See prior comment; I asked jmg to re-commandeer for the commit. I only commandeered it so I could update it -- had no intention of truly owning it and wanted jmg to drive it.

But... it looks like jmg is relinquishing to me by way of implicit ETIMEOUT on taking-back.
I'll go ahead and drive this in (most likely by EOD today).

Ping? :) @dteske @jmg

dteske closed this revision.Dec 28 2015, 5:37 PM

Committed to HEAD as SVN r292832 -- will merge to stable/{9,10} after MFC timeout

Revision Contents
Changeset List

PathSize
usr.sbin/
bsdinstall/
scripts/
6 lines

Diff 9488

View Options

usr.sbin/bsdinstall/scripts/entropy

Loading...