Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #173 #174

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Fixes #173 #174

wants to merge 3 commits into from

Conversation

vzuevsky
Copy link

Fixes #173

@vzuevsky vzuevsky changed the title Fixes NLnetLabs/ldns#173 Fixes #173 Apr 18, 2022
@wtoorop
Copy link
Member

wtoorop commented Apr 19, 2022

Thanks @vzuevsky , that looks like a valuable contribution! And properly documented as well! Much appreciated. I'll try to review it shortly.

@psvz
Copy link

psvz commented May 10, 2022

Hi @wtoorop, any news on release with the fix? Cheers

@wtoorop
Copy link
Member

wtoorop commented May 11, 2022

@psvz I do think I can have a good look coming Friday. I'll keep you posted

@wtoorop
Copy link
Member

wtoorop commented May 16, 2022

Thank you Vitaly! Very clever! Is this algorithm by your own making?

W.r.t. the PR, I don't want to change the function definitions (in order not to break backwards compatibility), so I'm happy to change the default in ldns_pkt2buffer_wire(), but I'd like to keep the definitions of ldns_dname2buffer_wire_compress(), ldns_rdf2buffer_wire_compress(), ldns_rr2buffer_wire_compress() and ldns_pkt2buffer_wire_compress() and then add new definitions for them that use ldns_llnode_t. I'd also prefer a better name for ldns_llnode_t. For example ldns_compression_node_t. We can add text in the description of the old definitions in which we strongly recommend to use the new definitions.

So I will make those changes after RIPE84 in the run up to a new ldns release, or you can do them if you want to help me out :) It would be appreciated, but I also really don't mind if you leave that to me.

In any case thanks for this excellent contribution/improvement!

@vzuevsky
Copy link
Author

vzuevsky commented May 16, 2022

Thank you Vitaly! Very clever! Is this algorithm by your own making?

@wtoorop Thanks for your consideration. The algo is my own development. I am happy with the way forward you suggest. They are simple edits, so I would wait for a due stable release with all security fixes including mine. Any tentative date for that?

@wtoorop
Copy link
Member

wtoorop commented Jul 14, 2022

@vzuevsky I'm going to postpone this for after the next release, because we think we found an even better way to do this.

@psvz
Copy link

psvz commented Oct 22, 2022

Hi @wtoorop is it fixed in a release yet?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Performance vulnerability in dname compression
3 participants