Skip to content

Remove global mutable SigningMethod registry #419

@ash2k

Description

@ash2k

In v6 (#335) I suggest to remove the global mutable SigningMethod registry. Instead of it we could have a global immutable registry for built-in signing methods and WithSigningMethods(...SigningMethod) option for the Parser (i.e. ParserOption).

The existing WithValidMethods() can be removed too. If it's not set then any registered method is accepted. The behavior remains the same - if you want to use certain signing methods only, you need to specify them using the proposed WithSigningMethods(). The benefits are:

  • no need to register a custom singing method in the registry. Instead it is specified per-Parser.
  • no chance of accidentally changing the behavior (making it less strict - not good!) of parsers that don't have WithValidMethods() specified.
  • Less fragile code. You could depend on a signing method being registered in some other package "far away" in the program from where you use it. If it stops being registered over there, you get a subtle breakage over here - "benefits" of global mutable state. Third-party libraries can change behavior of your program when you import their packages.
  • RegisterSigningMethod() doesn't check if it's overwriting an existing registration. This can lead to undetected "bad" implementations being substituted in place of the good ones. A third party package can mess with standard registered implementations.
  • etc, etc, etc - all the drawbacks of global mutable state.

One extra benefit: if you have many implementation that are just types (e.g. structs) but they are not used anywhere - they will be trimmed from the final binary by the linker. This does not happen when you have an init() { RegisterSigningMethod(MyImpl{}) } because the type is referenced and an instance is retained in the global map. Result is more bloated binary and bigger RAM footprint.


With WithSigningMethods(...SigningMethod) parser configuration is localized and nothing can change it. The only way to change it is to actually reconfigure the parser explicitly where it is constructed. I think this is a really big deal for security-sensitive code, such as what this library is for.


Some background. I'm trying to use HMAC + SHA3 512. I have to register the custom-made signing method implementation, ensure that the package with the registration is imported where I use the signing method, specify WithValidMethods("MY ID OF THE METHOD") when constructing the parser. This is so much hassle and brittleness vs just using WithSigningMethods(MySHA3HMAC{}).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions