<html>
  <head>
    <meta content="text/html; charset=windows-1252"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix">On 09/07/2015 21:52, Karl Vogel wrote:<br>
    </div>
    <blockquote
      cite="mid:20150709195246.GA29553@bsd118.area52.afnoapps.usaf.mil"
      type="cite">
      <blockquote type="cite">
        <blockquote type="cite">
          <pre wrap="">On 08/07/2015 20:23, Salz, Rich wrote:
</pre>
        </blockquote>
      </blockquote>
      <pre wrap="">  > 1. Is there any good reason to remove this code?

R> Yes.  If it's not tested, reviewed, or in general use, then it's
R> more likely to be harmful (source of bugs) than useful.

</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre wrap="">On Wed, 08 Jul 2015 20:47:43 +0200, Jakob Bohm replied:
</pre>
        </blockquote>
      </blockquote>
      <pre wrap="">
J> That's an overly general criteria...

   Nope, Rich is right on the money.</pre>
    </blockquote>
    <tt>You are obviously quoting others without deep understanding</tt><tt>.</tt><br>
    <blockquote
      cite="mid:20150709195246.GA29553@bsd118.area52.afnoapps.usaf.mil"
      type="cite">
      <pre wrap="">

J> To objectively consider the potential harm of rarely used code,
J> one must clearly determine if there is any way this code could be
J> invoked inadvertently or remotely.

   How do stack-smashers work?  Don't they trick a system into running
   part of a program inadvertently, often with elevated privileges?</pre>
    </blockquote>
    <tt>Actually, mostly they work by tricking a system into </tt><tt><br>
    </tt><tt>running code that was *part of* the stack smasher itself.</tt><tt>
      <br>
      Second most popular option is to use parts of the general <br>
      system code (libc etc.) loaded in every process (because <br>
      attackers like their attack code to be reusable across <br>
      different victims).  Reusing part of whichever program or <br>
      library that had the remote code execution flaw is <br>
      typically last on their list, because it is so much more <br>
      work.</tt><br>
    <blockquote
      cite="mid:20150709195246.GA29553@bsd118.area52.afnoapps.usaf.mil"
      type="cite">
      <pre wrap="">

   How many of us build and run OpenSSL using compiler optimization?
   Have a look at <a class="moz-txt-link-freetext" href="http://pdos.csail.mit.edu/~xi/papers/stack-sosp13.pdf">http://pdos.csail.mit.edu/~xi/papers/stack-sosp13.pdf</a>
   From the blurb:

     What if you put security into your code but your compiler took it
     out without you realizing it?  That's exactly what's happening when
     you use most compilers on the market, according to researchers at
     MIT as disclosed in a 2013 paper.

   The authors describe some security operations (null pointer checks,
   buffer overflow safeguards, etc) seen by the compiler as being
   unnecessary, and hence removed.  I don't know that this is actually
   happening anywhere in the codebase, but it's a *big* codebase, and
   that's the problem.
</pre>
    </blockquote>
    <tt>That paper was hopefully a major wake up for compiler <br>
      writers, nothing anyone else can do about (short of <br>
      writing pure assembler or turning off optimizations, <br>
      both very ugly "solutions").</tt><tt><br>
    </tt>
    <blockquote
      cite="mid:20150709195246.GA29553@bsd118.area52.afnoapps.usaf.mil"
      type="cite">
      <pre wrap="">
   How about the NTP reflection attacks we saw recently?  From
   <a class="moz-txt-link-freetext" href="http://www.mail-archive.com/tech@openbsd.org/msg21729.html">http://www.mail-archive.com/tech@openbsd.org/msg21729.html</a>

     [...] openntpd is a modern piece of code <5000 lines long written
     using best known practices of the time, whereas ntp.org's codebase
     is reportedly 100,000 lines of unknown or *largely unused code*,
     poorly smithed in the past when these kinds of programming mistakes
     were not a significant consideration.
</pre>
    </blockquote>
    <tt>Generalization beyond relevance, yes ntpd contains <br>
      lots of hard to fathom code, and yes some of that <br>
      may have been involved in attacks.  But most of the <br>
      recent ntpd related attacks didn't actually involve <br>
      bugs in the code *at all*.<br>
      <br>
      Those were attacks on the protocol and on the <br>
      incompetent ISPs not implementing standard anti-<br>
      spoofing filters.   So by sending a *valid* but <br>
      obscure query with a false return address, people <br>
      got the ntp servers to respond with *valid* larger <br>
      replies to the victims whose address had been <br>
      falsified.  The primary changes added to ntpd were <br>
      to actively detect and block overly frequent info <br>
      queries pretending to be from the same address.<br>
      <br>
    </tt><tt>Openntpd just happened not to support the <br>
      diagnostic protocol</tt><tt> </tt><tt>commands used in the
      attacks, <br>
      it was too simple to fall victim.</tt><tt>  The code in <br>
      question was probably some of the most heavily <br>
      tested in ntpd, since its heaviest users are the <br>
      NTP expert teams diagnosing and fine tuning <br>
      production servers.<br>
    </tt>
    <blockquote
      cite="mid:20150709195246.GA29553@bsd118.area52.afnoapps.usaf.mil"
      type="cite">
      <pre wrap="">
