Support all Certificate Fingerprint Algorithms

[Sean-Der]

Before was hardcoded to sha256

Resolves #1076

[Sean-Der]

This PR look ok @paullouisageneau ?

Anything I can do to make it easier to merge/test/use?

[Sean-Der]

@paullouisageneau I shared builds of OBS with this patch and testers have confirmed it fixes the issue obsproject/obs-studio#9800 (comment) !

[paullouisageneau]

Thank you for adding the feature!

While it works, I think there are some architectural issues with the proposed API. I think the FingerprintAlgorithm enum should actually be defined in Certificate, it doesn't really belong in Description. Also, carrying algorithm and digest separately side-by-side feels like a cumbersome design.

It could make sense to introduce Certificate::Fingerprint like this instead:

struct Fingerprint {
    enum Algorithm { Sha1, Sha224, Sha256, Sha384, Sha512 };
    static string AlgorithmIdentifier(Algorithm algorithm);
    static size_t AlgorithmSize(Algorithm algorithm);

    Algorithm algorithm;
    string value;
};

What do you think?

[Sean-Der]

Looks great to me! I can implement this tonight.

Certificate isn't part of the public API. Where should I put Certificate::Fingerprint instead?

[paullouisageneau]

Certificate isn't part of the public API. Where should I put Certificate::Fingerprint instead?

Uh yes, my bad, sorry. I'd like to expose it but it isn't part of the public API currently. You could put it as CertificateFingerprint in include/rtc/description.hpp outside of Description for now.

[Sean-Der]

@paullouisageneau I believe I have updated everything. Could I get another review? thanks!

[Sean-Der]

@paullouisageneau I have addressed all the comments. Can I get another review please. thank you!

[paullouisageneau]

Only a few nitpicks, after that it looks good to me, thanks!

[Sean-Der]

@paullouisageneau done!

[paullouisageneau]

Thank you, it looks good to me now!

[Sean-Der]

huzzah! Thank you @paullouisageneau

Could I also get a tag? This closes out all OBS bugs, will be nice to see them all gone :)

[paullouisageneau]

Would it be possible to wait a few days for a 0.20.0 release? This PR breaks ABI (because of the field change in public-facing Description class) so it can't be released on 0.19. Of course I could set a 0.20.0-alpha tag, but wouldn't it be an issue for the packetizers of OBS?

[Sean-Der]

@paullouisageneau yea! No rush I also need to get CMake change in

sorry for the churn