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

Make module dependencies clearer #15

Closed
randombit opened this issue Apr 17, 2014 · 6 comments
Closed

Make module dependencies clearer #15

randombit opened this issue Apr 17, 2014 · 6 comments
Assignees

Comments

@randombit
Copy link
Owner

Someone used --no-autoload and then was rightfully confused when the RNG didn't seed, because dev_random wasn't loaded. Add an option to configure that dumps the module dependency tree, and potentially merge some mutually dependent modules.

@randombit randombit self-assigned this Apr 17, 2014
@webmaster128
Copy link
Collaborator

I think this is the dependency graph of Botan modules:

table_small
full resolution (3.5 MB)

What I found most confusing is that "base" depends on other modules. Does that make sense? Shouldn't a base be dependency free?

@webmaster128
Copy link
Collaborator

Can we move get_pbkdf from base/lookup.h to pbkdf/pbkdf_utils.h? That would remove the dependency base -> pbkdf. It might force the user to add an additional header.

@randombit
Copy link
Owner Author

pbkdf_utils.h is an internal header used by PBKDF implementations. If moved anywhere it should go to pbkdf.h

But - is there a particular reason to break this dependency? get_pbkdf was declared in lookup.h in 1.10, and also in 1.8.x after 1.8.10, so I'd be inclined to leave it there. Dependency minimization in order to make the amalgamation smaller is reasonable, but adding pbkdf means under 100 lines of header and no sources so I don't see how avoiding the dependency helps much.

@webmaster128
Copy link
Collaborator

Jap I have: If you build

./configure.py --no-autoload --enable-modules=base
make

then you load base and pbkdf but not pbkdf1 or pbkdf2. Removing the dependency would allow us to disable the test_pbkdf.cpp and (if the was a) pbkdf command line tool using BOTAN_HAS_PBKDF.

@webmaster128
Copy link
Collaborator

If you are saying that (a) you want to dump a dependency tree and (b) you might want to merge mutually dependent modules, I think the overall goal can and should be to eliminate or reduce circular dependencies.

Having no circular dependencies makes the module system much easier to parse, draw and understand by humans, which reduces the potential of errors.

When I created the script for the map I first thought I could hold the dependencies in a tree structure. Then I realized that this will never be possible because complex protocols like TLS will always depend on multiple atomic modules. So it has to be a directed graph, which I thought could line up easily from base at the left of the picture to more complex things like tls on the right, where modules at the right always depends on modules on the left. But it was getting even more complex when I realized that there are circular dependencies.

Given issues like #146, I think this is not only about how much a compile time or library size increases but about transparency. The user should be able to understand what modules are loaded and why.

If I e.g. just want to build the missing download checksum checker sha256sum for Windows and build a lib with sha2 and Botan goes like

Loading modules alloc auto_rng base block entropy hash hex hmac hmac_rng mac mdx_hash modes pbkdf rng sha2_32 sha2_64 stream utils

I ask my self, why the hack do I need entropy and pbkdf. I don't know that I cannot actually use PBKDF but Botan tells me the module will be loaded.

Aiming towards better software design and transparency for our users, I'd like to reduce circular dependencies where possible.

In case of PBKDF: Moving get_pbkdf() to pbkdf.h is even better because this header will be included in the majority of applications that use PDKDF1/2 today.

@randombit
Copy link
Owner Author

Would be good to reduce deps over time but not sure there is anything in particular to do for this ticket. Closing.

0xa5a5 pushed a commit to 0xa5a5/botan that referenced this issue Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants