CVE-2015-2318: Improper Certificate Validation (CWE-295)
value: The TLS stack in Mono before 3.12.1 allows man-in-the-middle attackers to conduct message skipping attacks and consequently impersonate clients by leveraging missing handshake state validation, aka a "SMACK SKIP-TLS" issue.
CWE description: The product does not validate, or incorrectly validates, a certificate.
CVSS v3 base score: 8.1
Published: 2018-01-08T19:29Z
Repository: https://github.com/mono/mono
Language: C#
Files changed: mcs/class/Mono.Security/Mono.Security.Protocol.Tls/ClientRecordProtocol.cs, mcs/class/Mono.Security/Mono.Security.Protocol.Tls/Context.cs, mcs/class/Mono.Security/Mono.Security.Protocol.Tls/RecordProtocol.cs, mcs/class/Mono.Security/Mono.Security.Protocol.Tls/ServerRecordProtocol.cs
Commit date: 2015-03-06 10:34:14 -0500
Commit message: TLS protocol: add handshake state validation

Diff: diff --git a/mcs/class/Mono.Security/Mono.Security.Protocol.Tls/ClientRecordProtocol.cs b/mcs/class/Mono.Security/Mono.Security.Protocol.Tls/ClientRecordProtocol.cs
index 7cece5060e6..acaa0c2c37c 100644
--- a/mcs/class/Mono.Security/Mono.Security.Protocol.Tls/ClientRecordProtocol.cs
+++ b/mcs/class/Mono.Security/Mono.Security.Protocol.Tls/ClientRecordProtocol.cs
@@ -127,10 +127,11 @@ private HandshakeMessage createClientHandshakeMessage(HandshakeType type)
 
 		private HandshakeMessage createServerHandshakeMessage(
 			HandshakeType type, byte[] buffer)
 		{
 			ClientContext context = (ClientContext)this.context;
+			var last = context.LastHandshakeMsg;
 
 			switch (type)
 			{
 				case HandshakeType.HelloRequest:
 					if (context.HandshakeState != HandshakeState.Started)
@@ -146,34 +147,56 @@ private HandshakeMessage createClientHandshakeMessage(HandshakeType type)
 							AlertDescription.NoRenegotiation);
 					}
 					return null;
 
 				case HandshakeType.ServerHello:
+					if (last != HandshakeType.HelloRequest)
+						break;
 					return new TlsServerHello(this.context, buffer);
 
+					// Optional
 				case HandshakeType.Certificate:
+					if (last != HandshakeType.ServerHello)
+						break;
 					return new TlsServerCertificate(this.context, buffer);
 
+					// Optional
 				case HandshakeType.ServerKeyExchange:
-					return new TlsServerKeyExchange(this.context, buffer);
+					// only for RSA_EXPORT
+					if (last == HandshakeType.Certificate && context.Current.Cipher.IsExportable)
+						return new TlsServerKeyExchange(this.context, buffer);
+					break;
 
+					// Optional
 				case HandshakeType.CertificateRequest:
-					return new TlsServerCertificateRequest(this.context, buffer);
+					if (last == HandshakeType.ServerKeyExchange || last == HandshakeType.Certificate)
+						return new TlsServerCertificateRequest(this.context, buffer);
+					break;
 
 				case HandshakeType.ServerHelloDone:
-					return new TlsServerHelloDone(this.context, buffer);
+					if (last == HandshakeType.CertificateRequest || last == HandshakeType.Certificate || last == HandshakeType.ServerHello)
+						return new TlsServerHelloDone(this.context, buffer);
+					break;
 
 				case HandshakeType.Finished:
-					return new TlsServerFinished(this.context, buffer);
-
+					// depends if a full (ServerHelloDone) or an abbreviated handshake (ServerHello) is being done
+					bool check = context.AbbreviatedHandshake ? (last == HandshakeType.ServerHello) : (last == HandshakeType.ServerHelloDone);
+					// ChangeCipherSpecDone is not an handshake message (it's a content type) but still needs to be happens before finished
+					if (check && context.ChangeCipherSpecDone) {
+						context.ChangeCipherSpecDone = false;
+						return new TlsServerFinished (this.context, buffer);
+					}
+					break;
+					
 				default:
 					throw new TlsException(
 						AlertDescription.UnexpectedMessage,
 						String.Format(CultureInfo.CurrentUICulture,
 							"Unknown server handshake message received ({0})", 
 							type.ToString()));
 			}
+			throw new TlsException (AlertDescription.HandshakeFailiure, String.Format ("Protocol error, unexpected protocol transition from {0} to {1}", last, type));
 		}
 
 		#endregion
 	}
 }
