[perl #117263] Re: Bleadperl v5.17.9-200-g0e0ab62 breaks MLEHMANN/JSON-XS-2.33.tar.gz

# New Ticket Created by  yves orton 
# Please include the string:  [perl #117263]
# in the subject line of all future correspondence about this issue. 
# <URL: https://rt.perl.org:443/rt3/Ticket/Display.html?id=117263 >


On 21 March 2013 14:00, Marc Lehmann <schmorp@schmorp.de> wrote:
> On Thu, Mar 21, 2013 at 12:28:54PM +0100, demerphq <demerphq@gmail.com> wrote:
> +-> > >> > - Unless I am mistaken, this is to avoid a DOS attack. DOS attacks are easy
> +-> > >> >   to protect against, and this won't protect me against basically any form
> +-> > >> >   of attack, so it's useless. Far better methods exist that prevent every
> +-> > >> >   form of memory resource and/or cpu time starvation.
> |   > >>
> |   > >> You are mistaken.
> |   > >
> |   > > You are really claiming that this is one of the best methods to prevent
> |   > > memory resource and/or cpu time starvation? Because that's what the satteemnt
> |   > > you quoted says.
> |   >
> |   > I am sorry, I seem to be missing some context here. What statement did
> |   > I quote? I just checked this thread I did not quote anything.
> |
> +-- These ones (hope you can read my ascii art).
>
> I interpreted it to mean to refer to the last one you quoted.
>
> Now, if you didn't quote anything, what would your statement refer to?

Ah, right, I didn't consider that "quoting" even tho of course it is.

>> The purpose of hash iterator randomization is make it harder, and
>> hopefully outright impossible to discover the hash seed a given
>> process is using.
>
> That in itself is obviously a no-goal. No, I don't believe that is the real
> purpose. It must be something else, like: "to make it harder to use DOS
> attacks exploiting knowledge about hash collisions".
>
> Or something like that.
>
> In case you wonder why it is a no-goal, I can discover these hash seeds
> with a debugger, or an XS module, and I doubt your change will stop me
> from doing it (to the extent this still exists).

Peter Rabbitson already replied to this.  Here is the dialog that ensued:

On 21 March 2013 16:03, Marc Lehmann <schmorp@schmorp.de> wrote:
> On Fri, Mar 22, 2013 at 01:20:40AM +1100, Peter Rabbitson <rabbit-p5p@rabbit.us> wrote:
>> > > There is no access to the running process itself, no XS, no debugging
>> > > tools. Just "black box" observation. The point of the changes is to make
>> > > such type of observations futile, nothing more.
>> >
>> > I know. Please read the whole thread before making comments, thank you.
>> >
>>
>> I am confused... If you know there is no access to the running
>> interpreter, why did you bring up XS/debugging in the first place...?
>
> Because I originally said it's about avoiding DOS attacks (and later
> refining that to mean cpu/memory starvation), and yves said, no, it's
> purpose is to make it hard to discover the seed, after which I pointed out
> that this is unlikely the real purpose, because you can already discover
> this seed in other ways (for example, by setting an env variable before
> starting the program).

You know that this is not about XS nor having access to a debugger,
nor the ability to set the environment var, yet bring it up anyway.

So you are trolling.


>> Once an attacker can determine the hash seed they can launch
>> algorithmic complexity attacks on our hash implementation. However the
>> iterator randomization does not in any way prevent a DOS, it prevents
>> obtaining the information that makes a DOS possible.
>
> I think you are splitting hairs, but you get it wrong.

Your attitude makes me want to split hairs with you. You employ a
highly disagreeable style of communicating with people and should not
be surprised when they do not want to be helpful towards you.

The patch to make hash iterator traversal random is specifically to
make it harder for an attacker to determine the hash seed.

> The purpose of your change is to make it harder to apply such DOS attacks.
> Saying that's not the purpose, but the purpose is just to hide this
> information is dishonest, because hiding this information isn't a useful
> purpose in itself.

I haven't been in any way dishonest. There is nothing false about my
statement at all.

