<headius[m]>
so there's two security-related PRs, the cherry-picked URLClassLoader workaround, and RubyGems update outstanding right now I believe
Antiarc has joined #jruby
<headius[m]>
the webrick security issue is out there too but I am not sure how to proceed... CRuby's shipping webrick doesn't match any released version
<enebo[m]>
yeah I saw that above
<enebo[m]>
My first comment is: Does anyone use webrick in production?
<enebo[m]>
Which is formed as a question but the statement part is clear :)
<headius[m]>
well, that's a really good point
<headius[m]>
I think that means either leaving it or just bumping all branches to 1.6.0 would both be pretty mundane changes
<enebo[m]>
My long term response is once we get clarification we can do a security release if we feel we should but I do not think we are being too unsafe since I do not think anyone uses webrick over puma
<enebo[m]>
We could opt to do simple workaround fixes as well but it is just propagating the original issue
<enebo[m]>
but it is also an option
<enebo[m]>
I did not really see all of the CVE errors other than numeric hashing and tainting / size limits of Pack
<enebo[m]>
All of those are in though so I don't know which ones are not
<enebo[m]>
Actually the taint one is not right?
<headius[m]>
the main webrick issue was that you could embed a newline into a request and cause webrick to split the response and send something back from client
<enebo[m]>
well there must be a commit for this locally in MRI whether it is in the official gem or not
<headius[m]>
so I guess the case would be that client A submits some content with the newline hack in it and if webrick serves that to client B it can trick B into receiving this split header
<enebo[m]>
Is there even a fix in the official gem?
<headius[m]>
the 1.6.0 gem differs only slightly from Ruby master's webrick
<enebo[m]>
This seems as though a commit for header processing probably just keys not containing bad stuff like a newline
<enebo[m]>
but no doubt the unofficial fix for this is only a few lines
<enebo[m]>
we could apply it manially if it is not clean and then push on them to fix their syncing or lack thereof later so 2.3 has a real version
<headius[m]>
the diff I showed you was the difference between ruby master and webrick 1.6.0
<headius[m]>
this diff is from ruby 2.5 HEAD to webrick 1.6.0
<enebo[m]>
so far some 2.7 checks and require_relative
<enebo[m]>
err quite a bit more
<enebo[m]>
bleh
<enebo[m]>
Do you actually see that fix in that diff?
<headius[m]>
no, this is not the diff for that
<headius[m]>
if you want to see that I can show you from our 1.4.2 to webrick's 1.6.0, but it's big
<enebo[m]>
no thanks
<headius[m]>
well our 1.4.2 and webrick's 1.4.2 are the same
<headius[m]>
I believe we would need to go to 1.6.0 to get all the cve fixes
<headius[m]>
it's hard to tell where they are because the one I mentioned above never appears in the commit logs
<enebo[m]>
webrick-jruby gem for a release? but that messes with default gems
<headius[m]>
yes, can't upgrade it then
<enebo[m]>
hehe but who does that
<headius[m]>
for it to remain a default gem we either need to cheat in the same way CRuby does, or upgrade to 1.6.0
<enebo[m]>
I am not saying we should not honor it
<enebo[m]>
but it is funny
<enebo[m]>
people gem updating webrick and using it in production...HAHAHA
<enebo[m]>
ok that is not helpful
<headius[m]>
I don't get why it's such a problem for them to leave webrick sources in webrick
<enebo[m]>
yeah I really don't get it
<headius[m]>
I'd vote to either leave it as is or just update to 1.6.0 for this release
<enebo[m]>
Maybe the svn only guy would stop not committing to it
<enebo[m]>
yeah I think 1.6.0 would tick all the boxes but potential risk
<enebo[m]>
We can test webrick on 9.2 though and at least tell if it works
<headius[m]>
so that diff above is from CRuby 2.5 HEAD to webrick 1.6.0
<headius[m]>
the diff from us to CRuby 2.5 HEAD is extensive
<headius[m]>
so I lean toward just using 1.6.0
<enebo[m]>
ok lets examine that diff a little more to see if there is anything weird there
<enebo[m]>
yeah I do atm as well
<chrisseaton[m]>
This generally represents lack of buy-in on the idea of standard and default gems - which I think we're all supposed to be switching to at some point.
<enebo[m]>
If nothing pops out we will try using it and use a little faith
<headius[m]>
chrisseaton: funny that JRuby seems to be the only one doing it
<enebo[m]>
chrisseaton: when this idea was originally proposed not all the Japenese hackers liked the idea of the source being thrown out into many different repos
<chrisseaton[m]>
Yes TruffleRuby just copies whatever bits are in MRI
<headius[m]>
enebo: I can do a 1.6.0 update PR
<enebo[m]>
I think most were on board
<enebo[m]>
headius: sure
<enebo[m]>
I feel like any project has personalities where some times you make a compromise vs hurting their feelings
<enebo[m]>
The MRI not moving fully to git is one major example
<enebo[m]>
it loses support for wasm
<enebo[m]>
:)
<headius[m]>
they're fully git now though, so that finally happened
Antiarc has quit [Ping timeout: 258 seconds]
<enebo[m]>
yeah it happened but took about 5 years over mostly a single personality objecting
<headius[m]>
but they host their own canonical repo
<headius[m]>
any jars normally on the filesystem just go through the normal URLClassLoader logic, and now will not be closed to avoid the bug
<enebo[m]>
I wonder if we have ever had issues at this point on windows
<headius[m]>
could be yes
<enebo[m]>
Will we unconditionally be the only source accessing that file
<enebo[m]>
AHA we do have this problem
<headius[m]>
it's unpacked to a tempfile so it should be unique
<enebo[m]>
HAHA
<headius[m]>
closing it as in the PR or before using URLClassLoader should allow it to be deleted
<enebo[m]>
I forgot about this. People used to report about how our unpacked I think jffi jars would just collect endlessly in whever that temp dir is on Windows
<enebo[m]>
we do nothing on Exception
<enebo[m]>
anyways it is not in your PR but it stuck out
<enebo[m]>
atavistic memory from maybe forgetting we had some temp jar removal problem in the past (or still) on windows if you run it millions of times
<enebo[m]>
Now I am dredging up more memories....Wayne did not want a known location in case the machine was targetted and someone replaces the jar between runs
<enebo[m]>
Perhaps we do always have it actually closed now and it will remove...I need to pop over to my windows box...be back in a few
<headius[m]>
hmm
<headius[m]>
looking at 9.2 I don't see where we close the classloader at all now
<enebo[m]>
yeah so I was just going to mention this fact
<headius[m]>
because it more directly controls the lifecycle of the runtime
<enebo[m]>
this will only effect embedding
<headius[m]>
so this does not affect normal dist usage at all
<enebo[m]>
and those users can toggle off the close/no-close if there is a problem
<enebo[m]>
so we have no problems I think
<enebo[m]>
It is really about the default value for them but for normal non-embedders there is no issue
<headius[m]>
right... so this will basically make the no-close behavior default, but still close temp file jars
<headius[m]>
so that will avoid two runtimes stepping on each other for shared jar file references
<enebo[m]>
but we will be changing the default for .12?
<headius[m]>
enabling the close will return it to previous behavior of closing everything
<enebo[m]>
We could not change it and then tell them it will be on default for 9.3 and use this option to not close
<headius[m]>
well for .12 it would close everything, which breaks multiple embedded runtimes
<headius[m]>
I mean for .11 that's what it does, we're still deciding .12
<headius[m]>
the change in .12 would be that it does not close jar files that are likely shared across embedded runtimes, avoiding the bug
<headius[m]>
if we merge this
<enebo[m]>
yeah we are talking about flipping a switch for one point release vs providing the switch (which is fine)
<enebo[m]>
2 users I hope are not all embedding users but perhaps they all have noticed this
<headius[m]>
are you saying we should have it on for 12 to reduce behavior change then?
<headius[m]>
on = close like in 11, which would hit the bug
<enebo[m]>
we should do what .11 and earlier does but let the users who have the issue know they can fix it by making it not close option
<headius[m]>
but with option to turn off to avoid the bug
<headius[m]>
so we add option but leave default as .11
<headius[m]>
right ok
<headius[m]>
I'm fine with that
<enebo[m]>
we will tell them it will do this by default in 9.3
<headius[m]>
right
<enebo[m]>
So it is a workaround for 9.2.12
<enebo[m]>
ok
<headius[m]>
yes
<enebo[m]>
Honestly all the folks are probably not affected but I am leery. I assume emebdders are two cases: 1) lots of ruby runtime evals 2) a single one which maybe jrebels or something some times
<enebo[m]>
for 2 probably not evne that
<enebo[m]>
s/not/now
<enebo[m]>
our reporters are #1 right?
<headius[m]>
it would only affect people that are churning through and tearing down embed runtimes
<headius[m]>
yeah the reporter is in that group
<headius[m]>
they use and throw away runtimes enough that they triggered this
<enebo[m]>
so default for #2 will not be noticed but for #1 it seems only some of those people ever notice?
<enebo[m]>
Maybe a lot of people just do not do full isolation and use pools of runtimes
<headius[m]>
well the reporter actually said they worked around it somehow but it's fairly hard to trigger
<headius[m]>
his test case burns through thousands of runtimes very fast and only fails about 50% of the time
<enebo[m]>
heh ok well we have a solution for .12
<enebo[m]>
and a carrot for 9.3 I guess
<headius[m]>
ok
<headius[m]>
updating issues
<headius[m]>
webrick PR appears to be fine in CI
<headius[m]>
so the remaining item is from kares update of RubyGems but that's still in flux
<headius[m]>
and it occurs to me now... if someone's running into the issues he wants to fix, they can still update rubygems in JRuby
<headius[m]>
so maybe we punt on that after all
<enebo[m]>
assume default gems work properly yeah
<headius[m]>
kares: I don't suppose you're around right now but maybe you could explain why you think we need to ship updated RG rather than just letting users update
<enebo[m]>
I would prefer that to the risk of shipping it without any bake
<headius[m]>
RG isn't a default gem, the updater actually overwrites stdlib
<headius[m]>
Ruby installers may even be doing this already as part of the install process
<enebo[m]>
system-update thing? I guess it is a chicken-egg thing?
<headius[m]>
yeah `gem update --system`
<enebo[m]>
yeah I forgot about rg being special (which makes sense)
<headius[m]>
so let's defer on that for another day and see what kares has to say
<headius[m]>
I don't like updating under duress and I like shipping a patched RG even less
<enebo[m]>
yeah kares speak up by my time tomorrow morning (I am 7 hours behind you) if it is something important otherwise I will test bits in the morning and hopefully just get it out
<headius[m]>
I will proceed to merge in the security fixes, webrick 1.6.0, and URLClassLoader fix with .11 behavior
<headius[m]>
ahh, and put the random seed behind the same option
<headius[m]>
ok I see the option actually changes how the random seed is acquired so that part's done