J> For example the heartbeat code was obviously callable from network
J> packets (even if it had no bugs), so needed this kind of cleanup,

   Was this only obvious after the fact?</pre>
    </blockquote>
    <tt>By definition, this code was intended to handle specific </tt><tt><br>
    </tt><tt>network packets and generate responses.  The bug was a </tt><tt><br>
    </tt><tt>massive input validation failure.  The code could *only* </tt><tt><br>
    </tt><tt>be</tt><tt> invoked from the network.</tt><br>
    <blockquote
      cite="mid:20150709195246.GA29553@bsd118.area52.afnoapps.usaf.mil"
      type="cite">
      <pre wrap="">

J> while the original eay DES API is only invokable from code that
J> knows about it, and would thus not need to be removed for lack of
J> use/testing.

   What about Apple's SSL/TLS bug (AKA CVE-2014-1266, or the "goto fail
   bug") in February 2014?  That was caused by unreachable code that
   needed to be reached in order to work properly.  The point is, more
   code == more eyes and mind-share that have to be devoted to finding
   unintended consequences.</pre>
    </blockquote>
    <tt>I have not reviewed that in detail, but it sounds like <br>
      there was a bug in a primary code path, not in a rarely <br>
      invoked separate function.<br>
    </tt>
    <blockquote
      cite="mid:20150709195246.GA29553@bsd118.area52.afnoapps.usaf.mil"
      type="cite">
      <pre wrap="">

   Have a look here for more reasons to trim out old code:
   <a class="moz-txt-link-freetext" href="http://www.techrepublic.com/blog/software-engineer/why-you-need-to-clean-out-dead-code-paths/">http://www.techrepublic.com/blog/software-engineer/why-you-need-to-clean-out-dead-code-paths/</a>
</pre>
    </blockquote>
    <tt>Just some guys opinion on a site that carries all sorts of </tt><tt><br>
    </tt><tt>opinion pieces.</tt><tt>  Not even worth reading.</tt><br>
    <blockquote
      cite="mid:20150709195246.GA29553@bsd118.area52.afnoapps.usaf.mil"
      type="cite">
      <pre wrap="">
   Cliff-notes version:

  * Code changes gets ugly because you are trying to keep orphaned code in
    line with the rest of the system, but there is often no real regression
    testing or anything else.
</pre>
    </blockquote>
    <tt>Applies in general,  but may or may not apply to any <br>
      specific case, therefore must be evaluated on a case-<br>
      by-case basis.</tt><br>
    <blockquote
      cite="mid:20150709195246.GA29553@bsd118.area52.afnoapps.usaf.mil"
      type="cite">
      <pre wrap="">
  * Maintaining code after a long period away from it (or by someone else)
    is very difficult, because no one really knows why a piece of code
    is there, they just know that it is there.
</pre>
    </blockquote>
    <tt>Is equally much an argument not to remove unknown code, <br>
      if you don't understand, you don't know what you are <br>
      breaking.</tt><br>
    <blockquote
      cite="mid:20150709195246.GA29553@bsd118.area52.afnoapps.usaf.mil"
      type="cite">
      <pre wrap="">
  * The code is no longer a faithful representation of the business logic,
    because it contains logic that the specifications and business logic
    are not aware of.
</pre>
    </blockquote>
    <tt>This assumes that there is a specification, *and* that <br>
      this specification does not cover the code in question. </tt><tt><br>
    </tt><tt>Also assumes a completely different world (enterprise <br>
      development as opposed to general purpose development, <br>
      where the term "business logic" is nonsense).<br>
      <br>
      In contrast, the code in question implements an actual <br>
      specification, and is there (amongst others) to exchange <br>
      data with anyone else using that specification.  The <br>
      discussion is that someone wants to stop supporting that <br>
      specification because *he* doesn't know its purpose.<br>
    </tt><tt></tt>
    <blockquote
      cite="mid:20150709195246.GA29553@bsd118.area52.afnoapps.usaf.mil"
      type="cite">
      <pre wrap="">
  * It presents potential security risks, as unmaintained code can be
    reached (especially in Web applications, where tweaking parameters
    may trigger something you never intended).</pre>
    </blockquote>
    <tt>This is not a web application.  This code is not reachable <br>
      except by explicit reference.  It may or may not be reachable <br>
      via a format-detecting data import function or a format-<br>
      selecting output function, in which case it may be debatable <br>
      if it should be demoted to explicit invocation only (as in <br>
      data conversion programs and programs that specifically need <br>
      that format).</tt><br>
    <blockquote
      cite="mid:20150709195246.GA29553@bsd118.area52.afnoapps.usaf.mil"
      type="cite">
      <pre wrap="">

  OpenSSL is a critical part of security in too many places for us to
  take on any unnecessary technical debt.</pre>
    </blockquote>
    <tt>This is a somewhat empty argument as long as no one bothers <br>
      to properly determine if a piece of code is a debt or an <br>
      asset.</tt><br>
    <br>
    <br>
    <pre class="moz-signature" cols="72">Enjoy

Jakob
-- 
Jakob Bohm, CIO, Partner, WiseMo A/S.  <a class="moz-txt-link-freetext" href="http://www.wisemo.com">http://www.wisemo.com</a>
Transformervej 29, 2860 Søborg, Denmark.  Direct +45 31 13 16 10
This public discussion message is non-binding and may contain errors.
WiseMo - Remote Service Management for PCs, Phones and Embedded </pre>
  </body>
</html>