Skip to content

Commit

Permalink
[security] Add nullability to (generated and manual) bindings (#14884)
Browse files Browse the repository at this point in the history
* Add nullability

* throw better null exceptions

* use is null and is not null AND fix spacing

* Rolfs suggestions and redo spacing

* missed a few spaces

* Found one more elusive space

Co-authored-by: TJ Lambert <tjlambert@microsoft.com>
  • Loading branch information
tj-devel709 and TJ Lambert authored May 12, 2022
1 parent 911cdf3 commit fcf1a19
Show file tree
Hide file tree
Showing 23 changed files with 237 additions and 196 deletions.
22 changes: 11 additions & 11 deletions src/Security/Authorization.cs
Original file line number Diff line number Diff line change
Expand Up @@ -235,27 +235,27 @@ static void EncodeString (ref AuthorizationItem item, string key, string? value)

try {
unsafe {
if (parameters != null){
if (parameters is not null){
ppars = &pars;
pars.ptrToAuthorization = (AuthorizationItem *) Marshal.AllocHGlobal (sizeof (AuthorizationItem) * 2);
if (parameters.PathToSystemPrivilegeTool != null)
if (parameters.PathToSystemPrivilegeTool is not null)
EncodeString (ref pars.ptrToAuthorization [pars.count++], "system.privilege.admin", parameters.PathToSystemPrivilegeTool);
if (parameters.IconPath != null)
if (parameters.IconPath is not null)
EncodeString (ref pars.ptrToAuthorization [pars.count++], "icon", parameters.IconPath);
}
if (environment != null || (parameters != null && parameters.Prompt != null)){
if (environment is not null || (parameters is not null && parameters.Prompt is not null)){
penv = &env;
env.ptrToAuthorization = (AuthorizationItem *) Marshal.AllocHGlobal (sizeof (AuthorizationItem) * 4);
if (environment != null){
if (environment.Username != null)
if (environment is not null){
if (environment.Username is not null)
EncodeString (ref env.ptrToAuthorization [env.count++], "username", environment.Username);
if (environment.Password != null)
if (environment.Password is not null)
EncodeString (ref env.ptrToAuthorization [env.count++], "password", environment.Password);
if (environment.AddToSharedCredentialPool)
EncodeString (ref env.ptrToAuthorization [env.count++], "shared", null);
}
if (parameters != null){
if (parameters.Prompt != null)
if (parameters is not null){
if (parameters.Prompt is not null)
EncodeString (ref env.ptrToAuthorization [env.count++], "prompt", parameters.Prompt);
}
}
Expand All @@ -265,14 +265,14 @@ static void EncodeString (ref AuthorizationItem item, string key, string? value)
return new Authorization (auth, true);
}
} finally {
if (ppars != null){
if (ppars is not null){
for (int i = 0; i < pars.count; i++){
Marshal.FreeHGlobal (pars.ptrToAuthorization [i].name);
Marshal.FreeHGlobal (pars.ptrToAuthorization [i].value);
}
Marshal.FreeHGlobal ((IntPtr)pars.ptrToAuthorization);
}
if (penv != null){
if (penv is not null){
for (int i = 0; i < env.count; i++){
Marshal.FreeHGlobal (env.ptrToAuthorization [i].name);
if (env.ptrToAuthorization [i].value != IntPtr.Zero)
Expand Down
54 changes: 27 additions & 27 deletions src/Security/Certificate.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,15 @@ internal SecCertificate (NativeHandle handle, bool owns)
public SecCertificate (NSData data)
{
if (data is null)
throw new ArgumentNullException (nameof (data));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (data));

Initialize (data);
}

public SecCertificate (byte[] data)
{
if (data is null)
throw new ArgumentNullException (nameof (data));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (data));

using (NSData cert = NSData.FromArray (data)) {
Initialize (cert);
Expand All @@ -89,7 +89,7 @@ public SecCertificate (byte[] data)
public SecCertificate (X509Certificate certificate)
{
if (certificate is null)
throw new ArgumentNullException (nameof (certificate));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (certificate));

#if NATIVE_APPLE_CERTIFICATE
var handle = certificate.Impl.GetNativeAppleCertificate ();
Expand Down Expand Up @@ -124,7 +124,7 @@ internal SecCertificate (X509CertificateImpl impl)
public SecCertificate (X509Certificate2 certificate)
{
if (certificate is null)
throw new ArgumentNullException (nameof (certificate));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (certificate));

#if NATIVE_APPLE_CERTIFICATE
var handle = certificate.Impl.GetNativeAppleCertificate ();
Expand Down Expand Up @@ -197,9 +197,9 @@ internal static bool Equals (SecCertificate first, SecCertificate second)
* SecCertificateRef's for equality.
*/
if (first is null)
throw new ArgumentNullException (nameof (first));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (first));
if (second is null)
throw new ArgumentNullException (nameof (second));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (second));
if (first.Handle == second.Handle)
return true;

Expand Down Expand Up @@ -601,7 +601,7 @@ public SecCertificate Certificate {
public static SecIdentity Import (byte[] data, string password)
{
if (data is null)
throw new ArgumentNullException (nameof (data));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (data));
if (string.IsNullOrEmpty (password)) // SecPKCS12Import() doesn't allow empty passwords.
throw new ArgumentException (nameof (password));
using (var pwstring = new NSString (password))
Expand All @@ -618,7 +618,7 @@ public static SecIdentity Import (byte[] data, string password)
public static SecIdentity Import (X509Certificate2 certificate)
{
if (certificate is null)
throw new ArgumentNullException (nameof (certificate));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (certificate));
if (!certificate.HasPrivateKey)
throw new InvalidOperationException ("Need X509Certificate2 with a private key.");

Expand Down Expand Up @@ -713,7 +713,7 @@ public SecKey (NativeHandle handle, bool owns)
public static SecStatusCode GenerateKeyPair (NSDictionary parameters, out SecKey? publicKey, out SecKey? privateKey)
{
if (parameters is null)
throw new ArgumentNullException (nameof (parameters));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (parameters));

IntPtr pub, priv;

Expand Down Expand Up @@ -832,7 +832,7 @@ public SecStatusCode RawSign (SecPadding padding, IntPtr dataToSign, int dataToS
public unsafe SecStatusCode RawSign (SecPadding padding, byte [] dataToSign, out byte [] result)
{
if (dataToSign is null)
throw new ArgumentNullException (nameof (dataToSign));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (dataToSign));

fixed (byte *bp = dataToSign)
return _RawSign (padding, (IntPtr) bp, dataToSign.Length, out result);
Expand Down Expand Up @@ -903,9 +903,9 @@ public unsafe SecStatusCode RawVerify (SecPadding padding, IntPtr signedData, in
public SecStatusCode RawVerify (SecPadding padding, byte [] signedData, byte [] signature)
{
if (signature is null)
throw new ArgumentNullException (nameof (signature));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (signature));
if (signedData is null)
throw new ArgumentNullException (nameof (signedData));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (signedData));
unsafe {
// SecKeyRawVerify will try to read from the signedData/signature pointers even if
// the corresponding length is 0, which may crash (happens in Xcode 11 beta 1)
Expand Down Expand Up @@ -972,9 +972,9 @@ public unsafe SecStatusCode Encrypt (SecPadding padding, IntPtr plainText, nint
public SecStatusCode Encrypt (SecPadding padding, byte [] plainText, byte [] cipherText)
{
if (cipherText is null)
throw new ArgumentNullException (nameof (cipherText));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (cipherText));
if (plainText is null)
throw new ArgumentNullException (nameof (plainText));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (plainText));
unsafe {
fixed (byte *cp = cipherText)
fixed (byte *pp = plainText) {
Expand Down Expand Up @@ -1043,7 +1043,7 @@ public unsafe SecStatusCode Decrypt (SecPadding padding, IntPtr cipherText, nint
SecStatusCode _Decrypt (SecPadding padding, byte [] cipherText, ref byte []? plainText)
{
if (cipherText is null)
throw new ArgumentNullException (nameof (cipherText));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (cipherText));

unsafe {
fixed (byte *cp = cipherText) {
Expand Down Expand Up @@ -1094,7 +1094,7 @@ public SecStatusCode Decrypt (SecPadding padding, byte [] cipherText, out byte [
static public SecKey? CreateRandomKey (NSDictionary parameters, out NSError? error)
{
if (parameters is null)
throw new ArgumentNullException (nameof (parameters));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (parameters));

IntPtr err;
var key = SecKeyCreateRandomKey (parameters.Handle, out err);
Expand Down Expand Up @@ -1137,7 +1137,7 @@ public SecStatusCode Decrypt (SecPadding padding, byte [] cipherText, out byte [
static public SecKey? CreateRandomKey (SecKeyGenerationParameters parameters, out NSError? error)
{
if (parameters is null)
throw new ArgumentNullException (nameof (parameters));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (parameters));
if (parameters.KeyType == SecKeyType.Invalid)
throw new ArgumentException ("invalid 'SecKeyType'", "SecKeyGeneration.KeyType");

Expand Down Expand Up @@ -1174,9 +1174,9 @@ public SecStatusCode Decrypt (SecPadding padding, byte [] cipherText, out byte [
static public SecKey? Create (NSData keyData, NSDictionary parameters, out NSError? error)
{
if (keyData is null)
throw new ArgumentNullException (nameof (keyData));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (keyData));
if (parameters is null)
throw new ArgumentNullException (nameof (parameters));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (parameters));

IntPtr err;
var key = SecKeyCreateWithData (keyData.Handle, parameters.Handle, out err);
Expand Down Expand Up @@ -1376,7 +1376,7 @@ public bool IsAlgorithmSupported (SecKeyOperationType operation, SecKeyAlgorithm
public NSData? CreateSignature (SecKeyAlgorithm algorithm, NSData dataToSign, out NSError? error)
{
if (dataToSign is null)
throw new ArgumentNullException (nameof (dataToSign));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (dataToSign));

var data = SecKeyCreateSignature (Handle, algorithm.GetConstant ().GetHandle (), dataToSign.Handle, out var err);
error = Runtime.GetNSObject<NSError> (err);
Expand Down Expand Up @@ -1412,9 +1412,9 @@ public bool IsAlgorithmSupported (SecKeyOperationType operation, SecKeyAlgorithm
public bool VerifySignature (SecKeyAlgorithm algorithm, NSData signedData, NSData signature, out NSError? error)
{
if (signedData is null)
throw new ArgumentNullException (nameof (signedData));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (signedData));
if (signature is null)
throw new ArgumentNullException (nameof (signature));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (signature));

var result = SecKeyVerifySignature (Handle, algorithm.GetConstant ().GetHandle (), signedData.Handle, signature.Handle, out var err);
error = Runtime.GetNSObject<NSError> (err);
Expand Down Expand Up @@ -1449,7 +1449,7 @@ public bool VerifySignature (SecKeyAlgorithm algorithm, NSData signedData, NSDat
public NSData? CreateEncryptedData (SecKeyAlgorithm algorithm, NSData plaintext, out NSError? error)
{
if (plaintext is null)
throw new ArgumentNullException (nameof (plaintext));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (plaintext));

var data = SecKeyCreateEncryptedData (Handle, algorithm.GetConstant ().GetHandle (), plaintext.Handle, out var err);
error = Runtime.GetNSObject<NSError> (err);
Expand Down Expand Up @@ -1484,7 +1484,7 @@ public bool VerifySignature (SecKeyAlgorithm algorithm, NSData signedData, NSDat
public NSData? CreateDecryptedData (SecKeyAlgorithm algorithm, NSData ciphertext, out NSError? error)
{
if (ciphertext is null)
throw new ArgumentNullException (nameof (ciphertext));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (ciphertext));

var data = SecKeyCreateDecryptedData (Handle, algorithm.GetConstant ().GetHandle (), ciphertext.Handle, out var err);
error = Runtime.GetNSObject<NSError> (err);
Expand Down Expand Up @@ -1519,9 +1519,9 @@ public bool VerifySignature (SecKeyAlgorithm algorithm, NSData signedData, NSDat
public NSData? GetKeyExchangeResult (SecKeyAlgorithm algorithm, SecKey publicKey, NSDictionary parameters, out NSError? error)
{
if (publicKey is null)
throw new ArgumentNullException (nameof (publicKey));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (publicKey));
if (parameters is null)
throw new ArgumentNullException (nameof (parameters));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (parameters));

var data = SecKeyCopyKeyExchangeResult (Handle, algorithm.GetConstant ().GetHandle (), publicKey.Handle, parameters.Handle, out var err);
error = Runtime.GetNSObject<NSError> (err);
Expand All @@ -1542,7 +1542,7 @@ public bool VerifySignature (SecKeyAlgorithm algorithm, NSData signedData, NSDat
public NSData? GetKeyExchangeResult (SecKeyAlgorithm algorithm, SecKey publicKey, SecKeyKeyExchangeParameter parameters, out NSError? error)
{
if (parameters is null)
throw new ArgumentNullException (nameof (parameters));
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (parameters));

return GetKeyExchangeResult (algorithm, publicKey, parameters.Dictionary!, out error);
}
Expand Down
2 changes: 2 additions & 0 deletions src/Security/CssmKeyUse.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
// THE SOFTWARE.
//

#nullable enable

#if MONOMAC

using System;
Expand Down
2 changes: 2 additions & 0 deletions src/Security/CssmTPAppleCertStatus.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
// THE SOFTWARE.
//

#nullable enable

#if MONOMAC

using System;
Expand Down
6 changes: 4 additions & 2 deletions src/Security/ImportExport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
// WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
//

#nullable enable

using System;
using System.Runtime.InteropServices;
using ObjCRuntime;
Expand All @@ -48,8 +50,8 @@ static public SecStatusCode ImportPkcs12 (byte[] buffer, NSDictionary options, o

static public SecStatusCode ImportPkcs12 (NSData data, NSDictionary options, out NSDictionary[] array)
{
if (options == null)
throw new ArgumentNullException ("options");
if (options is null)
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (options));

IntPtr handle;
SecStatusCode code = SecPKCS12Import (data.Handle, options.Handle, out handle);
Expand Down
Loading

5 comments on commit fcf1a19

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ [CI Build] Tests on macOS Mac Catalina (10.15) failed ❌

Failed tests are:

  • linksdk

Pipeline on Agent
Hash: fcf1a196317e2ebdeef52dd288c00ff5acc39370

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📋 [CI Build] API Diff 📋

API Current PR diff

ℹ️ API Diff (from PR only) (please review changes)

View API diff
View dotnet API diff
View dotnet legacy API diff
View dotnet iOS-MacCatalayst API diff

API diff

✅ API Diff from stable

View API diff
View dotnet API diff
View dotnet legacy API diff
View dotnet iOS-MacCatalayst API diff

Generator diff

Generator Diff (no change)

Pipeline on Agent XAMMINI-049.Monterey'
Hash: fcf1a196317e2ebdeef52dd288c00ff5acc39370

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ [CI Build] Tests on macOS M1 - Mac Big Sur (11.5) failed ❌

Failed tests are:

  • dontlink
  • linksdk
  • linkall
  • xammac_tests
  • monotouch-test

Pipeline on Agent
Hash: fcf1a196317e2ebdeef52dd288c00ff5acc39370

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📚 [CI Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent XAMMINI-062.Monterey'
Hash: fcf1a196317e2ebdeef52dd288c00ff5acc39370

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ [CI Build] Tests failed on VSTS: simulator tests iOS ❌

Tests failed on VSTS: simulator tests iOS.

Test results

23 tests failed, 211 tests passed.

Failed tests

  • link sdk/Mac [dotnet]/Debug [dotnet]: Failed (Test run failed.
    Tests run: 117 Passed: 107 Inconclusive: 0 Failed: 2 Ignored: 8)
  • link sdk/Mac [dotnet]/Release [dotnet]: Failed (Test run failed.
    Tests run: 117 Passed: 107 Inconclusive: 0 Failed: 1 Ignored: 9)
  • link sdk/Mac Catalyst [dotnet]/Debug [dotnet]: Failed (Test run failed.
    Tests run: 129 Passed: 120 Inconclusive: 0 Failed: 1 Ignored: 8)
  • link sdk/Mac Catalyst [dotnet]/Release [dotnet]: Failed (Test run failed.
    Tests run: 129 Passed: 118 Inconclusive: 0 Failed: 2 Ignored: 9)
  • link sdk/Mac Modern/Release: Failed (Test run failed.
    Tests run: 9 Passed: 8 Inconclusive: 0 Failed: 1 Ignored: 0)
  • link sdk/iOS Unified 64-bits - simulator/Debug [dotnet]: Failed
  • link sdk/iOS Unified 64-bits - simulator/Release [dotnet]: Failed
  • link sdk/iOS Unified 64-bits - simulator/Debug: Failed
  • link sdk/iOS Unified 64-bits - simulator/Release: Failed
  • link sdk/tvOS - simulator/Debug [dotnet]: Failed
  • link sdk/tvOS - simulator/Release [dotnet]: Failed
  • link sdk/tvOS - simulator/Debug: Failed
  • link sdk/tvOS - simulator/Release: Failed
  • trimmode link/Mac [dotnet]/Debug [dotnet]: Failed (Test run failed.
    Tests run: 117 Passed: 108 Inconclusive: 0 Failed: 1 Ignored: 8)
  • trimmode link/Mac [dotnet]/Release [dotnet]: Failed (Test run failed.
    Tests run: 117 Passed: 106 Inconclusive: 0 Failed: 2 Ignored: 9)
  • trimmode link/Mac Catalyst [dotnet]/Debug [dotnet]: Failed (Test run failed.
    Tests run: 129 Passed: 120 Inconclusive: 0 Failed: 1 Ignored: 8)
  • trimmode link/Mac Catalyst [dotnet]/Release [dotnet]: Failed (Test run failed.
    Tests run: 129 Passed: 118 Inconclusive: 0 Failed: 2 Ignored: 9)
  • trimmode link/iOS Unified 64-bits - simulator/Debug [dotnet]: Failed
  • trimmode link/iOS Unified 64-bits - simulator/Release [dotnet]: Failed
  • trimmode link/tvOS - simulator/Debug [dotnet]: Failed
  • trimmode link/tvOS - simulator/Release [dotnet]: Failed
  • link all/Mac Modern/Debug: Failed (Test run failed.
    Tests run: 20 Passed: 18 Inconclusive: 0 Failed: 1 Ignored: 1)
  • link all/Mac Modern/Release: Failed (Test run failed.
    Tests run: 20 Passed: 18 Inconclusive: 0 Failed: 1 Ignored: 1)

Pipeline on Agent XAMBOT-1030.Monterey'
[security] Add nullability to (generated and manual) bindings (#14884)

Please sign in to comment.