Add PK_Signature_Options

[randombit]

Continuation / rebase of #4318

This allows controlling all details of how signatures are created, without having to stuff values into the single parameters string which was previously available.

[coveralls]

coverage: 90.475% (-0.2%) from 90.685%
when pulling 0729830 on jack/new-pk-sign-builder
into 1eacc5b on master.

[reneme]

One thing I'd like to raise for a final time would be support for move-vs-copy. (Feel free to just give a thumbs down reaction on this comment for a hard veto.) 😅

I know that most of the expected std::string contents in this instance are probably falling within short-string-optimization (which seems to be ~12 chars for 32bit machines [source]). However, setting a "context" buffer would result in unnecessary allocations/copies of arbitrarily large data. Especially when users set it as the first option of a chain of with_... calls. Other builders might behave worse here. E.g. the CertificateParametersBuilder (#4996) will likely hold plenty of vectors and DNs that would be copied around a lot when implemented in the same way as this PR.

Making the builder smarter in this regard comes with a bit of boilerplate, but to me it seems small enough to consider (here's the example below with some explanation in Compiler Explorer). The essential idea would be to overload each setter for an rvalue-this and a const-ref-this. The latter is pure boilerplate and just calls the former. The rvalue-this implementation can be put into an implementation file and does not need to create an explicit next copy anymore.

class Options {
private:
    Options myself() const& { return *this; }
    Options myself() && { return std::move(*this); }

public:
    // setters...

    Options with_hash(std::string_view hash_fn) const& { return myself().with_hash(hash_fn); }
    Options with_hash(std::string_view hash_fn) && {
        m_hash_fn = hash_fn;
        return myself();
    }

    Options with_der_encoding(bool der = true) const& { return myself().with_der_encoding(der); }
    Options with_der_encoding(bool der = true) && {
        m_der_encoding = der;
        return myself();
    }
    
    // getters...

    const auto& hash() const { return m_hash_fn; }
    bool der() const { return m_der_encoding; }

private:
    std::optional<std::string> m_hash_fn;
    bool m_der_encoding = false;
};

[reneme]

I like this idiom of passing a defaulted bool value to be able to disable the option explicitly. I would suggest to apply this to with_deterministic_signature() and with_explicit_trailer_field() as well.

[reneme]

Perhaps not in the first iteration, but it would be awesome if we were able to pass a user-defined hash function here. We discussed that already in #4318, for instance to let users offload the hash calculation into some (obscure) hardware.

Holding a std::unique_ptr<HashFunction> as a member will break the current design of relying on the copy-constructor, though. So we would have to keep the hash object in a shared pointer, which seems like the right thing to do but could cause trouble downstream when we have to pass a unique pointer inside some API, I don't know. Or we would have to make the PK_Signature_Options move-only making it harder to use as a policy-like object across multiple signatures and forcing us to re-introduce lvalue& and rvalue& overloads for every setter.

[reneme]

Consistency-nit: The ordinary hash_function is spelled out, let's not abbreviate this one for consistency:

Suggested change

	const std::optional<std::string>& prehash_fn() const { return m_prehash; }
	const std::optional<std::string>& prehash_function() const { return m_prehash; }

[reneme]

Suggested change

	next.m_using_prehash = true;
	next.m_prehash = std::move(prehash_fn);
	// Calling this with a std::nullopt enables prehashing with an algorithm-
	// specific hash function that is not user-defined. Hence the bool flag.
	next.m_using_prehash = true;
	next.m_prehash = std::move(prehash_fn);

... I needed a moment to understand why m_prehash.has_value() wasn't enough to decide whether prehash was enabled or not.

[reneme]

Nits:

• std::optional::emplace instead of explicit assignment of std::vector
• implement string_view overload in terms of span version

Suggested change

	PK_Signature_Options PK_Signature_Options::with_context(std::span<const uint8_t> context) {
	BOTAN_STATE_CHECK_MSG(!using_context(), "PK_Signature_Options::with_context cannot specify context twice");
	auto next = (*this);
	next.m_context = std::vector<uint8_t>(context.begin(), context.end());
	return next;
	}
	PK_Signature_Options PK_Signature_Options::with_context(std::string_view context) {
	BOTAN_STATE_CHECK_MSG(!using_context(), "PK_Signature_Options::with_context cannot specify context twice");
	auto next = (*this);
	auto contextb = as_span_of_bytes(context);
	next.m_context = std::vector<uint8_t>(contextb.begin(), contextb.end());
	return next;
	}
	PK_Signature_Options PK_Signature_Options::with_context(std::span<const uint8_t> context) {
	BOTAN_STATE_CHECK_MSG(!using_context(), "PK_Signature_Options::with_context cannot specify context twice");
	auto next = (*this);
	next.m_context.emplace(context.begin(), context.end());
	return next;
	}
	PK_Signature_Options PK_Signature_Options::with_context(std::string_view context) {
	return with_context(as_span_of_bytes(context));
	}