>> > The perlfunc 5.16 documentation on "keys". Older versions are even more
>> > explicit.
>> >
>> > all them are ambiguous, but I already mentioned that.
>>
>> Well, I beg to differ. I think you are conflating "does not say anything
>> on the subject" and "says something ambiguous".
>
> I think it strongly implies it. Beat me if you wish, but your
> interpretation is as good as mine, regardless of the intent behind it.

No, my interpretation is correct and yours is a figment of your
imagination. There is no equivalence there.

>> The documentation for keys(), and as far as I know all other
>> documentation relating to hashes says nothing about consistency,
>> either cross process or in between hashes.
>
> Sorry, but "guaranteed to be the same as either the keys ..." clearly
> implies some consistency, at least between those. Also saying the order
> is the same for the "same hash" is ambiguous, as "same" has tow meanings,
> namely, "identical" (of same identity) and "similar in some kind" (e.g.
> same contents).

No, no, no. There is no consistency, there never has been, there never
will be, and arguing about it is unproductive.

>> It says that the keys are returned in an apparently random order, that
>> the random order is subject to change in future versions of perl,
>
> No problem with that.
>
>> the only guarantee of any form provided is that the keys() function
>> will return items in the same order that each() and values would return
>> them assuming the hash has not been modified (a secondary clause in the
>> documentation for "each" specifies it is safe to delete the key that was
>> just returned by each()).
>
> It also says the ordering is the same for the same hash.

Yes, as in identity. If you find that

keys(%hash)

returns items in a different order than

values(%hash)

does then please file a bug.

> I know how it's meant, but I also know what is said, and assuming that
> "the same hash will result in the same ordering" is not unreasonable.

The docs say "the same hash unmodified", so that implies very strongly
that it means identity.

> Note that the earlier randomisation patch already broke the semantics, as
> the ordering changed even within the same version (but different program
> runs).

No, it didnt break any semantics. Stop making stuff up.

>> Furthermore I cannot find any evidence that the wording for this has
>> changed in terms of specificity in the lifetime of perlfunc.pod:
>
> Saying it changes in future versions but is the same for the same hash
> STRONGLY implies that the ordering is the same for the same version of
> perl, which is already not true.

No it doesn't.

> I give you, as I said, that the wording is ambiguous, and that it is
> actually an older change that broke it, but this interprettaion is
> entirely valid, and I am not the only one who used it.
>
>> perlfunc.pod didnt exist prior to that commit.
>
> And it clearly changed, if you took notice.

Not in any way relevant to this discussion, if you took notice.

>> I see nothing ambiguous in the documentation, nor do I see it becoming
>> more or less specific over time.
>
> Saying you don'T se something has no value whatsoever. People tend to chose
> one interpretation of another.
>
> You'd have to make the stronger claim that no other interprettaion is
> reasonably possible, and I don't think you can, simply because "same", in
> english, is ambiguous as to whether mean identity or equality.

There is nothing here to argue about, you are wrong, and there your
interpretation is wrong, and that is all there is to it.

>
>> > I don't think that, and haven't made that claim. I think you can make
>> > faster hashes with better properties without the drawbacks.
>>
>> So then why did you bring it up in this thread? If you have issues
>> with hash performance and think you can do better then please start a
>> new thread, preferably by posting patches.
>
>> I don't think that, and haven't made that claim. I think you can make
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Are you really so stupid as to claim you didnt say something only a
few lines after you say it?

Did you not say:

"I think you can make faster hashes with better properties without the
drawbacks."

And was I not replying to you saying it?

> I said it before, and Is aid it again: I didn't bring up what you put into my
> mouth, and I explain it again what I meant, and this time, please either
> believe what I write, or argue against it, but please don't attack strawmen:
>
> I think one can make faster and better hashes which fix this problem without
> breaking code.
>
> That means the breakage is not necessary.

So prove it. Patches welcome.

