Breaking changes in upstream error types.

[cpu]

There were several upstream breaking API changes in Rustls related to error types:

• Rework certificate errors rustls#1182
• Improve error reporting for corrupt messages rustls#1172
• Adds more certificate errors. rustls#1215
• Move three internal enums to public API rustls#1229
• msgs: yield Result instead of Option from Reader/Codec rustls#1198

The upstream changes will have to be adapted to error.rs and client.rs.

• rustls::internal::msgs::enums::AlertDescription needs to be changed to the non-internal import.
• InvalidCertificateEncoding, InvalidCertificateSignatureType, InvalidCertificateSignature, InvalidCertificateData, CorruptMessage, CorruptMessagePayload, PeerIncompatibleError, PeerMisbehavedError have been removed and need to be adapted to the more fine-grained error types that replaced them.

[cpu]

@jsha Do you have any thoughts on how the rustls_result enum should adapt to the upstream changes:

rustls-ffi/src/error.rs

Lines 137 to 152 in 55df122

	// From https://docs.rs/rustls/0.20.0/rustls/enum.Error.html
	CorruptMessage = 7100,
	NoCertificatesPresented = 7101,
	DecryptError = 7102,
	FailedToGetCurrentTime = 7103,
	FailedToGetRandomBytes = 7113,
	HandshakeNotComplete = 7104,
	PeerSentOversizedRecord = 7105,
	NoApplicationProtocol = 7106,
	BadMaxFragmentSize = 7114,
	UnsupportedNameType = 7115,
	EncryptError = 7116,
	CertInvalidEncoding = 7117,
	CertInvalidSignatureType = 7118,
	CertInvalidSignature = 7119,
	CertInvalidData = 7120, // Last added

For e.g. CertInvalidEncoding = 7117. The upstream InvalidCertificateEncoding entry in the Error enum this was being mapped to previously was removed and now we have InvalidCertificate(x) where x is something more specific like CertificateError::BadEncoding.

A couple questions I have:

• Should all new additions use distinct u32 values not previously used, leaving gaps for the removed types?
• Should there be a u32 per more specific CertificateError variant, or just for the top level InvalidCertificate?

I'm trying to implement some of these changes without having had time to do a lot of reading of the existing code, or the git blame. Any guidance is welcome :-)

[jsha]

Should all new additions use distinct u32 values not previously used, leaving gaps for the removed types?

Yes, please.

Should there be a u32 per more specific CertificateError variant, or just for the top level InvalidCertificate?

Yes. The error design here is more-or-less "combinatorial explosion," since I looked at the error enum in rustls at the time and decided that splitting out errors for sub-variants was reasonable in scope.

For the Other sub-variant, we will have to drop the Arc<_> and any information in it, which is a bummer but the best approach for now.

I think sometime soonish we may want to rework errors in the FFI binding to accommodate more complex error types like Other and General without dropping useful information on the floor. But that can come after the next release; it will be a significant change.

[cpu]

I just noticed there was another PR that touched error content I missed in my original scan: rustls/rustls#1198 Added to the list in the PR desc.

I'm almost done adapting all the changes. I might run out of time today before I have a cleaned up PR ready for review but if that happens I should have something to share early tomorrow.

[cpu]

Fixed with #303