diff --git a/mcs/class/Mono.Security/Mono.Security.Protocol.Tls/Context.cs b/mcs/class/Mono.Security/Mono.Security.Protocol.Tls/Context.cs
index b4caf28b5c7..3923daf1a8b 100644
--- a/mcs/class/Mono.Security/Mono.Security.Protocol.Tls/Context.cs
+++ b/mcs/class/Mono.Security/Mono.Security.Protocol.Tls/Context.cs
@@ -120,10 +120,12 @@ public bool	ProtocolNegotiated
 		{
 			get { return this.protocolNegotiated; }
 			set { this.protocolNegotiated = value; }
 		}
 
+		public bool ChangeCipherSpecDone { get; set; }
+
 		public SecurityProtocolType SecurityProtocol
 		{
 			get 
 			{
 				if ((this.securityProtocol & SecurityProtocolType.Tls) == SecurityProtocolType.Tls ||	
diff --git a/mcs/class/Mono.Security/Mono.Security.Protocol.Tls/RecordProtocol.cs b/mcs/class/Mono.Security/Mono.Security.Protocol.Tls/RecordProtocol.cs
index 589510685a6..e8ae131f2a5 100644
--- a/mcs/class/Mono.Security/Mono.Security.Protocol.Tls/RecordProtocol.cs
+++ b/mcs/class/Mono.Security/Mono.Security.Protocol.Tls/RecordProtocol.cs
@@ -86,10 +86,12 @@ protected virtual void ProcessChangeCipherSpec ()
 			if (ctx is ClientContext) {
 				ctx.EndSwitchingSecurityParameters (true);
 			} else {
 				ctx.StartSwitchingSecurityParameters (false);
 			}
+
+			ctx.ChangeCipherSpecDone = true;
 		}
 
 		public virtual HandshakeMessage GetMessage(HandshakeType type)
 		{
 			throw new NotSupportedException();
@@ -346,13 +348,10 @@ private void InternalReceiveRecordCallback(IAsyncResult asyncResult)
 				}
 
 				// Try to read the Record Content Type
 				int type = internalResult.InitialBuffer[0];
 
-				// Set last handshake message received to None
-				this.context.LastHandshakeMsg = HandshakeType.ClientHello;
-
 				ContentType	contentType	= (ContentType)type;
 				byte[] buffer = this.ReadRecordBuffer(type, record);
 				if (buffer == null)
 				{
 					// record incomplete (at the moment)
@@ -456,13 +455,10 @@ public byte[] ReceiveRecord(Stream record)
 			}
 
 			// Try to read the Record Content Type
 			int type = recordTypeBuffer[0];
 
-			// Set last handshake message received to None
-			this.context.LastHandshakeMsg = HandshakeType.ClientHello;
-
 			ContentType	contentType	= (ContentType)type;
 			byte[] buffer = this.ReadRecordBuffer(type, record);
 			if (buffer == null)
 			{
 				// record incomplete (at the moment)
diff --git a/mcs/class/Mono.Security/Mono.Security.Protocol.Tls/ServerRecordProtocol.cs b/mcs/class/Mono.Security/Mono.Security.Protocol.Tls/ServerRecordProtocol.cs
index 6e316dc3659..31c2547902b 100644
--- a/mcs/class/Mono.Security/Mono.Security.Protocol.Tls/ServerRecordProtocol.cs
+++ b/mcs/class/Mono.Security/Mono.Security.Protocol.Tls/ServerRecordProtocol.cs
@@ -31,10 +31,12 @@
 
 namespace Mono.Security.Protocol.Tls
 {
 	internal class ServerRecordProtocol : RecordProtocol
 	{
+		TlsClientCertificate cert;
+		
 		#region Constructors
 
 		public ServerRecordProtocol(
 			Stream			innerStream, 
 			ServerContext	context) : base(innerStream, context)
@@ -91,34 +93,49 @@ protected override void ProcessHandshakeMessage(TlsStream handMsg)
 		#region Server Handshake Message Factories
 
 		private HandshakeMessage createClientHandshakeMessage(
 			HandshakeType type, byte[] buffer)
 		{
+			var last = context.LastHandshakeMsg;
 			switch (type)
 			{
 				case HandshakeType.ClientHello:
 					return new TlsClientHello(this.context, buffer);
 
 				case HandshakeType.Certificate:
-					return new TlsClientCertificate(this.context, buffer);
+					if (last != HandshakeType.ClientHello)
+						break;
+					cert = new TlsClientCertificate(this.context, buffer);
+					return cert;
 
 				case HandshakeType.ClientKeyExchange:
-					return new TlsClientKeyExchange(this.context, buffer);
+					if (last == HandshakeType.ClientHello || last == HandshakeType.Certificate)
+						return new TlsClientKeyExchange(this.context, buffer);
+					break;
 
 				case HandshakeType.CertificateVerify:
-					return new TlsClientCertificateVerify(this.context, buffer);
+					if (last == HandshakeType.ClientKeyExchange && cert != null)
+						return new TlsClientCertificateVerify(this.context, buffer);
+					break;
 
 				case HandshakeType.Finished:
-					return new TlsClientFinished(this.context, buffer);
-
+					// Certificates are optional, but if provided, they should send a CertificateVerify
+					bool check = (cert == null) ? (last == HandshakeType.ClientKeyExchange) : (last == HandshakeType.CertificateVerify);
+					// ChangeCipherSpecDone is not an handshake message (it's a content type) but still needs to be happens before finished
+					if (check && context.ChangeCipherSpecDone) {
+						context.ChangeCipherSpecDone = false;
+						return new TlsClientFinished(this.context, buffer);
+					}
+					break;
+					
 				default:
-					throw new TlsException(
-						AlertDescription.UnexpectedMessage,
-						String.Format(CultureInfo.CurrentUICulture,
-							"Unknown server handshake message received ({0})", 
-							type.ToString()));
+					throw new TlsException(AlertDescription.UnexpectedMessage, String.Format(CultureInfo.CurrentUICulture,
+														 "Unknown server handshake message received ({0})", 
+														 type.ToString()));
+					break;
 			}
+			throw new TlsException (AlertDescription.HandshakeFailiure, String.Format ("Protocol error, unexpected protocol transition from {0} to {1}", last, type));
 		}
 
 		private HandshakeMessage createServerHandshakeMessage(
 			HandshakeType type)
 		{

Vulnerable code: 					return new TlsServerKeyExchange(this.context, buffer);
					return new TlsServerCertificateRequest(this.context, buffer);
					return new TlsServerHelloDone(this.context, buffer);
					return new TlsServerFinished(this.context, buffer);

				// Set last handshake message received to None
				this.context.LastHandshakeMsg = HandshakeType.ClientHello;

			// Set last handshake message received to None
			this.context.LastHandshakeMsg = HandshakeType.ClientHello;

					return new TlsClientCertificate(this.context, buffer);
					return new TlsClientKeyExchange(this.context, buffer);
					return new TlsClientCertificateVerify(this.context, buffer);
					return new TlsClientFinished(this.context, buffer);

					throw new TlsException(
						AlertDescription.UnexpectedMessage,
						String.Format(CultureInfo.CurrentUICulture,
							"Unknown server handshake message received ({0})", 
							type.ToString()));
Fixed code: 			var last = context.LastHandshakeMsg;
					if (last != HandshakeType.HelloRequest)
						break;
					// Optional
					if (last != HandshakeType.ServerHello)
						break;
					// Optional
					// only for RSA_EXPORT
					if (last == HandshakeType.Certificate && context.Current.Cipher.IsExportable)
						return new TlsServerKeyExchange(this.context, buffer);
					break;
					// Optional
					if (last == HandshakeType.ServerKeyExchange || last == HandshakeType.Certificate)
						return new TlsServerCertificateRequest(this.context, buffer);
					break;
					if (last == HandshakeType.CertificateRequest || last == HandshakeType.Certificate || last == HandshakeType.ServerHello)
						return new TlsServerHelloDone(this.context, buffer);
					break;
					// depends if a full (ServerHelloDone) or an abbreviated handshake (ServerHello) is being done
					bool check = context.AbbreviatedHandshake ? (last == HandshakeType.ServerHello) : (last == HandshakeType.ServerHelloDone);
					// ChangeCipherSpecDone is not an handshake message (it's a content type) but still needs to be happens before finished
					if (check && context.ChangeCipherSpecDone) {
						context.ChangeCipherSpecDone = false;
						return new TlsServerFinished (this.context, buffer);
					}
					break;
					
			throw new TlsException (AlertDescription.HandshakeFailiure, String.Format ("Protocol error, unexpected protocol transition from {0} to {1}", last, type));
		public bool ChangeCipherSpecDone { get; set; }


			ctx.ChangeCipherSpecDone = true;
		TlsClientCertificate cert;
		
			var last = context.LastHandshakeMsg;
					if (last != HandshakeType.ClientHello)
						break;
					cert = new TlsClientCertificate(this.context, buffer);
					return cert;
					if (last == HandshakeType.ClientHello || last == HandshakeType.Certificate)
						return new TlsClientKeyExchange(this.context, buffer);
					break;
					if (last == HandshakeType.ClientKeyExchange && cert != null)
						return new TlsClientCertificateVerify(this.context, buffer);
					break;
					// Certificates are optional, but if provided, they should send a CertificateVerify
					bool check = (cert == null) ? (last == HandshakeType.ClientKeyExchange) : (last == HandshakeType.CertificateVerify);
					// ChangeCipherSpecDone is not an handshake message (it's a content type) but still needs to be happens before finished
					if (check && context.ChangeCipherSpecDone) {
						context.ChangeCipherSpecDone = false;
						return new TlsClientFinished(this.context, buffer);
					}
					break;
					
					throw new TlsException(AlertDescription.UnexpectedMessage, String.Format(CultureInfo.CurrentUICulture,
														 "Unknown server handshake message received ({0})", 
														 type.ToString()));
					break;
			throw new TlsException (AlertDescription.HandshakeFailiure, String.Format ("Protocol error, unexpected protocol transition from {0} to {1}", last, type));