>> >> > - Taking out the inability to write a proper hash on all the perl developers
>> >> >   out there is just wrong - Perl is the runtime, and should get it right,
>> >> >   if possible.
>> >>
>> >> This doesn't make grammatical sense,
>> >
>> > I think it's a perfectly fine english sentence. Maybe a bit complicated for
>> > you to understand though.
>>
>> I guess you meant "your" when you said "the".
>
> No, I specifically meant "the", because I didn't want to say "your".
>
>> If so then I will just say that I find this comment rude and
>> uniformed.
>
> Since your assumption is wrong, your conclusion is irrelevant fprtunately.
>
>> I am not responsible for our current hash implementation,
>
> And I didn't claim so. On purpose, in fact.
>
>> nor do I think you could do any better. I am pretty sure that the only
>> way one can improve perls hash performance is to make it do less, and
>> that by doing so you would upset far more people than you would make
>> happy by making it a nanosecond faster.
>>
>> But please, go ahead and prove me wrong. Post some patches.
>
> I already did some benchmarks with unordered hashes. I wonder what they
> do less than perl hashes, specially now that almost all conssitency in
> ordering is gone, which is usually the only problem.

Which just shows you really have no idea at all how Perls hashes work.

Which pretty much ends this discussion.

>> > I am trying to say that the solution for bad hash tables is better hash
>> > tables, or a sense of proportion, not breaking perfectly fine existing
>> > code.
>>
>> I find it really hard to take you seriously given how often you make
>> silly statements like "not breaking perfectly fine existing code".
>
> If you want me to respect your opinions on what is fine code, please
> respect mine.

I don't really give a shit about your opinion actually. Especially as
you frame it rudely, make personal aspersions, and act like a jerk
when you express it.

>> your code broke because of this change it is because you were making
>> unwarranted assumptions about how perl's hash function operated,
>
> I don't know if they are unwarranted, but at least they were documented,
> even if that turns out to be ambiguous wording.

As I said before, there is nothing ambiguous about saying nothing
about something. Ambiguity comes when there are more than one option,
here there are zero options. There is no reasonable interpretation of
the documentation that supports your position.

>> as such your code wasn't perfectly fine. In fact if your code is
>> breaking due to this change then I could trivially contrive code in an
>> older version of perl that would also break your assumptions.
>
> Well, the code in question (which defines my assumptions) works with older
> versions of perl, so I call your bluff.

So sure, I can make it break with a one line change.

>> > To me, breaking existing code for questionable or no reason is making it
>> > worse. It is obvious that you disagree and think you didn't make it worse,
>> > but at best, you have to disagree with my perfectly valid opinion on this
>> > topic-
>>
>> I have to disagree? What? Did you mean "agree"?
>
> No, I mean "disagree".
>
> You seem to be very combative, and I guess this is why you make so many
> simple mistakes: I usually really mean what I write (and even then, I do make
> mistakes).

You are rude and insulting in your manner, you should not be surprised
when people you communicate with are combative.

> I _really_ mean that you disagree, and I really mean that you have to
> disagree with me if you have another opinion.
>
>> Anyway, it doesnt matter. I don't break existing code for no reason.
>
> Sure. I am saying they are bad reasons, and badly executed, not that it is
> without reason.
>
>> A) I have reasons,
>
> Never said anything to the contrary.

You certainly implied it with "breaking existing code for questionable
or no reason is making it worse".

>> B) any code broken was already broken but the author did not know it
>
> Empty claim.

Ive already demonstrated how your assumptions are broken by a simple hash copy.

>> C) there is a reason the docs say "The actual random order is subject to
>> change in future versions of perl"
>
> The actual random ordering changes with each make test run, too, but the
> testsuite still passes. That is cleatrly not the bug here.
>
>> and make no other guarantees that the limited ones I enumerated
>> above. The reason of course is to allow the hash function to be changed
>> or improved over time.
>
> It says the ordering is the same for the same hash.

And it is.  (So long as you dont modify it by inserting new items.)

> Changing the hash function doesn't break the code. Changing the hash
> function within the same version of perl, and within the same process, is
> what changes it.

We don't change the hash function during the process.

> At this point I must actually admit that I didn't even check that this is
> true, I only assume it.

Well you are wrong.

> So if the ordering is really the same for the same hash, within the same
> process (using the same perl), then I am indeed totally wrong.
>
> Is that the case?

@keys1= keys %hash;
@keys2= keys %hash;

will always return identical @keys1 and @keys2 assuming %hash is a
"normal" perl hash.

>
>> > I am fully aware of them being volunteers, but so am I (and I am a regular
>> > contributor as well), please keep that in mind.
>>
>> Perhaps you are, I never advanced an opinion on the subject.
>
> Well, I am. I tell you so. What are your reasons do doubt me so much?

