5 ms·

Corrected headline: OpenBSD's new, untested file(1) implementation. The only thing "safe" here is that the author was completely unharmed by the unit tests th

Corrected headline: OpenBSD's new, untested file(1) implementation. The only thing "safe" here is that the author was completely unharmed by the unit tests that he failed to write. Where I work 2500 lines of new C code without a single line of tests would be laughed off the code review system and then probably revisited around performance review time. Heroic programming is how we got to where we are today, running the world on gigantic piles of untested gotos and question mallocs and frees. There are no real heroes in programming, only people who haven't yet figured out how to write tests.

brohee 12y ago
Considering file is supposed to be invoked on unknown files, I fail to see how tests other than fuzzing would give any assurance security wise. A very anal code review on the other hand... And the OpenBSD people are very aware of fuzzing, this new implementation of file is a direct reaction to Michal Zalewski findings...
- randomfool 12y ago
  Fuzzing is like rolling dice- there's only a chance it'll find the lurking issues. Unit tests enforce contracts and assist future maintenance. I sincerely hope the lack of unit tests is because it's covered by existing tests.
  - e12e 12y ago
    Well, with tools like AFL (American Fuzzy Loop) it is more like rolling dice to find a path through a maze of numbered doors - adjusting the dice rolled (6-sided, 2-sided ...) based on how far one has come, and how many doors there is to choose from... ok, it's not really like rolling dice any more at all. Did you read: http://lcamtuf.blogspot.no/2014/11/pulling-jpegs-out-of-thin-air.html http://lcamtuf.blogspot.no/2014/11/pulling-jpegs-out-of-thin...
    - Animats 12y ago
      "American Fuzzy Lop" - it's named after a breed of rabbit where the ears sag down.
      - e12e 12y ago
        And here I thought it was named after some obscure cereal brand ;-)
- cnst 12y ago
  Interestingly, the old file(1) was also written by an OpenBSD developer -- Ian Darwin of Toronto. I met him at a BSDCan, nice chap. http://mdoc.su/f/file.1 http://mdoc.su/f/file.1 http://darwinsys.com/file/ http://darwinsys.com/file/ Apparently, Ian no longer considers himself a maintainer, devoting such title to Christos Zoulas of NetBSD and Two Sigma.
- thrownaway2424 12y ago
  A good and systematic approach to incorporate fuzzing findings would be as follows: 1) Identify a harmful input through fuzzing 2) Reduce the input to minimal testcase 3) Contribute testcase and fix to existing code Note the absence of "rewrite the whole thing without tests" in this process. From-scratch rewrites that may or may not have fewer bugs than their predecessors are known as CADT. http://www.jwz.org/doc/cadt.html http://www.jwz.org/doc/cadt.html
  - userbinator 12y ago
    CADT refers mostly to rewrites in which lots of new features and complexity are being added - the most likely sources of bugs - not to ones like this that go in the opposite direction.
    - acqq 12y ago
      Let me quote from the source of the CADT term: http://www.jwz.org/doc/cadt.html http://www.jwz.org/doc/cadt.html "This is, I think, the most common way for my bug reports to open source software projects to ever become closed. I report bugs; they go unread for a year, sometimes two; and then (surprise!) that module is rewritten from scratch -- and the new maintainer can't be bothered to check whether his new version has actually solved any of the known problems that existed in the previous version." The OP rightly points that there are no known tests that demonstrate the known problems in the older implementations and also no proofs that the new implementation passes them.
      - raverbashing 12y ago
        Don't compare the Gnome development process with the OpenBSD development process. They are very different. Especially one being a core utility another a Desktop Environment. I agree with that critique of the Gnome development process, but the OpenBSD development process prides itself on security and that's a first commit, new changes are being added (as it can be seen by the log of that file - http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/file/file.c?sortby=date http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/file/fi... )
- perlgeek 12y ago
  Security isn't the only concern. What about correctness? Tests could help there. I'm sure I'll manage to write a secure file(1) implementation, if it's not required to produce correct output.
