renomination adds attributes after message-integrity and fingerprint

[fippo]

Spotted by @zshang-oai while looking at #876

sendNominationRequest here is adding the renomination attributes after M-I and fingerprint have been added to the message (which should lead to a programming error as stated here

This is a violation of the STUN spec and means renomination is not covered by M-I or fingerprint.
This does lead to test failures after pion/stun#273 which is not tagged yet and will have to be done more carefully.
This might be a good excuse to change the ice-option and attribute type @juberti?

Fixing the order in renomination should be safe though. @zshang-oai do you want to make a PR?

[JoTurk]

Following up on our recent discord convo, I think we should also downgrade pion/stun@bc40ca2 to just a log (We'll have to introduce a new constructor for Message to pass a logger). and / or make it only fail behind a flag.

[juberti]

yeah I was already planning to rev the ice-option just to ensure alignment on the attribute value and syntax. I'm guessing Pion being the sender of renomination is pretty uncommon though given it's usually used on the server?

[fippo]

and then the ice-option in webrtc triggers the flag in ice which triggers the flag in stun. Plumbing at its finest! ;-)

[JoTurk]

I'm guessing Pion being the sender of renomination is pretty uncommon though given it's usually used on the server?

We don't have a general idea about the usage, It's not enabled by default, so it's fair to assume it's pretty rare for Pion users. and until recently we used a different default attribute than libwebrtc 8e16375

(We're using renomination in a staging environment at Upwork and I'm not aware of any other users).