Recent suggested code changes to the SFL
"Colestock, Robert" <[email protected]> Mon, 29 Mar 2004 13:47:46 -0500
| Newsgroups | gmane.ietf.sfl |
|---|---|
| Message-ID | <[email protected]> |
This is a multi-part message in MIME format. ------_=_NextPart_001_01C415BE.53D354B0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable John: Sorry about not responding to these earlier, I was no longer subscribed to this list and did not realize it. Thank you for spending the time to investigate these issues and provide this level of detail. =20 * Subject: Memory management problems in SFL 2.3=20 * From: "John Stark" <jas@xxxxxxxxxxxx <mailto:[email protected]> >=20 * Date: Wed, 24 Mar 2004 16:41:02 -0000=20 1. Memory leaks Sign/verify leaks Encrypt/decrypt leaks PKCS#12 "ComputePkcs12MAC(...)" leak <<<< I have performed memory leak testing on sign/verify and encrypt/decrypt and have found no such leaks. Hopefully they will not be present in your tests when you run R2.4. I will be re-checking the memory leak issue before release. I will re-build you test programs to search for memory leaks to be sure these are fixed as well. If I discover any specific fixes here I will e-mail. 2. Memory overrun CryptoPP::X917RNG::X917RNG(CryptoPP::BlockTransformation*, unsigned char const*, unsigned long)+160 =20 <<<<<<FIXED to supply 8 bytes of memory. Odd that this was not found by our MS Windows BoundsChecker. =20 3. Mismatched library calls =20 I have encountered three of these. They are benign with GCC, but might matter on some platforms. =20 a. SNACC::AsnFileSeg::AsnFileSeg(char const*) allocates member m_filename using strdup() (and hence malloc()) but frees it using delete. c. SNACC::ConsStringDeck::~ConsStringDeck() uses delete rather than delete [] to free memory allocated using new[] in SNACC::AsnBuf::GetSeg(long). <<<< FIXED to properly reflect construction of memory. 4. Lack of thread safety =20 <<<< This issue was known at release time. All of our lists, including the SNACC lists, have internal pointers indicating the "Curr()" current value. Threads interfere with this value causing problems. Our solution in the SFL was to simply lock all references to such lists. You have probably discovered a location that was not protected. The upcoming R2.4 release no longer uses lists with internal pointers to "Curr()", we now use the std::list::iterator concept, which provides full thread protection. Unfortunately it means that all referencing applications must convert as well, but it is not difficult. =20 5. Lack of handling of std::exception exceptions thrown by crypto++ =20 <<<< I will add such exception checking to the sm_free3 CTIL to wrap the crypto++ exceptions. Previously this was caught by the upper level application with "catch(...)", but there is no call stack nor any indication of what the failure was. =20 6. Less robust than SFL 2.1 =20 <<< Without specifics this is hard to address. If your high-level application was not catching (...), then perhaps the crashes were due to the Crypto++ decryption failures? The fix for Number 5 in the next release (R4.2) should fix these. Any ASN.1 decode failures are reported directly, so they should not cause a crash/segmentation fault. =20 7. Memory usage processing large messages =20 <<<< This is a know issue, we have not made the SFL fully compatible with file-based processing; it is on our wish-list. Unfortunately the existing SNACC buffer handling requires memory resident data for processing. This has no easy solution. Our hope was to develop a simple stream-based input/output capability, but this changes the entire SNACC API as well as the SNACC compiler generated logic. This is no easy task and is not required by our customer at this time, so it has been difficult to implement thus far. =20 * =20 * Subject: Patch for PKCS#12 files without passwords=20 * From: John Stark <jas@xxxxxxxxxxxx <mailto:[email protected]> >=20 * Date: Fri, 19 Mar 2004 18:36:35 +0000=20 <<<< Added this feature as described, but untested (I will have to create a program to generate a PKCS12 file with no password). =20 =20 =20 =20 * Subject: sm_free3.cpp patch for RSA verify=20 * From: "John Stark" <jas@xxxxxxxxxxxx <mailto:[email protected]> >=20 * Date: Thu, 18 Mar 2004 23:34:58 -0000=20 <<<< Added this feature as described. ------_=_NextPart_001_01C415BE.53D354B0 Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable <html> <head> <meta http-equiv=3DContent-Type content=3D"text/html; = charset=3Dus-ascii"> <meta name=3DGenerator content=3D"Microsoft Word 11 (filtered)"> <style> <!-- /* Font Definitions */ @font-face {font-family:Wingdings; panose-1:5 0 0 0 0 0 0 0 0 0;} /* Style Definitions */ p.MsoNormal, li.MsoNormal, div.MsoNormal {margin:0in; margin-bottom:.0001pt; font-size:12.0pt; font-family:"Times New Roman";} a:link, span.MsoHyperlink {color:blue; text-decoration:underline;} a:visited, span.MsoHyperlinkFollowed {color:purple; text-decoration:underline;} pre {margin:0in; margin-bottom:.0001pt; font-size:10.0pt; font-family:"Courier New";} span.EmailStyle17 {font-family:Arial; color:windowtext;} @page Section1 {size:8.5in 11.0in; margin:1.0in 1.25in 1.0in 1.25in;} div.Section1 {page:Section1;} /* List Definitions */ ol {margin-bottom:0in;} ul {margin-bottom:0in;} --> </style> </head> <body lang=3DEN-US link=3Dblue vlink=3Dpurple> <div class=3DSection1> <p class=3DMsoNormal style=3D'margin-left:.25in'><em><i><font size=3D3 face=3D"Times New Roman"><span = style=3D'font-size:12.0pt;font-style:normal'>John:</span></font></i></em>= </p> <p class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt;font-style:normal'>Sorry about not responding = to these earlier, I was no longer subscribed to this list and did not realize = it. Thank you for spending the time to investigate these issues and provide = this level of detail.</span></font></i></em></p> <p class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt;font-style:normal'> </span></font></i></em= ></p> <ul type=3Ddisc> <li class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt'>Subject</span></font></i></em>: Memory = management problems in SFL 2.3 </li> <li class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt'>From</span></font></i></em>: "John Stark" <<a = href=3D"mailto:[email protected]">jas@xxxxxxxxxxxx</a>> </li> <li class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt'>Date</span></font></i></em>: Wed, 24 Mar = 2004 16:41:02 -0000 </li> </ul> <pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'>1. Memory leaks</span></font></pre><pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'>Sign/verify = leaks</span></font></pre><pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'>Encrypt/decrypt = leaks</span></font></pre><pre><font size=3D2 face=3D"Courier New"><span style=3D'font-size:10.0pt'>PKCS#12 = “ComputePkcs12MAC(…)” leak</span></font></pre> <p class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt;font-style:normal'><<<< I have = performed memory leak testing on sign/verify and encrypt/decrypt and have found no = such leaks. Hopefully they will not be present in your tests when you = run R2.4. I will be re-checking the memory leak issue before = release. I will re-build you test programs to search for memory leaks to be sure = these are fixed as well. If I discover any specific fixes here I will = e-mail.</span></font></i></em></p> <pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'>2. Memory = overrun</span></font></pre><pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'>CryptoPP::X917RNG::X917RNG(CryptoPP::BlockTran= sformation*,</span></font></pre><pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'> unsigned char = const*, unsigned long)+160</span></font></pre><pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'> </span></font></pre><pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'><<<<<<FIXED to supply 8 = bytes of memory. Odd that this was not found by our MS Windows = BoundsChecker.</span></font></pre> <p class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt;font-style:normal'> </span></font></i></em= ></p> <pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'>3. Mismatched library = calls</span></font></pre><pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'> </span></font></pre><pre><font size=3D2 face=3D"Courier New"><span style=3D'font-size:10.0pt'>I have = encountered three of these. They are benign with = GCC,</span></font></pre><pre><font size=3D2 face=3D"Courier New"><span style=3D'font-size:10.0pt'>but might = matter on some platforms.</span></font></pre><pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'> </span></font></pre><pre><font size=3D2 face=3D"Courier New"><span style=3D'font-size:10.0pt'>a. = SNACC::AsnFileSeg::AsnFileSeg(char const*) allocates = member</span></font></pre><pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'> m_filename using strdup() (and = hence malloc()) but frees it</span></font></pre><pre><font size=3D2 face=3D"Courier New"><span style=3D'font-size:10.0pt'> = using delete.</span></font></pre><pre><font size=3D2 face=3D"Courier New"><span style=3D'font-size:10.0pt'>c. = SNACC::ConsStringDeck::~ConsStringDeck() uses delete = rather</span></font></pre><pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'> than delete [] to free memory = allocated using new[] in</span></font></pre><pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'> = SNACC::AsnBuf::GetSeg(long).</span></font></pre> <p class=3DMsoNormal><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt; font-family:"Courier New"'><<<< FIXED to properly reflect = construction of memory.</span></font></p> <pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'>4. Lack of thread = safety</span></font></pre><pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'> </span></font></pre> <p class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt;font-style:normal'><<<< This issue = was known at release time. All of our lists, including the SNACC = lists, have internal pointers indicating the “Curr()” current = value. Threads interfere with this value causing problems. Our solution = in the SFL was to simply lock all references to such lists. You have = probably discovered a location that was not protected. The upcoming R2.4 = release no longer uses lists with internal pointers to “Curr()”, we = now use the std::list::iterator concept, which provides full thread = protection. Unfortunately it means that all referencing applications must convert as = well, but it is not difficult.</span></font></i></em></p> <p class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt;font-style:normal'> </span></font></i></em= ></p> <pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'>5. Lack of handling of std::exception = exceptions thrown by crypto++</span></font></pre><pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'> </span></font></pre> <p class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt;font-style:normal'><<<< I will add = such exception checking to the sm_free3 CTIL to wrap the crypto++ = exceptions. Previously this was caught by the upper level application with = “catch(…)”, but there is no call stack nor any indication of what the failure = was.</span></font></i></em></p> <p class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt;font-style:normal'> </span></font></i></em= ></p> <pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'>6. Less robust than SFL = 2.1</span></font></pre><pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'> </span></font></pre> <p class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt;font-style:normal'><<< Without = specifics this is hard to address. If your high-level application was not = catching (…), then perhaps the crashes were due to the Crypto++ decryption = failures? The fix for Number 5 in the next release (R4.2) should fix these. Any = ASN.1 decode failures are reported directly, so they should not cause a = crash/segmentation fault.</span></font></i></em></p> <p class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt;font-style:normal'> </span></font></i></em= ></p> <pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'>7. Memory usage processing large = messages</span></font></pre><pre><font size=3D2 face=3D"Courier New"><span = style=3D'font-size:10.0pt'> </span></font></pre> <p class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt;font-style:normal'><<<< This is a = know issue, we have not made the SFL fully compatible with file-based = processing; it is on our wish-list. Unfortunately the existing SNACC buffer = handling requires memory resident data for processing. This has no easy = solution. Our hope was to develop a simple stream-based input/output capability, but = this changes the entire SNACC API as well as the SNACC compiler generated = logic. This is no easy task and is not required by our customer at this time, = so it has been difficult to implement thus far.</span></font></i></em></p> <p class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt;font-style:normal'> </span></font></i></em= ></p> <ul type=3Ddisc> <li class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span = style=3D'font-size:12.0pt;font-style:normal'> </span></font></i></em= ></li> <li class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt'>Subject</span></font></i></em>: Patch = for PKCS#12 files without passwords </li> <li class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt'>From</span></font></i></em>: John Stark = <<a href=3D"mailto:[email protected]">jas@xxxxxxxxxxxx</a>> </li> <li class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt'>Date</span></font></i></em>: Fri, 19 Mar = 2004 18:36:35 +0000 </li> </ul> <p class=3DMsoNormal><font size=3D2 face=3DArial><span = style=3D'font-size:10.0pt; font-family:Arial'><<<< Added this feature as described, but untested (I will have to create a program to generate a PKCS12 file with = no password).</span></font></p> <p class=3DMsoNormal><font size=3D2 face=3DArial><span = style=3D'font-size:10.0pt; font-family:Arial'> </span></font></p> <p class=3DMsoNormal><font size=3D2 face=3DArial><span = style=3D'font-size:10.0pt; font-family:Arial'> </span></font></p> <p class=3DMsoNormal><font size=3D2 face=3DArial><span = style=3D'font-size:10.0pt; font-family:Arial'> </span></font></p> <p class=3DMsoNormal><font size=3D2 face=3DArial><span = style=3D'font-size:10.0pt; font-family:Arial'> </span></font></p> <ul type=3Ddisc> <li class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt'>Subject</span></font></i></em>: = sm_free3.cpp patch for RSA verify </li> <li class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt'>From</span></font></i></em>: "John Stark" <<a = href=3D"mailto:[email protected]">jas@xxxxxxxxxxxx</a>> </li> <li class=3DMsoNormal><em><i><font size=3D3 face=3D"Times New = Roman"><span style=3D'font-size:12.0pt'>Date</span></font></i></em>: Thu, 18 Mar = 2004 23:34:58 -0000 </li> </ul> <p class=3DMsoNormal><font size=3D2 face=3DArial><span = style=3D'font-size:10.0pt; font-family:Arial'><<<< Added this feature as = described.</span></font></p> </div> </body> </html> ------_=_NextPart_001_01C415BE.53D354B0--