nossabova 12y ago
I was under the impression that OpenBSD's unit testing files reside in the src/regress directory. Here is the relevant directory for file: http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/regress/usr.bin/file/ http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/regress/usr.bin... The files in that directory sure look like tests to me. Am I missing something obvious?
- acqq 12y ago
  Yes, the obvious thing you miss is that according to the OP the unit tests should contain the tests for the bugs detected by recent third-party fuzzing and the "file" should contain the fix to that. Instead, there most recent test is 6 years old but there's the whole brand new implementation of the whole "file" utility. It does sound CADT-y. http://www.jwz.org/doc/cadt.html http://www.jwz.org/doc/cadt.html "It hardly seems worth even having a bug system if the frequency of from-scratch rewrites always outstrips the pace of bug fixing. Why not be honest and resign yourself to the fact that version 0.8 is followed by version 0.8, which is then followed by version 0.8? But that's what happens when there is no incentive for people to do the parts of programming that aren't fun. Fixing bugs isn't fun; going through the bug list isn't fun; but rewriting everything from scratch is fun (because "this time it will be done right", ha ha) and so that's what happens, over and over again." Specifically, why didn't somebody just remove the ELF part from the existing code, if the goal was "reducing the attack surface by removing the features?" (which can be a reasonable strategy for increasing the security).
  - nossabova 12y ago
    I agree with you that more specific tests should be included for the recent fuzzing results, but the great-grandparent comment seems to suggest that there are no tests at all, which is clearly not true. Instead there most recent test is 6 years old I don't feel that this detail is relevant. Because file is a basic utility, and its function is not supposed to change much over time, a test written 6 years ago for file should still be relevant to file today. Specifically, why didn't somebody just remove the ELF part from the existing code I think this was not done because much of the file code followed outdated coding conventions, and would have to be replaced anyways. Take a look at this diff comparison of file.h: http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/file/file.h.diff?r1=1.24&r2=1.25&f=h http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/file/fi...
raverbashing 12y ago
> untested file(1) implementation Completely unfounded assumption. > the author was completely unharmed by the unit tests that he failed to write That's always valid, having written unit tests or not, and a lot of unit test lovers write thousands of irrelevant unit tests that let bugs slide > Where I work 2500 lines of new C code without a single line of tests would be laughed off the code review system A lot of software has been written without unit tests which doesn't mean they don't work (as opposed to what detractors might say). In fact there are a lot of them you're using right now. > There are no real heroes in programming, only people who haven't yet figured out how to write tests. Yes, sure, "Unit tests are the only true way", not to mention unit tests in C are much LESS frequently possible, especially in low level stuff. I just find risible how people accepted this unit test fetichism without questioning. Uncle Bob fans always makes me laugh
- deleted 12y ago
  [deleted]
- bandrami 12y ago
  Wait five minutes and it will be the next fad somebody is complaining about. Another plus of OpenBSD: a largely fad-free experience.

brohee 12y ago

Considering file is supposed to be invoked on unknown files, I fail to see how tests other than fuzzing would give any assurance security wise. A very anal code review on the other hand... And the OpenBSD people are very aware of fuzzing, this new implementation of file is a direct reaction to Michal Zalewski findings...

randomfool 12y ago
Fuzzing is like rolling dice- there's only a chance it'll find the lurking issues. Unit tests enforce contracts and assist future maintenance. I sincerely hope the lack of unit tests is because it's covered by existing tests.
- e12e 12y ago
  Well, with tools like AFL (American Fuzzy Loop) it is more like rolling dice to find a path through a maze of numbered doors - adjusting the dice rolled (6-sided, 2-sided ...) based on how far one has come, and how many doors there is to choose from... ok, it's not really like rolling dice any more at all. Did you read: http://lcamtuf.blogspot.no/2014/11/pulling-jpegs-out-of-thin-air.html http://lcamtuf.blogspot.no/2014/11/pulling-jpegs-out-of-thin...
  - Animats 12y ago
    "American Fuzzy Lop" - it's named after a breed of rabbit where the ears sag down.
    - e12e 12y ago
      And here I thought it was named after some obscure cereal brand ;-)