Perhaps you are, I never advanced an opinion on the subject.

But since you bring it up again I will note I checked the log and you
are mentioned a whole 6 times.

$ git log --grep "Marc Lehmann" --pretty=oneline
a5b310e3129d1a2cd55b8d79445eb65964c997d1 Update Digest-SHA to CPAN version 5.83
d9a4b459f94297889956ac3adc42707365f274c2 Restore building Encode's
subextensions for a static build.
50c1ac04356af5f8e8f967db7ed083187aacb550 Pod nit in Encode.pm, found
by Marc Lehmann in RT #36949.
1a08a6ab91582d74e16135fa1c131885305144d0 [perl #22141] patch for
Time::HiRes to get rid of -lrt on linux From: Marc Lehmann (via RT)
<perlbug-followup@perl.org> Message-Id:
<rt-22141-56710.3.69543054121962@bugs6
591166816e29d06350e6ec8041e206c1afa73419 In the UTF-8 branch of
crypt() the extra \0 byte is required, found by Marc Lehmann.
bf8afc63e607fcc7c1b66ceb8d1c6a9f20862f5b Fix for a segfault, from Marc Lehmann.

>> > The perl5porters are not "better" than me as a major contributor to perl
>> > in any way, and have no higher moral ground. Acting uppity because you are
>> > part of a mailinglist and contribute more often than me is proving nothing
>> > really.
>>
>> I never made any comments about anyones individuals merits. Nor do I
>> know what I did that was "uppity".
>
> I interpreted your comment as irony, as am pretty confident it was meant
> like it. If you really honetsly say you meant it as-is, I will, however,
> believe you and admit my reaction to your last comment was wrong.

I meant every word in that sentence literally.

>> Well I have responded to the only cogent concern that I have seen you
>> express, which is that you think that these changes contradict the
>> documentation. The documentation does not support you in this regard.
>
> Well, it clearly does. It's your interpretation that doesn't support mine,
> and now the code.

There is no "clearly does" involved here. The documentation simply
does not say what you seem to think it says. If it did it would say it
explicitly, but given how perls hash works (which I do know, and which
you clearly do not), I am very very certain that no person that did
understand how perls hashes work would ever say what you think the
documentation says.

>> Is there some other matter I should respond to?
>
> You don't have to respond at all, but if you do so, I personally expect
> you to use more than empty claims, and actually use evidence (which you
> have done, for the most part).
>
>> Oh, I suppose I forgot. Afaik, you don't even have to fix this, just
>> take the patches I did to the JSON::PP tests and apply them.
>>
>> See: https://rt.cpan.org/Public/Bug/Display.html?id=83421
>
> But the bug is in the perl core. Nothing I apply can fix that, becasue I
> can't apply fixes to the perl core.

Excuse me? No the bug is in your code, expecting to snapshot a
serialized hash and have that be the same as some other serialized
hash with the same keys. There has never been any reasonable basis to
expect this to always happen.

> Besides, it makes no sense to me to say "you don't have to fix that, just
> apply the fix". What you mean is "you don't have to make the work to invent a
> fix, it has already been done".

If you prefer that wording then so be it.

> The problem is that I think the code is fine, and even if not (by sheer power
> of force by you applying a buggy patch and claiming it's a good and
> reasonable and corretc patch and my code is broken), I think it *should* be
> correct, because taking away a useful feature that doesn't (in itself) gain
> you anything
>
> All I urge you, again, is to consider doing the "hardening" either better,
> in a compatible way (which I think is possible), or consider the trade-off
> between a rather ineffective patch (that doesn't shield against similar
> DOS attacks) and breaking code that ought to work.
>
> I don't have any strong hope for that, and think you will opt for the
> ignore-and-break-it route, which is a valid way to proceed from your side
> of things. I did feel the moral obligation to tell you why it is wrong,
> and if you still do it, you at least do it in an informed way.

Post some patches or shut up. I dont care what your opinion is on this
subject anymore.

Yves


-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

• [perl #117263] Re: Bleadperl v5.17.9-200-g0e0ab62 breaks MLEHMANN/JSON-XS-2.33.tar.gz by yves orton