After adding a line password="supersecret" to /boot/loader.conf OS does not booting unless correct password is given. In older versions of FreeBSD (I was tested 4.11, 5.5, 6.4, 7.4, 8.3) this setting will protect Boot Menu to prevent setting custom options on boot, but not to completely stopping OS booting. This problem affects only FreeBSD 9.0. Fix: Add a line "0 autoboot" as first command in section "check-password" of /boot/check-password.4th: : check-password ( -- ) 0 autoboot \ Exit if a password was not set s" password" getenv dup -1 = if drop exit then begin \ Loop as long as it takes to get the right password s" Password: " \ Output a prompt for a password read \ Read the user's input until Enter 2dup readval readlen @ compare 0= if 2drop exit \ Correct password then \ Bad Password 3000 ms ." loader: incorrect password" 10 emit again \ Not the right password; repeat ; How-To-Repeat: Add a line: password="supersecret" to /boot/loader.conf and reboot the machine. After that you cannot load OS without typing correct password.
Responsible Changed From-To: freebsd-bugs->dteske Bug caused by my commit SVN r222417 -- I'll fix.
On Nov 26, 2012, at 9:11 AM, Alexander Verbod wrote: > Hi Devin! > Hi Alex, > I'm sorry for contacting you directly, but it looks like bug tracking system on FreeBSD doesn't work as expected, a bunch of bug reports not assigned... :( I've taken ownership of PR 170110 for tracking, so now you can submit followups to the PR and I will get the mail. You did the right thing -- file the PR and then send me an e-mail so I can take a look (and as I did, take ownership if appropriate). NOTE: Adding this reply to the PR audit-trail as a followup. > Could you please take a look at this issue: > http://www.freebsd.org/cgi/query-pr.cgi?pr=170110&cat= > The cause for this regression is rooted in a 13-year discrepancy between the loader.conf(5) man-page and the actual functionality w/respect to this "password" setting. loader.conf(5) states the following (both today and ever since SVN r53672, made 13 years ago): password Provides a password to be asked by check-password before execution is allowed to continue. Essentially, when I was rewriting the Forth code (for SVN r222417, made 18 months ago) -- I considered the existing functionality to be broken because it didn't match the documentation (read: in testing, tried as I might, I could not reproduce what the man-page appeared to be saying [above]). The documentation leads one to believe that when a password is set, that password must be entered before anything can continue (booting implied[?]). So, when I was unable to replicate the documented functionality (thinking it should stop even boot), I changed the code to match the documentation (and harmony was achieved -- or so I thought). It's clear to me now that the functionality wasn't broken but instead the documentation was inappropriate. A more appropriate description of the password variable in loader.conf(5) for the past 13 years might instead have been: password Sets a password to be required if autoboot fails for any reason. Nonetheless, I agree that this regression needs to be addressed to prevent POLA. Someone might be astonished if they are using the password feature in 8.x or lower and then they upgrade to 9.0 or 9.1 to find that their system now requires the password to boot (versus only requiring the password if one wants to make changes by attempting to interrupt the autoboot process). The original functionality (despite being badly documented in loader.conf(5)) will need to be restored (and loader.conf(5) updated in the process). > There is already a fix supplied. Thanks, I'll have a look. In addition, man-page updates to check-password.4th(8) and loader.conf(5) will be required. I'm also going to take this opportunity to improve the code a bit if/where possible. Given the nature of the discrepancy that caused this regression, I'd like to take this chance to provide both functionalities as I can see value in both meanings (regardless of whose interpretation is correct). NOTE: One use-case for requiring a password to boot (versus just protecting boot options) is protecting a PXE server that you either want to make private or as a method of preventing accidental destruction of a machine by fully-automated PXE-based installation scripts (much hardware today requiring only a single key at boot time to boot from the network -- F12 for example -- we sometimes want to prevent access to network boot without password). > Password in loader.conf works for decades and now it is broken. 13 years to be exact (SVN r53672), and inappropriately documented in loader.conf(5) for just as long. Given the situation, I think the proper approach would be to (in order): 1. Restore original meaning of password variable (ask for password only if autoboot fails) 2. Update loader.conf(5) to be [more] accurate 3. Create a new variable to track the alternative functionality of not allowing boot to continue until password is entered (I like "boot_password") 4. Update both loader.conf(5) and check-password.4th(8) manuals > Hope it would be patched before 9.1 release. > I'm afraid this report comes much too late for a fix to be included in 9.1-R (*wink*wink*). -- Devin _____________ The information contained in this message is proprietary and/or confidential. If you are not the intended recipient, please: (i) delete the message and all copies; (ii) do not disclose, distribute or use the message in any manner; and (iii) notify the sender immediately. In addition, please be aware that any message addressed to our domain is subject to archiving and review by persons other than the intended recipient. Thank you.
On 11/26/2012 4:40 PM, Devin Teske wrote: > > I've taken ownership of PR 170110 for tracking, so now you can submit > followups to the PR and I will get the mail. I really appreciate that! > > The cause for this regression is rooted in a 13-year discrepancy between > the loader.conf(5) man-page and the actual functionality w/respect to > this "password" setting. > I agree that loader.conf(5) man-page is kind of confusing about this feature. > > Essentially, when I was rewriting the Forth code (for SVN r222417, made > 18 months ago) -- I considered the existing functionality to be broken > because it didn't match the documentation (read: in testing, tried as I > might, I could not reproduce what the man-page appeared to be saying > [above]). The documentation leads one to believe that when a password is > set, that password must be entered before anything can continue (booting > implied[?]). IMHO broken documentation, it is questionable that it will hart someone, but in case of broken functionality this will broke some systems for sure. :) This functionality is in use for many years and is very handy. For example: dedicated server on collocation where a box itself protected physically by a metal jail and only keyboard exposed outside. In case if one have some bad/pry "neighbor" - this feature will protect boot menu. Another example, - ask some technician on another side of the globe to make some basic operation from the boot menu without granting him whole access and disclosing root's password... One more example, - insider in an organization has user access to a server and can prepare and upload some files, then when nobody can see him, reboot a server and boot it with prepared/infected kernel module(s) or gain read access to some area where he shouldn't be... Well there could be a different workflow, but in any case loader.conf->password protect perfectly operations/commands in the boot menu that can disclose internal server's structure and most importantly - gives real access to the files(!!!) and IMHO must be used anywhere where server can be accessed via console by unauthorized people. There's a few commands in the boot menu, but "more, load, show, boot-conf..." is enough to make some malicious actions, so it MUST be protected. > > So, when I was unable to replicate the documented functionality > (thinking it should stop even boot), I changed the code to match the > documentation (and harmony was achieved -- or so I thought). Documentation is a virtual thing, but functionality is real :( A short story: (that actually bring me here) We have some test facility that virtually allocated on VPS(virtual private server) with a provider that doesn't allow to use custom installation, so in case when a virtual machine is somehow broken - the only way to back it to live - it is reset the whole remote virtual machine, include virtual harddrive, or in another word, if one can't repair a system over KVM(or VNC) via primary console - the whole system need to be erased(!!!) and installed from scratch. To test software under FreeBSD 9.1RC3 we did upgrade and applied our automated script that tight security settings and ... The code that perfectly worked before FreeBSD 9.0: echo "-m" >/boot.config; echo -e "\npassword=\"some_password\"\n" >>/boot.loader.local will effectively lock-down VPS forever... include all accumulated data inside that test virtual machine - no access over console, neither over network. So, IMHO it is much more safer to change documentation in this case instead of functionality that is actually very usable. I believe, I personally get this information first from some book about FreeBSD security. > > It's clear to me now that the functionality wasn't broken but instead > the documentation was inappropriate. Yes, it is ! > > Nonetheless, I agree that this regression needs to be addressed to > prevent POLA. Someone might be astonished if they are using the password > feature in 8.x or lower and then they upgrade to 9.0 or 9.1 to find that > their system now requires the password to boot (versus only requiring > the password if one wants to make changes by attempting to interrupt the > autoboot process). Thank you for understanding the issue !!! I wish that PHP developers would respect POLA issues as you do. > I'm also going to take this opportunity to improve the code a bit > if/where possible. May be not an appropriate place to ask about this, but from security point of view, IMHO it would be a great improvement if password that protect boot menu will be kept as some hash instead of clear text. > > Given the nature of the discrepancy that caused this regression, I'd > like to take this chance to provide both functionalities as I can see > value in both meanings (regardless of whose interpretation is correct). > > NOTE: One use-case for requiring a password to boot (versus just > protecting boot options) is protecting a PXE server that you either want > to make private or as a method of preventing accidental destruction of a > machine by fully-automated PXE-based installation scripts (much hardware > today requiring only a single key at boot time to boot from the network > -- F12 for example -- we sometimes want to prevent access to network > boot without password). This would be handy to have both functionalities, I think this feature(lock autoboot) can find its place in embedded hardware, especially with upcoming ARM support, in addition that you described with PXE server. > Given the situation, I think the proper approach would be to (in order): > > 1. Restore original meaning of password variable (ask for password only > if autoboot fails) Let me add: "ask for the password only if autoboot fails or if any key was pressed during countdown period specified by 'autoboot_delay' variable" Attached patch will restore exactly the same functionality as it works for 13 years(bad 13 number I guess :) ) and credit for the patch is going to the original PR submitter > 2. Update loader.conf(5) to be [more] accurate Yes, something like below would be more understandable than it is right now: "[password] Protect boot menu with a password without interrupting auto boot process. The password should be in clear text format. If a password is set, boot menu will not appear until any key is pressed during countdown period specified by 'autoboot_delay' variable or autoboot process fails. In both cases user should provide specified password to be able to access boot menu." > 3. Create a new variable to track the alternative functionality of not > allowing boot to continue until password is entered (I like "boot_password") Base on our experience :) I would call it: "bootlock_password" or "lockdown_password" but you're right, "boot_password" will match other variables from /boot/defaults/loader.conf even if "autoboot_password" IMHO is more descriptive. > 4. Update both loader.conf(5) and check-password.4th(8) manuals Yes, loader.conf(5) is confusing about "password", but check-password.4th(8) is completely misleading. BTW, while we are talking about documentation, there is related "documentation" bug: /boot/defaults/loader.conf assumes that autoboot_delay="NO" will "... disable autobooting", but in fact it doesn't work and completely ignored. Devin, thank you for taking this issue seriously ! Best regards, Alex Verbod
State Changed From-To: open->analyzed We agree there's a problem and it needs to be addressed. Working on a patchset and testing.
On Nov 26, 2012, at 11:14 PM, Alexander Verbod wrote: > BTW, while we are talking about documentation, there is related "document= ation" bug: > /boot/defaults/loader.conf > assumes that >=20 > autoboot_delay=3D"NO" >=20 > will "... disable autobooting", but in fact it doesn't work and completel= y ignored. Can you create a new PR for the above-described issue? Didn't want to lose track of it while working on this PR. --=20 Thanks, Devin _____________ The information contained in this message is proprietary and/or confidentia= l. If you are not the intended recipient, please: (i) delete the message an= d all copies; (ii) do not disclose, distribute or use the message in any ma= nner; and (iii) notify the sender immediately. In addition, please be aware= that any message addressed to our domain is subject to archiving and revie= w by persons other than the intended recipient. Thank you.
On Nov 26, 2012, at 11:14 PM, Alexander Verbod wrote: > On 11/26/2012 4:40 PM, Devin Teske wrote: >> >> The cause for this regression is rooted in a 13-year discrepancy between >> the loader.conf(5) man-page and the actual functionality w/respect to >> this "password" setting. >> > > I agree that loader.conf(5) man-page is kind of confusing about this feature. > > >> >> It's clear to me now that the functionality wasn't broken but instead >> the documentation was inappropriate. > > Yes, it is ! > > >> Nonetheless, I agree that this regression needs to be addressed to >> prevent POLA. Someone might be astonished if they are using the password >> feature in 8.x or lower and then they upgrade to 9.0 or 9.1 to find that >> their system now requires the password to boot (versus only requiring >> the password if one wants to make changes by attempting to interrupt the >> autoboot process). > > Thank you for understanding the issue !!! > I wish that PHP developers would respect POLA issues as you do. > > >> I'm also going to take this opportunity to improve the code a bit >> if/where possible. > > May be not an appropriate place to ask about this, but from security point of view, IMHO it would be a great improvement if password that protect boot menu will be kept as some hash instead of clear text. > Two concerns with that: 1. The size of the implementation for a given hash algorithm 2. The efficiency of said implementation 3. The amount of effort required to integrate said implementation into the FICL layer I'm not saying "no," but rather "not right now" ^_^ There's a high likelihood that this would be a large undertaking. >> Given the nature of the discrepancy that caused this regression, I'd >> like to take this chance to provide both functionalities as I can see >> value in both meanings (regardless of whose interpretation is correct). >> >> NOTE: One use-case for requiring a password to boot (versus just >> protecting boot options) is protecting a PXE server that you either want >> to make private or as a method of preventing accidental destruction of a >> machine by fully-automated PXE-based installation scripts (much hardware >> today requiring only a single key at boot time to boot from the network >> -- F12 for example -- we sometimes want to prevent access to network >> boot without password). > > This would be handy to have both functionalities, I think this feature(lock autoboot) can find its place in embedded hardware, especially with upcoming ARM support, in addition that you described with PXE server. > I indeed was able to fit this into the up-coming patch. >> Given the situation, I think the proper approach would be to (in order): >> >> 1. Restore original meaning of password variable (ask for password only >> if autoboot fails) > > Let me add: "ask for the password only if autoboot fails or if any key was pressed during countdown period specified by 'autoboot_delay' variable" That was implied by "if autoboot fails" (deeming user intervention as failure to complete an autoboot sequence). > Attached patch will restore exactly the same functionality as it works for 13 years(bad 13 number I guess :) ) and credit for the patch is going to the original PR submitter > Well, the original patch submitted was not correct. If applied, it would have wiped out the beastie menu completely (the call to autoboot needs to be _after_ the check for $password else autoboot will always be invoked on i386/amd64 where check-password is invoked every boot regardless of whether $password is set). > >> 2. Update loader.conf(5) to be [more] accurate > > Yes, something like below would be more understandable than it is right now: > "[password] Protect boot menu with a password without interrupting auto boot process. > The password should be in clear text format. > If a password is set, boot menu will not appear until any key is pressed during countdown period specified by 'autoboot_delay' variable or autoboot process fails. In both cases user should provide specified password to be able to access boot menu." > Right on the money. Crediting you with the updated text. >> 3. Create a new variable to track the alternative functionality of not >> allowing boot to continue until password is entered (I like "boot_password") > Base on our experience :) I would call it: "bootlock_password" Sounds good, so it is. >> 4. Update both loader.conf(5) and check-password.4th(8) manuals > Yes, loader.conf(5) is confusing about "password", but check-password.4th(8) is completely misleading. > No worries, I've updated check-password.4th(8) to be more clear. (please see attached patch.txt) > Devin, thank you for taking this issue seriously ! > No problem. Thank you for reviewing and helping refine the fixes. Can you have a look at the attached patch.txt to see if I've addressed everything? (and then some) -- Cheers, Devin _____________ The information contained in this message is proprietary and/or confidential. If you are not the intended recipient, please: (i) delete the message and all copies; (ii) do not disclose, distribute or use the message in any manner; and (iii) notify the sender immediately. In addition, please be aware that any message addressed to our domain is subject to archiving and review by persons other than the intended recipient. Thank you.
On Dec 10, 2012, at 2:52 PM, Devin Teske wrote: > Can you have a look at the attached patch.txt to see if I've addressed ev= erything? (and then some) Don't install it. Found a couple typos that lead to bad situations if you t= ry this code (BTX halt, etc.). Man-page changes need reviewing, but please hold for the completed Forth ch= anges. --=20 Devin _____________ The information contained in this message is proprietary and/or confidentia= l. If you are not the intended recipient, please: (i) delete the message an= d all copies; (ii) do not disclose, distribute or use the message in any ma= nner; and (iii) notify the sender immediately. In addition, please be aware= that any message addressed to our domain is subject to archiving and revie= w by persons other than the intended recipient. Thank you.
On Dec 10, 2012, at 3:01 PM, Devin Teske wrote: > > On Dec 10, 2012, at 2:52 PM, Devin Teske wrote: > >> Can you have a look at the attached patch.txt to see if I've addressed everything? (and then some) > > Don't install it. Found a couple typos that lead to bad situations if you try this code (BTX halt, etc.). > > Man-page changes need reviewing, but please hold for the completed Forth changes. All done final patch for review attached as patch.txt Here's what I tested: password="foo" # Could boot but not get to menu without password bootlock_password="foo" # Could not boot without password bootlock_password="foo" password="bar" # Could not boot without "foo" # Could boot without "bar" but not get into menu without "bar" And of course, neither # Loaded menu and then booted All expected results. Let me know if you have any questions, objections, concerns, trepidations, quivers, etc. -- Cheers, Devin _____________ The information contained in this message is proprietary and/or confidential. If you are not the intended recipient, please: (i) delete the message and all copies; (ii) do not disclose, distribute or use the message in any manner; and (iii) notify the sender immediately. In addition, please be aware that any message addressed to our domain is subject to archiving and review by persons other than the intended recipient. Thank you.
State Changed From-To: analyzed->patched Committed to HEAD as r244158.
State Changed From-To: patched->closed MFC'd to stable/9 with r254109