cnst 12y ago
Interestingly, the old file(1) was also written by an OpenBSD developer -- Ian Darwin of Toronto. I met him at a BSDCan, nice chap. http://mdoc.su/f/file.1 http://mdoc.su/f/file.1 http://darwinsys.com/file/ http://darwinsys.com/file/ Apparently, Ian no longer considers himself a maintainer, devoting such title to Christos Zoulas of NetBSD and Two Sigma.
thrownaway2424 12y ago
A good and systematic approach to incorporate fuzzing findings would be as follows: 1) Identify a harmful input through fuzzing 2) Reduce the input to minimal testcase 3) Contribute testcase and fix to existing code Note the absence of "rewrite the whole thing without tests" in this process. From-scratch rewrites that may or may not have fewer bugs than their predecessors are known as CADT. http://www.jwz.org/doc/cadt.html http://www.jwz.org/doc/cadt.html
- userbinator 12y ago
  CADT refers mostly to rewrites in which lots of new features and complexity are being added - the most likely sources of bugs - not to ones like this that go in the opposite direction.
  - acqq 12y ago
    Let me quote from the source of the CADT term: http://www.jwz.org/doc/cadt.html http://www.jwz.org/doc/cadt.html "This is, I think, the most common way for my bug reports to open source software projects to ever become closed. I report bugs; they go unread for a year, sometimes two; and then (surprise!) that module is rewritten from scratch -- and the new maintainer can't be bothered to check whether his new version has actually solved any of the known problems that existed in the previous version." The OP rightly points that there are no known tests that demonstrate the known problems in the older implementations and also no proofs that the new implementation passes them.
    - raverbashing 12y ago
      Don't compare the Gnome development process with the OpenBSD development process. They are very different. Especially one being a core utility another a Desktop Environment. I agree with that critique of the Gnome development process, but the OpenBSD development process prides itself on security and that's a first commit, new changes are being added (as it can be seen by the log of that file - http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/file/file.c?sortby=date http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/file/fi... )
perlgeek 12y ago
Security isn't the only concern. What about correctness? Tests could help there. I'm sure I'll manage to write a secure file(1) implementation, if it's not required to produce correct output.

nossabova 12y ago

I was under the impression that OpenBSD's unit testing files reside in the src/regress directory. Here is the relevant directory for file: http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/regress/usr.bin/file/ http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/regress/usr.bin... The files in that directory sure look like tests to me. Am I missing something obvious?

acqq 12y ago
Yes, the obvious thing you miss is that according to the OP the unit tests should contain the tests for the bugs detected by recent third-party fuzzing and the "file" should contain the fix to that. Instead, there most recent test is 6 years old but there's the whole brand new implementation of the whole "file" utility. It does sound CADT-y. http://www.jwz.org/doc/cadt.html http://www.jwz.org/doc/cadt.html "It hardly seems worth even having a bug system if the frequency of from-scratch rewrites always outstrips the pace of bug fixing. Why not be honest and resign yourself to the fact that version 0.8 is followed by version 0.8, which is then followed by version 0.8? But that's what happens when there is no incentive for people to do the parts of programming that aren't fun. Fixing bugs isn't fun; going through the bug list isn't fun; but rewriting everything from scratch is fun (because "this time it will be done right", ha ha) and so that's what happens, over and over again." Specifically, why didn't somebody just remove the ELF part from the existing code, if the goal was "reducing the attack surface by removing the features?" (which can be a reasonable strategy for increasing the security).
- nossabova 12y ago
  I agree with you that more specific tests should be included for the recent fuzzing results, but the great-grandparent comment seems to suggest that there are no tests at all, which is clearly not true. Instead there most recent test is 6 years old I don't feel that this detail is relevant. Because file is a basic utility, and its function is not supposed to change much over time, a test written 6 years ago for file should still be relevant to file today. Specifically, why didn't somebody just remove the ELF part from the existing code I think this was not done because much of the file code followed outdated coding conventions, and would have to be replaced anyways. Take a look at this diff comparison of file.h: http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/file/file.h.diff?r1=1.24&r2=1.25&f=h http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/file/fi...

raverbashing 12y ago

> untested file(1) implementation Completely unfounded assumption. > the author was completely unharmed by the unit tests that he failed to write That's always valid, having written unit tests or not, and a lot of unit test lovers write thousands of irrelevant unit tests that let bugs slide > Where I work 2500 lines of new C code without a single line of tests would be laughed off the code review system A lot of software has been written without unit tests which doesn't mean they don't work (as opposed to what detractors might say). In fact there are a lot of them you're using right now. > There are no real heroes in programming, only people who haven't yet figured out how to write tests. Yes, sure, "Unit tests are the only true way", not to mention unit tests in C are much LESS frequently possible, especially in low level stuff. I just find risible how people accepted this unit test fetichism without questioning. Uncle Bob fans always makes me laugh

deleted 12y ago
[deleted]
bandrami 12y ago
Wait five minutes and it will be the next fad somebody is complaining about. Another plus of OpenBSD: a largely fad-free experience.