[lgtm] Fix LGTM-reported issues (#1074)

Remember CodeQL (acfc1ef)?  CodeQL basically runs [GitHub LGTM][0]
on source code, looking for possible security issues.

Now that CodeQL is running, we can begin addressing reported issues.

Add a `.lgtm.yml` file to exclude `cs/campaign/constantine`; this is
a campaign asking for contact regarding certain constructs, and is
just noise in the LGTM reporting page.

Problems found include:

  * HttpClient created with CheckCertificateRevocationList disabled
  * Wrong type of arguments to formatting function
  * Weak cryptography
  * Possible information leakage from uninitialized padding bytes
  * ML Training and Serialization Files Referenced

~~ HttpClient created with CheckCertificateRevocationList disabled ~~

Apparently the `HttpClient` default constructor is "bad"; we should
instead use the [`HttpClient(HttpMessageHandler)` constructor][1],
provide our own `HttpClientHandler`, and ensure that
[`HttpClientHandler.CheckCertificateRevocationList`][2] is True.

~~ Wrong type of arguments to formatting function ~~

Apparently LGTM doesn't realize that in C++ `long int` is synonymous
with `long`, and thus warns that they're not the same. 🤦
Remove a cast to `long int`.

~~ Weak cryptography ~~

This is in `AuthDigestSession.cs`.  Unfortunately, RFC2617 requires
MD5, so we kinda need to use MD5.  Add a `// lgtm [cs/weak-crypto]`
comment to disable the warning.

~~ Possible information leakage from uninitialized padding bytes ~~

This is in `cpp-util.hh`, and it seems that LGTM doesn't appreciate
our use of template metaprogramming to construct a `char_array<Len+1>`
wherein `Len` is computed at compile time with no wasted padding.

~~ ML Training and Serialization Files Referenced ~~

LGTM apparently assumes that mentions of `.pb` are mentions of ML
data training files.  In our case, these were part of error messages
from `aapt2` that we were attempting to translate.

Add a `//lgtm [csharp/responsible-ai/ml-training-and-serialization-files-referenced]`
comment to disable this warning.

Co-authored-by: Alex Hsu <csigs@users.noreply.github.com>

[0]: https://github.com/marketplace/lgtm
[1]: https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httpclient.-ctor?view=netstandard-2.0#system-net-http-httpclient-ctor(system-net-http-httpmessagehandler)
[2]: https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httpclienthandler.checkcertificaterevocationlist?view=net-7.0

.lgtm.yml

@@ -0,0 +1,2 @@
+queries:
+  - exclude: cs/campaign/constantine

build-tools/xa-prep-tasks/Xamarin.Android.BuildTools.PrepTasks/DownloadUri.cs

@@ -43,7 +43,17 @@ public override bool Execute ()
 
 			var source  = cancellationTokenSource = new CancellationTokenSource ();
 			var tasks   = new Task<ITaskItem> [SourceUris.Length];
-			using (var client = new HttpClient ()) {
+
+			// LGTM recommendation
+			//
+			// Using HttpClient without providing a platform specific handler (WinHttpHandler or CurlHandler) where the CheckCertificateRevocationList property is set
+			// to true, will allow revoked certificates to be accepted by the HttpClient as valid.
+			//
+			var handler = new HttpClientHandler {
+				CheckCertificateRevocationList = true,
+			};
+
+			using (var client = new HttpClient (handler)) {
 				client.Timeout = TimeSpan.FromHours (3);
 				for (int i = 0; i < SourceUris.Length; ++i) {
 					tasks [i] = DownloadFile (client, source, SourceUris [i], DestinationFiles [i]);

build-tools/xaprepare/xaprepare/Application/Utilities.cs

@@ -471,12 +471,21 @@ static decimal SignificantDigits (decimal number, int maxDigitCount)
 			return (success, size);
 		}
 
+		public static HttpClient CreateHttpClient ()
+		{
+			var handler = new HttpClientHandler {
+				CheckCertificateRevocationList = true,
+			};
+
+			return new HttpClient (handler);
+		}
+
 		public static async Task<(bool success, ulong size, HttpStatusCode status)> GetDownloadSizeWithStatus (Uri url)
 		{
 			TimeSpan delay = ExceptionRetryInitialDelay;
 			for (int i = 0; i < ExceptionRetries; i++) {
 				try {
-					using (var httpClient = new HttpClient ()) {
+					using (HttpClient httpClient = CreateHttpClient ()) {
 						httpClient.Timeout = WebRequestTimeout;
 						var req = new HttpRequestMessage (HttpMethod.Head, url);
 						req.Headers.ConnectionClose = true;
@@ -524,7 +533,7 @@ public static async Task<bool> Download (Uri url, string targetFile, DownloadSta
 
 		static async Task DoDownload (Uri url, string targetFile, DownloadStatus status)
 		{
-			using (var httpClient = new HttpClient ()) {
+			using (HttpClient httpClient = CreateHttpClient ()) {
 				httpClient.Timeout = WebRequestTimeout;
 				Log.DebugLine ("Calling GetAsync");
 				HttpResponseMessage resp = await httpClient.GetAsync (url, HttpCompletionOption.ResponseHeadersRead);
@@ -820,7 +829,7 @@ public static string GetStringFromStdout (ProcessRunner runner, bool throwOnErro
 					LogError ($"failed with exit code {runner.ExitCode}");
 					return String.Empty;
 				}
-				
+
 				string ret = sw.ToString ();
 				if (trimTrailingWhitespace)
 					return ret.TrimEnd ();

build-tools/xaprepare/xaprepare/Steps/Step_Get_Windows_Binutils.cs

@@ -157,7 +157,7 @@ string GetStampFile (string file, string destinationDirectory, string ndkVersion
 		async Task<bool> FetchFiles (Dictionary<string, string> neededFiles, string destinationDirectory, Uri url)
 		{
 			Utilities.CreateDirectory (destinationDirectory);
-			using (var httpClient = new HttpClient ()) {
+			using (HttpClient httpClient = Utilities.CreateHttpClient ()) {
 				bool success;
 				long size;
 

src-ThirdParty/NUnitLite/Constraints/ConstraintExpression.cs

@@ -350,20 +350,6 @@ public UniqueItemsConstraint Unique
 
         #endregion
 
-        #region BinarySerializable
-
-#if !NETCF && !SILVERLIGHT
-        /// <summary>
-        /// Returns a constraint that tests whether an object graph is serializable in binary format.
-        /// </summary>
-        public BinarySerializableConstraint BinarySerializable
-        {
-            get { return (BinarySerializableConstraint)this.Append(new BinarySerializableConstraint()); }
-        }
-#endif
-
-        #endregion
-
         #region XmlSerializable
 
 #if !SILVERLIGHT

src-ThirdParty/NUnitLite/Constraints/ConstraintFactory.cs

@@ -297,20 +297,6 @@ public UniqueItemsConstraint Unique
 
         #endregion
 
-        #region BinarySerializable
-
-#if !NETCF && !SILVERLIGHT
-        /// <summary>
-        /// Returns a constraint that tests whether an object graph is serializable in binary format.
-        /// </summary>
-        public BinarySerializableConstraint BinarySerializable
-        {
-            get { return new BinarySerializableConstraint(); }
-        }
-#endif
-
-        #endregion
-
         #region XmlSerializable
 
 #if !SILVERLIGHT

src-ThirdParty/NUnitLite/Is.cs

@@ -157,20 +157,6 @@ public static UniqueItemsConstraint Unique
 
         #endregion
 
-        #region BinarySerializable
-
-#if !NETCF && !SILVERLIGHT
-        /// <summary>
-        /// Returns a constraint that tests whether an object graph is serializable in binary format.
-        /// </summary>
-        public static BinarySerializableConstraint BinarySerializable
-        {
-            get { return new BinarySerializableConstraint(); }
-        }
-#endif
-
-        #endregion
-
         #region XmlSerializable
 
 #if !SILVERLIGHT

src/Mono.Android/Xamarin.Android.Net/AuthDigestSession.cs

@@ -2,7 +2,7 @@
 // Adapted from:
 //
 // System.Net.DigestClient.cs
-//                                      
+//
 // Authors:
 //      Greg Reinacker (gregr@rassoc.com)
 //      Sebastien Pouliot (spouliot@motus.com)
@@ -55,7 +55,7 @@ public string? QOP {
 		}
 
 		public string CNonce {
-			get { 
+			get {
 				if (_cnonce == null) {
 					// 15 is a multiple of 3 which is better for base64 because it
 					// wont end with '=' and risk messing up the server parsing
@@ -73,20 +73,20 @@ public DateTime LastUse {
 		}
 
 		[System.Diagnostics.CodeAnalysis.SuppressMessage ("Security", "CA5351:Do Not Use Broken Cryptographic Algorithms", Justification = "Only supported algorithm by RFC2617.")]
-		public bool Parse (string challenge) 
+		public bool Parse (string challenge)
 		{
 			parser = new AuthDigestHeaderParser (challenge);
 			if (!parser.Parse ())
 				return false;
 
 			// build the hash object (only MD5 is defined in RFC2617)
 			if ((parser.Algorithm == null) || (parser.Algorithm.StartsWith ("MD5", StringComparison.OrdinalIgnoreCase)))
-				hash = MD5.Create ();
+				hash = MD5.Create (); // lgtm [cs/weak-crypto] This is part of RFC2617 and we cannot change the algorithm here.
 
 			return true;
 		}
 
-		string? HashToHexString (string toBeHashed) 
+		string? HashToHexString (string toBeHashed)
 		{
 			if (hash == null)
 				return null;
@@ -100,26 +100,26 @@ public bool Parse (string challenge)
 			return sb.ToString ();
 		}
 
-		string? HA1 (string username, string password) 
+		string? HA1 (string username, string password)
 		{
 			string ha1 = $"{username}:{Realm}:{password}";
 			if (String.Compare (Algorithm, "md5-sess", StringComparison.OrdinalIgnoreCase) == 0)
 				ha1 = $"{HashToHexString (ha1)}:{Nonce}:{CNonce}";
 			return HashToHexString (ha1);
 		}
 
-		string? HA2 (HttpURLConnection webRequest) 
+		string? HA2 (HttpURLConnection webRequest)
 		{
 			var uri = new Uri (webRequest.URL?.ToString ()!);
 			string ha2 = $"{webRequest.RequestMethod}:{uri.PathAndQuery}";
 			if (QOP == "auth-int") {
 				// TODO
 				// ha2 += String.Format (":{0}", hentity);
-			}		
+			}
 			return HashToHexString (ha2);
 		}
 
-		string? Response (string username, string password, HttpURLConnection webRequest) 
+		string? Response (string username, string password, HttpURLConnection webRequest)
 		{
 			string response = $"{HA1 (username, password)}:{Nonce}:";
 			if (QOP != null)
@@ -128,13 +128,13 @@ public bool Parse (string challenge)
 			return HashToHexString (response);
 		}
 
-		public Authorization? Authenticate (HttpURLConnection request, ICredentials credentials) 
+		public Authorization? Authenticate (HttpURLConnection request, ICredentials credentials)
 		{
 			if (parser == null)
 				throw new InvalidOperationException ();
 			if (request == null)
 				return null;
-	
+
 			lastUse = DateTime.Now;
 			var uri = new Uri (request.URL?.ToString ()!);
 			NetworkCredential cred = credentials.GetCredential (uri, "digest");

src/Xamarin.Android.Build.Tasks/Tasks/Aapt2.cs

@@ -314,7 +314,7 @@ static string GetErrorCode (string message)
 			Tuple.Create ("APT2097", "failed to open directory"),
 			Tuple.Create ("APT2098", "failed to open file"),
 			Tuple.Create ("APT2099", "failed to open resources.arsc"),
-			Tuple.Create ("APT2100", "failed to open resources.pb"),
+			Tuple.Create ("APT2100", "failed to open resources.pb"), // lgtm [csharp/responsible-ai/ml-training-and-serialization-files-referenced] These are not the droids you are looking for. Not ML data training files.
 			Tuple.Create ("APT2101", "failed to open"),
 			Tuple.Create ("APT2102", "failed to parse binary XML"),
 			Tuple.Create ("APT2103", "failed to parse binary"),
@@ -367,7 +367,7 @@ static string GetErrorCode (string message)
 			Tuple.Create ("APT2150", "invalid preferred density"),
 			Tuple.Create ("APT2151", "invalid resource ID"),
 			Tuple.Create ("APT2152", "invalid resource name"),
-			Tuple.Create ("APT2153", "invalid resources.pb"),
+			Tuple.Create ("APT2153", "invalid resources.pb"), // lgtm [csharp/responsible-ai/ml-training-and-serialization-files-referenced] These are not the droids you are looking for. Not ML data training files.
 			Tuple.Create ("APT2154", "invalid split name"),
 			Tuple.Create ("APT2155", "invalid split parameter"),
 			Tuple.Create ("APT2156", "invalid static library"),