Page MenuHomeFreeBSD

www/h2o: resolve PR222281 and clean up port
ClosedPublic

Authored by dch on Oct 7 2017, 8:36 PM.

Details

Summary
  • ensure rc_load_config is done before h2o_config is tested
  • pass portlint
  • pass rclint
  • update to 2.2.3 just released

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

dch created this revision.Oct 7 2017, 8:36 PM
dch added a comment.EditedOct 7 2017, 8:38 PM

following warnings are present in many of the ports I've touched.

  • DATADIR seems fine
  • the 2 rclint warnings also are necessary
dch@wintermute /p/f/p/w/h2o> sudo portlint -C
WARN: /projects/freebsd/ports/www/h2o/pkg-plist: [2]: If and only if your port is DATADIR-safe (that is, a user can override DATADIR when building this port and the port will still work correctly) consider using DATADIR macro; if you are unsure if this port is DATADIR-safe, then ignore this warning
WARN: /projects/freebsd/ports/www/h2o/pkg-plist: [3]: If and only if your port is DATADIR-safe (that is, a user can override DATADIR when building this port and the port will still work correctly) consider using DATADIR macro; if you are unsure if this port is DATADIR-safe, then ignore this warning
WARN: /projects/freebsd/ports/www/h2o/pkg-plist: [4]: If and only if your port is DATADIR-safe (that is, a user can override DATADIR when building this port and the port will still work correctly) consider using DATADIR macro; if you are unsure if this port is DATADIR-safe, then ignore this warning
WARN: /projects/freebsd/ports/www/h2o/pkg-plist: [5]: If and only if your port is DATADIR-safe (that is, a user can override DATADIR when building this port and the port will still work correctly) consider using DATADIR macro; if you are unsure if this port is DATADIR-safe, then ignore this warning
WARN: Makefile: possible use of absolute pathname "/var/log/${PORTNAME}...".
0 fatal errors and 5 warnings found.
dch@wintermute /p/f/p/w/h2o> rclint -v files/*in
Checking files/h2o.in
ERROR:root:[29]: Do not clobber blank values for non-mandatory variables 
==> Syntax for variables that are not mandatory is ${var=value};
    including := will override var="" set in rc.conf (man sh).  If
    this variable is mandatory just for this script, then ignore this
    error
ERROR:root:[30]: Do not clobber blank values for non-mandatory variables 
==> Syntax for variables that are not mandatory is ${var=value};
    including := will override var="" set in rc.conf (man sh).  If
    this variable is mandatory just for this script, then ignore this
    error
jrm accepted this revision.Oct 8 2017, 12:32 PM

The order in the Makefile might be tweaked based on Chapter 14. of the Porter's Handbook.

I'll have to check out rclint.

This revision is now accepted and ready to land.Oct 8 2017, 12:32 PM
jrm added inline comments.Oct 8 2017, 1:18 PM
www/h2o/Makefile
5 ↗(On Diff #33803)

DISTVERSION, plus order

22 ↗(On Diff #33803)

To use PORTDOCS, you need to add OPTIONS_DEFINE=DOCS.

48–57 ↗(On Diff #33803)

MRUBY_CMAKE_ON= -DWITH_MRUBY=ON
MRUBY_CMAKE_OFF= -DWITH_MRUBY=OFF
MRUBY_USES= bison
MRUBY_USE_RUBY= yes
MRUBY_VARS= RUBY_NO_RUN_DEPENDS=yes

66–73 ↗(On Diff #33803)

post-install:
${MKDIR} ${STAGEDIR}${ETCDIR} ${STAGEDIR}${H2O_LOGDIR}
${INSTALL_DATA} \

		${FILESDIR}/${PORTNAME}.conf.sample \
		${STAGEDIR}${ETCDIR}/${PORTNAME}.conf.sample

post-install-DOCS-on:
${MKDIR} ${STAGEDIR}${DOCSDIR}
${INSTALL_DATA} ${WRKSRC}/README.md ${STAGEDIR}${DOCSDIR}

jrm added inline comments.Oct 8 2017, 1:47 PM
www/h2o/Makefile
48–57 ↗(On Diff #33803)

Err.. not MRUBY_USE_RUBY= yes, but MRUBY_USE=RUBY

jrm requested changes to this revision.Oct 8 2017, 1:47 PM
This revision now requires changes to proceed.Oct 8 2017, 1:47 PM
jrm added inline comments.Oct 8 2017, 1:59 PM
www/h2o/Makefile
48–57 ↗(On Diff #33803)

Also, I think you can use MRUBY_CMAKE_BOOL=WITH_MRUBY in place of

MRUBY_CMAKE_ON= -DWITH_MRUBY=ON
MRUBY_CMAKE_OFF= -DWITH_MRUBY=OFF
dch marked 5 inline comments as done.Oct 20 2017, 8:20 PM
dch added inline comments.
www/h2o/Makefile
22 ↗(On Diff #33803)

do you mean from a policy perspective so people can disable all DOCS generation, or functionally, like it doesn't work correctly without it? Maybe referring to this?

5.13.1.3. Default Options
These options are always on by default.

DOCS — build and install documentation.

Note:
There is no need to add these to OPTIONS_DEFAULT. To have them active, and show up in the options selection dialog, however, they must be added to OPTIONS_DEFINE.

dch updated this revision to Diff 34196.Oct 20 2017, 8:54 PM
dch marked 3 inline comments as done.
  • include jrm tips
  • update to 2.2.3 incl announced vulns
dch edited the summary of this revision. (Show Details)Oct 20 2017, 8:55 PM
jrm added inline comments.Oct 20 2017, 11:29 PM
www/h2o/Makefile
22 ↗(On Diff #33803)

Yes, policy. I was quoting mat@ from a one of your recent diffs, but maybe it would be clearer to say, it makes sense to add a DOCS option when setting PORTDOCS.

14–19 ↗(On Diff #34196)

Nitpicking: If you want to follow Chapter 14. of the Porter's Handbook, maybe this.

USES= cmake:noninja compiler:c11 cpe perl5 shebangfix ssl
CPE_VENDOR= h2o_project
USE_GITHUB= yes
USE_PERL5= run

SHEBANG_FILES= share/h2o/start_server

40 ↗(On Diff #34196)

No need to add DOCS to OPTIONS_DEFAULT, because, as you wrote above, it will be included by default. You can confirm this by removing it and doing make -VOPTIONS_DEFAULT.

47 ↗(On Diff #34196)

Remove

51 ↗(On Diff #34196)

MRUBY_USE=ruby=yes

dch added a comment.Oct 21 2017, 12:24 AM

here's the vuln.xml diff, passes validation etc:

diff --git a/security/vuxml/vuln.xml b/security/vuxml/vuln.xml
index f6308b403f4d..acdf5fef1032 100644
--- a/security/vuxml/vuln.xml
+++ b/security/vuxml/vuln.xml
@@ -58,6 +58,38 @@ Notes:
   * Do not forget port variants (linux-f10-libxml2, libxml2, etc.)
 -->
 <vuxml xmlns="http://www.vuxml.org/apps/vuxml-1">
+  <vuln vid="10c0fabc-b5da-11e7-816e-00bd5d1fff09">
+    <topic>h2o -- DoS in workers</topic>
+    <affects>
+      <package>
+       <name>h2o</name>
+       <range><lt>2.2.3</lt></range>
+      </package>
+    </affects>
+    <description>
+      <body xmlns="http://www.w3.org/1999/xhtml">
+       <p>Frederik Deweerdt reports:</p>
+       <blockquote cite="https://github.com/h2o/h2o/releases/tag/v2.2.3">
+         <p>Multiple Denial-of-Service vulnerabilities exist in h2o workers -
+         see references for full details.</p>
+       <p>CVE-2017-10868: Worker processes may crash when receiving a request with invalid framing.</p>
+       <p>CVE-2017-10869: The stack may overflow when proxying huge requests.</p>
+       </blockquote>
+      </body>
+    </description>
+    <references>
+      <cvename>CVE-2017-10868</cvename>
+      <cvename>CVE-2017-10869</cvename>
+      <url>https://github.com/h2o/h2o/issues/1459</url>
+      <url>https://github.com/h2o/h2o/issues/1460</url>
+      <url>https://github.com/h2o/h2o/releases/tag/v2.2.3</url>
+    </references>
+    <dates>
+      <discovery>2017-07-19</discovery>
+      <entry>2017-10-17</entry>
+    </dates>
+  </vuln>
+
   <vuln vid="15a62f22-098a-443b-94e2-2d26c375b993">
     <topic>osip -- Improper Restriction of Operations within the Bounds of a Memory Buffer</topic>
     <affects>

Does it need a separate phab diff?

dch added a comment.Oct 21 2017, 12:29 AM

poudriere finised, 10 x64/x86 & 11.1 x64/x86 all happy. I should get around to arm stuff next week :-)

jrm added a comment.Oct 21 2017, 12:33 AM
In D12619#264560, @dch wrote:

poudriere finised, 10 x64/x86 & 11.1 x64/x86 all happy. I should get around to arm stuff next week :-)

Is this with the changes that I posted about an hour ago? Without the change to MRUBY_USE, I think there will be problems (plist issues), because ruby will not have been pulled in.

jrm added a comment.Oct 21 2017, 3:48 AM
In D12619#264559, @dch wrote:

here's the vuln.xml diff, passes validation etc:

...

Does it need a separate phab diff?

It's probably best to create a separate diff, because when you commit, you can reference that diff in the commit log.

dch marked 5 inline comments as done.Oct 21 2017, 8:23 PM
dch added inline comments.
www/h2o/Makefile
51 ↗(On Diff #34196)

This took me a few reads to understand but I've got it now. thanks!

dch marked an inline comment as done.Oct 21 2017, 8:33 PM
dch marked an inline comment as done.
dch updated this revision to Diff 34217.Oct 21 2017, 8:34 PM
  • tidy up MRUBY options handling
  • remove picotls option why did i even add this
  • tweak descriptions
jrm accepted this revision.Oct 21 2017, 9:58 PM

Looks good to me.

This revision is now accepted and ready to land.Oct 21 2017, 9:58 PM
This revision was automatically updated to reflect the committed changes.
mat added inline comments.Oct 26 2017, 2:43 PM
www/h2o/Makefile
22 ↗(On Diff #33803)

It is not so much a policy as a reality, PORTDOCS will not work properly if you do not have a DOCS option.

jrm added inline comments.Oct 26 2017, 3:06 PM
www/h2o/Makefile
22 ↗(On Diff #33803)

Good to know. What specifically will not work properly? In this case, if we remove the DOCS option and move the lines from the post-install-DOCS-on target to the post-install target all seems fine.