From 45a9548644f34264ce2e1a8a586e8e234d6f61e4 Mon Sep 17 00:00:00 2001 From: Jeremy Barton Date: Mon, 14 Feb 2022 16:24:52 -0800 Subject: [PATCH] Clean the OpenSSL error queue more consistently This change started with the intent of changing the error code / message from an OpenSSL-based exception from the most recent error in the queue to the oldest error that was produced after the operation started. This was motivated mostly by EVP_PKEY2PKCS8(pkey) incorrectly indicating a malloc failure after producing the original/correct error that pkey did not have a private key portion. Having fully developed the experiment, data showed that while for EVP_PKEY2PKCS8 the first (of two) errors was the better one, for everything else with more than one error reported, the last error was at least as good as, and often better, than the first error. With that data in hand, this change now represents more consistently cleaning the error queue, and reducing the overhead in producing the exception objects. Co-authored-by: Kevin Jones --- .../Interop.ERR.cs | 51 ++++++------ .../entrypoints.c | 2 +- .../openssl.c | 51 ++++++++++++ .../pal_asn1.c | 14 ++++ .../pal_bignum.c | 5 ++ .../pal_bio.c | 7 ++ .../pal_dsa.c | 11 +++ .../pal_ecc_import_export.c | 9 +++ .../pal_ecdsa.c | 4 + .../pal_eckey.c | 11 +++ .../pal_err.c | 6 +- .../pal_err.h | 7 +- .../pal_evp.c | 33 +++++++- .../pal_evp_cipher.c | 56 +++++++++++++ .../pal_evp_pkey.c | 17 ++++ .../pal_evp_pkey_dsa.c | 2 + .../pal_evp_pkey_ecdh.c | 4 + .../pal_evp_pkey_eckey.c | 2 + .../pal_evp_pkey_rsa.c | 12 +++ .../pal_hmac.c | 21 +++++ .../pal_ocsp.c | 4 + .../pal_pkcs7.c | 9 +++ .../pal_ssl.c | 78 +++++++++++++++++-- .../pal_x509.c | 76 ++++++++++++++++-- .../pal_x509_name.c | 2 + .../pal_x509_root.c | 4 + .../pal_x509ext.c | 6 ++ 27 files changed, 456 insertions(+), 48 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.ERR.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.ERR.cs index 5b70ebabacb1e..66b465b65e808 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.ERR.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.ERR.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Buffers; using System.Diagnostics; using System.Runtime.InteropServices; using System.Security.Cryptography; @@ -13,8 +14,8 @@ internal static partial class Crypto [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_ErrClearError")] internal static partial ulong ErrClearError(); - [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_ErrGetErrorAlloc")] - private static partial ulong ErrGetErrorAlloc([MarshalAs(UnmanagedType.Bool)] out bool isAllocFailure); + [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_ErrGetExceptionError")] + private static partial ulong ErrGetExceptionError([MarshalAs(UnmanagedType.Bool)] out bool isAllocFailure); [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_ErrPeekError")] internal static partial ulong ErrPeekError(); @@ -30,43 +31,38 @@ internal static partial class Crypto private static unsafe string ErrErrorStringN(ulong error) { - var buffer = new byte[1024]; + byte[] buffer = ArrayPool.Shared.Rent(1024); + string ret; + fixed (byte* buf = &buffer[0]) { ErrErrorStringN(error, buf, buffer.Length); - return Marshal.PtrToStringAnsi((IntPtr)buf)!; + ret = Marshal.PtrToStringAnsi((IntPtr)buf)!; } + + ArrayPool.Shared.Return(buffer); + return ret; } internal static Exception CreateOpenSslCryptographicException() { - // The Windows cryptography library reports error codes through - // Marshal.GetLastWin32Error, which has a single value when the - // function exits, last writer wins. + // The Windows cryptography libraries reports error codes through + // return values, or Marshal.GetLastWin32Error, either of which + // has a single value when the function exits. // // OpenSSL maintains an error queue. Calls to ERR_get_error read // values out of the queue in the order that ERR_set_error wrote // them. Nothing enforces that a single call into an OpenSSL - // function will guarantee at-most one error being set. + // function will guarantee at-most one error being set, and there + // are well-known cases where multiple errors are emitted. // - // In order to maintain parity in how error flows look between the - // Windows code and the OpenSSL-calling code, drain the queue - // whenever an Exception is desired, and report the exception - // related to the last value in the queue. - bool isAllocFailure; - ulong error = ErrGetErrorAlloc(out isAllocFailure); - ulong lastRead = error; - bool lastIsAllocFailure = isAllocFailure; - - // 0 (there's no named constant) is only returned when the calls - // to ERR_get_error exceed the calls to ERR_set_error. - while (lastRead != 0) - { - error = lastRead; - isAllocFailure = lastIsAllocFailure; - - lastRead = ErrGetErrorAlloc(out lastIsAllocFailure); - } + // In older versions of .NET, we collected the last error in the + // queue, by repeatedly calling into ERR_get_error from managed code + // and using the last error as the basis of the exception. + // Now, we call into the shim once, which is responsible for + // maintaining the error state and informing us of the one value to report. + // (and when fetching that error we go ahead and clear out the rest). + ulong error = ErrGetExceptionError(out bool isAllocFailure); // If we're in an error flow which results in an Exception, but // no calls to ERR_set_error were made, throw the unadorned @@ -82,7 +78,8 @@ internal static Exception CreateOpenSslCryptographicException() } // Even though ErrGetError returns ulong (C++ unsigned long), we - // really only expect error codes in the UInt32 range + // really only expect error codes in the UInt32 range, since that + // type is only 32 bits on x86 Linux. Debug.Assert(error <= uint.MaxValue, "ErrGetError should only return error codes in the UInt32 range."); // If there was an error code, and it wasn't something handled specially, diff --git a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c index 3ee4300de95d1..0518fa7987b74 100644 --- a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c +++ b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c @@ -89,7 +89,7 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_EncodeX509SubjectPublicKeyInfo) DllImportEntry(CryptoNative_ErrClearError) DllImportEntry(CryptoNative_ErrErrorStringN) - DllImportEntry(CryptoNative_ErrGetErrorAlloc) + DllImportEntry(CryptoNative_ErrGetExceptionError) DllImportEntry(CryptoNative_ErrPeekError) DllImportEntry(CryptoNative_ErrPeekLastError) DllImportEntry(CryptoNative_ErrReasonErrorString) diff --git a/src/native/libs/System.Security.Cryptography.Native/openssl.c b/src/native/libs/System.Security.Cryptography.Native/openssl.c index c18eb0ffb7c6b..4d1e7aec454cb 100644 --- a/src/native/libs/System.Security.Cryptography.Native/openssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/openssl.c @@ -81,6 +81,8 @@ int32_t CryptoNative_GetX509Thumbprint(X509* x509, uint8_t* pBuf, int32_t cBuf) return -SHA_DIGEST_LENGTH; } + ERR_clear_error(); + if (!X509_digest(x509, EVP_sha1(), pBuf, NULL)) { return 0; @@ -102,6 +104,8 @@ otherwise. */ const ASN1_TIME* CryptoNative_GetX509NotBefore(X509* x509) { + // No error queue impact. + if (x509) { return X509_get0_notBefore(x509); @@ -123,6 +127,8 @@ otherwise. */ const ASN1_TIME* CryptoNative_GetX509NotAfter(X509* x509) { + // No error queue impact. + if (x509) { return X509_get0_notAfter(x509); @@ -144,6 +150,8 @@ otherwise. */ const ASN1_TIME* CryptoNative_GetX509CrlNextUpdate(X509_CRL* crl) { + // No error queue impact. + if (crl) { return X509_CRL_get0_nextUpdate(crl); @@ -168,6 +176,9 @@ The encoded value of the version, otherwise: */ int32_t CryptoNative_GetX509Version(X509* x509) { + // No errors are expected to be written to the queue on this call, + // and the managed caller doesn't check for one. + if (x509) { return (int32_t)X509_get_version(x509); @@ -189,6 +200,8 @@ describing the object type. */ ASN1_OBJECT* CryptoNative_GetX509PublicKeyAlgorithm(X509* x509) { + // No error queue impact, all of the called routines are just field accessors. + if (x509) { X509_PUBKEY* pubkey = X509_get_X509_PUBKEY(x509); @@ -216,6 +229,8 @@ describing the object type. */ ASN1_OBJECT* CryptoNative_GetX509SignatureAlgorithm(X509* x509) { + // No error queue impact. + if (x509) { const X509_ALGOR* sigAlg = X509_get0_tbs_sigalg(x509); @@ -243,6 +258,8 @@ Any negative value: The input buffer size was reported as insufficient. A buffer */ int32_t CryptoNative_GetX509PublicKeyParameterBytes(X509* x509, uint8_t* pBuf, int32_t cBuf) { + ERR_clear_error(); + if (!x509) { return 0; @@ -302,6 +319,8 @@ the public key. */ ASN1_BIT_STRING* CryptoNative_GetX509PublicKeyBytes(X509* x509) { + // No error queue impact. + if (x509) { return X509_get0_pubkey_bitstr(x509); @@ -345,6 +364,8 @@ Any negative value: The input buffer size was reported as insufficient. A buffer */ int32_t CryptoNative_GetAsn1StringBytes(ASN1_STRING* asn1, uint8_t* pBuf, int32_t cBuf) { + // No error queue impact. + if (!asn1 || cBuf < 0) { return 0; @@ -380,6 +401,8 @@ Any negative value: The input buffer size was reported as insufficient. A buffer */ int32_t CryptoNative_GetX509NameRawBytes(X509_NAME* x509Name, uint8_t* pBuf, int32_t cBuf) { + ERR_clear_error(); + const uint8_t* nameBuf; size_t nameBufLen; @@ -433,6 +456,7 @@ Note that 0 does not always indicate an error, merely that GetX509EkuField shoul */ int32_t CryptoNative_GetX509EkuFieldCount(EXTENDED_KEY_USAGE* eku) { + // No error queue impact. return sk_ASN1_OBJECT_num(eku); } @@ -449,6 +473,7 @@ that particular OID. */ ASN1_OBJECT* CryptoNative_GetX509EkuField(EXTENDED_KEY_USAGE* eku, int32_t loc) { + // No error queue impact. return sk_ASN1_OBJECT_value(eku, loc); } @@ -467,6 +492,8 @@ BIO* CryptoNative_GetX509NameInfo(X509* x509, int32_t nameType, int32_t forIssue { static const char szOidUpn[] = "1.3.6.1.4.1.311.20.2.3"; + ERR_clear_error(); + if (!x509 || nameType < NAME_TYPE_SIMPLE || nameType > NAME_TYPE_URL) { return NULL; @@ -729,6 +756,8 @@ int32_t CryptoNative_CheckX509Hostname(X509* x509, const char* hostname, int32_t if (cchHostname < 0) return -5; + ERR_clear_error(); + // OpenSSL will treat a target hostname starting with '.' as special. // We don't expect target hostnames to start with '.', but if one gets in here, the fallback // and the mainline won't be the same... so just make it report false. @@ -771,6 +800,8 @@ int32_t CryptoNative_CheckX509IpAddress( if (!addressBytes) return -6; + ERR_clear_error(); + int subjectNid = NID_commonName; int sanGenType = GEN_IPADD; GENERAL_NAMES* san = (GENERAL_NAMES*)(X509_get_ext_d2i(x509, NID_subject_alt_name, NULL, NULL)); @@ -848,6 +879,7 @@ Note that 0 does not always indicate an error, merely that GetX509StackField sho */ int32_t CryptoNative_GetX509StackFieldCount(STACK_OF(X509) * stack) { + // No error queue impact. return sk_X509_num(stack); } @@ -864,6 +896,7 @@ that particular element. */ X509* CryptoNative_GetX509StackField(STACK_OF(X509) * stack, int loc) { + // No error queue impact. return sk_X509_value(stack, loc); } @@ -876,6 +909,7 @@ when done with it. */ void CryptoNative_RecursiveFreeX509Stack(STACK_OF(X509) * stack) { + // No error queue impact. sk_X509_pop_free(stack, X509_free); } @@ -899,6 +933,8 @@ int32_t CryptoNative_X509StoreSetVerifyTime(X509_STORE* ctx, int32_t second, int32_t isDst) { + ERR_clear_error(); + if (!ctx) { return 0; @@ -935,6 +971,7 @@ otherwise NULL. */ X509* CryptoNative_ReadX509AsDerFromBio(BIO* bio) { + ERR_clear_error(); return d2i_X509_bio(bio, NULL); } @@ -955,6 +992,8 @@ OpenSSL's BIO_tell */ int32_t CryptoNative_BioTell(BIO* bio) { + // No error queue impact. + if (!bio) { return -1; @@ -982,6 +1021,8 @@ OpenSSL's BIO_seek */ int32_t CryptoNative_BioSeek(BIO* bio, int32_t ofs) { + // No error queue impact. + if (!bio) { return -1; @@ -1002,6 +1043,7 @@ A STACK_OF(X509*) with no comparator. */ STACK_OF(X509) * CryptoNative_NewX509Stack() { + ERR_clear_error(); return sk_X509_new_null(); } @@ -1018,6 +1060,8 @@ Return values: */ int32_t CryptoNative_PushX509StackField(STACK_OF(X509) * stack, X509* x509) { + ERR_clear_error(); + if (!stack) { return 0; @@ -1039,6 +1083,7 @@ Returns a bool to managed code. */ int32_t CryptoNative_GetRandomBytes(uint8_t* buf, int32_t num) { + ERR_clear_error(); int ret = RAND_bytes(buf, num); return ret == 1; @@ -1063,6 +1108,8 @@ int32_t CryptoNative_LookupFriendlyNameByOid(const char* oidValue, const char** int nid; const char* ln; + ERR_clear_error(); + if (!oidValue || !friendlyName) { return -2; @@ -1121,6 +1168,7 @@ Version number as MNNFFRBB (major minor fix final beta/patch) */ int64_t CryptoNative_OpenSslVersionNumber() { + // No error queue impact. return (int64_t)OpenSSL_version_num(); } @@ -1130,6 +1178,9 @@ void CryptoNative_RegisterLegacyAlgorithms() if (API_EXISTS(OSSL_PROVIDER_try_load)) { OSSL_PROVIDER_try_load(NULL, "legacy", 1); + + // Doesn't matter if it succeeded or failed. + ERR_clear_error(); } #endif } diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_asn1.c b/src/native/libs/System.Security.Cryptography.Native/pal_asn1.c index 349bff95d6d6f..32661cf31dcbc 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_asn1.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_asn1.c @@ -13,16 +13,22 @@ c_static_assert(PAL_NID_secp521r1 == NID_secp521r1); const ASN1_OBJECT* CryptoNative_ObjTxt2Obj(const char* s) { + ERR_clear_error(); return OBJ_txt2obj(s, true); } int32_t CryptoNative_ObjObj2Txt(char* buf, int32_t buf_len, const ASN1_OBJECT* a) { + ERR_clear_error(); return OBJ_obj2txt(buf, buf_len, a, true); } const ASN1_OBJECT* CryptoNative_GetObjectDefinitionByName(const char* friendlyName) { + ERR_clear_error(); + + // Neither ln2nid nor sn2nid can generate errors, but nid2obj can in certain circumstances. + int nid = OBJ_ln2nid(friendlyName); if (nid == NID_undef) @@ -40,11 +46,13 @@ const ASN1_OBJECT* CryptoNative_GetObjectDefinitionByName(const char* friendlyNa int32_t CryptoNative_ObjTxt2Nid(const char* sn) { + ERR_clear_error(); return OBJ_txt2nid(sn); } const ASN1_OBJECT* CryptoNative_ObjNid2Obj(int32_t nid) { + ERR_clear_error(); return OBJ_nid2obj(nid); } @@ -60,6 +68,8 @@ ASN1_BIT_STRING* CryptoNative_DecodeAsn1BitString(const uint8_t* buf, int32_t le return NULL; } + ERR_clear_error(); + return d2i_ASN1_BIT_STRING(NULL, &buf, len); } @@ -70,11 +80,13 @@ void CryptoNative_Asn1BitStringFree(ASN1_STRING* a) ASN1_OCTET_STRING* CryptoNative_Asn1OctetStringNew() { + ERR_clear_error(); return ASN1_OCTET_STRING_new(); } int32_t CryptoNative_Asn1OctetStringSet(ASN1_OCTET_STRING* s, const uint8_t* data, int32_t len) { + ERR_clear_error(); return ASN1_OCTET_STRING_set(s, data, len); } @@ -85,10 +97,12 @@ void CryptoNative_Asn1OctetStringFree(ASN1_STRING* a) int32_t CryptoNative_GetAsn1IntegerDerSize(ASN1_INTEGER* i) { + ERR_clear_error(); return i2d_ASN1_INTEGER(i, NULL); } int32_t CryptoNative_EncodeAsn1Integer(ASN1_INTEGER* i, uint8_t* buf) { + ERR_clear_error(); return i2d_ASN1_INTEGER(i, &buf); } diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_bignum.c b/src/native/libs/System.Security.Cryptography.Native/pal_bignum.c index c1c61d01375bb..c347e3f2e8fea 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_bignum.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_bignum.c @@ -18,6 +18,8 @@ BIGNUM* CryptoNative_BigNumFromBinary(const uint8_t* s, int32_t len) return NULL; } + ERR_clear_error(); + return BN_bin2bn(s, len, NULL); } @@ -28,6 +30,8 @@ int32_t CryptoNative_BigNumToBinary(const BIGNUM* a, uint8_t* to) return 0; } + ERR_clear_error(); + return BN_bn2bin(a, to); } @@ -38,5 +42,6 @@ int32_t CryptoNative_GetBigNumBytes(const BIGNUM* a) return 0; } + // No impact on the error queue. return BN_num_bytes(a); } diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_bio.c b/src/native/libs/System.Security.Cryptography.Native/pal_bio.c index a074f2b310f07..0c349664b23dc 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_bio.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_bio.c @@ -7,11 +7,13 @@ BIO* CryptoNative_CreateMemoryBio() { + ERR_clear_error(); return BIO_new(BIO_s_mem()); } BIO* CryptoNative_BioNewFile(const char* filename, const char* mode) { + ERR_clear_error(); return BIO_new_file(filename, mode); } @@ -22,21 +24,25 @@ int32_t CryptoNative_BioDestroy(BIO* a) int32_t CryptoNative_BioGets(BIO* b, char* buf, int32_t size) { + ERR_clear_error(); return BIO_gets(b, buf, size); } int32_t CryptoNative_BioRead(BIO* b, void* buf, int32_t len) { + ERR_clear_error(); return BIO_read(b, buf, len); } int32_t CryptoNative_BioWrite(BIO* b, const void* buf, int32_t len) { + ERR_clear_error(); return BIO_write(b, buf, len); } int32_t CryptoNative_GetMemoryBioSize(BIO* bio) { + // No impact on error queue. long ret = BIO_get_mem_data(bio, NULL); // BIO_get_mem_data returns the memory size, which will always be @@ -47,6 +53,7 @@ int32_t CryptoNative_GetMemoryBioSize(BIO* bio) int32_t CryptoNative_BioCtrlPending(BIO* bio) { + // No impact on the error queue. size_t result = BIO_ctrl_pending(bio); assert(result <= INT32_MAX); return (int32_t)result; diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_dsa.c b/src/native/libs/System.Security.Cryptography.Native/pal_dsa.c index c8b6892654ea2..610272c046b11 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_dsa.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_dsa.c @@ -6,6 +6,7 @@ int32_t CryptoNative_DsaUpRef(DSA* dsa) { + // no impact on the error queue. return DSA_up_ref(dsa); } @@ -25,6 +26,8 @@ int32_t CryptoNative_DsaGenerateKey(DSA** dsa, int32_t bits) return 0; } + ERR_clear_error(); + *dsa = DSA_new(); if (!(*dsa)) { @@ -44,11 +47,13 @@ int32_t CryptoNative_DsaGenerateKey(DSA** dsa, int32_t bits) int32_t CryptoNative_DsaSizeSignature(DSA* dsa) { + // No error queue impact. return DSA_size(dsa); } int32_t CryptoNative_DsaSizeP(DSA* dsa) { + // No error queue impact. if (dsa) { const BIGNUM* p; @@ -65,6 +70,7 @@ int32_t CryptoNative_DsaSizeP(DSA* dsa) int32_t CryptoNative_DsaSizeQ(DSA* dsa) { + // No error queue impact. if (dsa) { const BIGNUM* q; @@ -92,6 +98,8 @@ int32_t CryptoNative_DsaSign( return 0; } + ERR_clear_error(); + // DSA_OpenSSL() returns a shared pointer, no need to free/cache. if (DSA_get_method(dsa) == DSA_OpenSSL()) { @@ -161,6 +169,7 @@ int32_t CryptoNative_GetDsaParameters( assert(yLength != NULL); assert(xLength != NULL); + // No error queue impact. DSA_get0_pqg(dsa, p, q, g); *pLength = BN_num_bytes(*p); *qLength = BN_num_bytes(*q); @@ -203,6 +212,8 @@ int32_t CryptoNative_DsaKeyCreateByExplicitParameters( return 0; } + ERR_clear_error(); + *outDsa = DSA_new(); if (!*outDsa) { diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ecc_import_export.c b/src/native/libs/System.Security.Cryptography.Native/pal_ecc_import_export.c index 8d2070dfe3aa0..fedac751ab601 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ecc_import_export.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ecc_import_export.c @@ -39,6 +39,7 @@ static const EC_METHOD* CurveTypeToMethod(ECCurveType curveType) static ECCurveType EcKeyGetCurveType( const EC_KEY* key) { + // Simple accessors, no error queue impact. const EC_GROUP* group = EC_KEY_get0_group(key); if (!group) return Unspecified; @@ -67,6 +68,8 @@ int32_t CryptoNative_GetECKeyParameters( BIGNUM *xBn = NULL; BIGNUM *yBn = NULL; + ERR_clear_error(); + ECCurveType curveType = EcKeyGetCurveType(key); const EC_POINT* Q = EC_KEY_get0_public_key(key); const EC_GROUP* group = EC_KEY_get0_group(key); @@ -165,6 +168,8 @@ int32_t CryptoNative_GetECCurveParameters( assert(seed != NULL); assert(cbSeed != NULL); + ERR_clear_error(); + // Get the public key parameters first in case any of its 'out' parameters are not initialized int32_t rc = CryptoNative_GetECKeyParameters(key, includePrivate, qx, cbQx, qy, cbQy, d, cbD); @@ -315,6 +320,8 @@ int32_t CryptoNative_EcKeyCreateByKeyParameters(EC_KEY** key, const char* oid, u *key = NULL; + ERR_clear_error(); + // oid can be friendly name or value int nid = OBJ_txt2nid(oid); if (!nid) @@ -426,6 +433,8 @@ EC_KEY* CryptoNative_EcKeyCreateByExplicitParameters( return 0; } + ERR_clear_error(); + EC_KEY* key = NULL; EC_POINT* G = NULL; EC_POINT* pubG = NULL; diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ecdsa.c b/src/native/libs/System.Security.Cryptography.Native/pal_ecdsa.c index c21ef80499c97..43792d35b6ea5 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ecdsa.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ecdsa.c @@ -7,6 +7,8 @@ int32_t CryptoNative_EcDsaSign(const uint8_t* dgst, int32_t dgstlen, uint8_t* sig, int32_t* siglen, EC_KEY* key) { + ERR_clear_error(); + if (!siglen) { return 0; @@ -21,10 +23,12 @@ CryptoNative_EcDsaSign(const uint8_t* dgst, int32_t dgstlen, uint8_t* sig, int32 int32_t CryptoNative_EcDsaVerify(const uint8_t* dgst, int32_t dgstlen, const uint8_t* sig, int32_t siglen, EC_KEY* key) { + ERR_clear_error(); return ECDSA_verify(0, dgst, dgstlen, sig, siglen, key); } int32_t CryptoNative_EcDsaSize(const EC_KEY* key) { + // No error queue impact. return ECDSA_size(key); } diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_eckey.c b/src/native/libs/System.Security.Cryptography.Native/pal_eckey.c index 990fbf3fa978a..5c51e98c3f063 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_eckey.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_eckey.c @@ -12,6 +12,8 @@ void CryptoNative_EcKeyDestroy(EC_KEY* r) EC_KEY* CryptoNative_EcKeyCreateByOid(const char* oid) { + ERR_clear_error(); + // oid can be friendly name or value int nid = OBJ_txt2nid(oid); return EC_KEY_new_by_curve_name(nid); @@ -19,19 +21,26 @@ EC_KEY* CryptoNative_EcKeyCreateByOid(const char* oid) int32_t CryptoNative_EcKeyGenerateKey(EC_KEY* eckey) { + ERR_clear_error(); + if (!EC_KEY_generate_key(eckey)) + { return 0; + } return EC_KEY_check_key(eckey); } int32_t CryptoNative_EcKeyUpRef(EC_KEY* r) { + // No error queue impact return EC_KEY_up_ref(r); } int32_t CryptoNative_EcKeyGetSize(const EC_KEY* key, int32_t* keySize) { + // No error queue impact + if (!keySize) return 0; @@ -51,6 +60,8 @@ int32_t CryptoNative_EcKeyGetSize(const EC_KEY* key, int32_t* keySize) int32_t CryptoNative_EcKeyGetCurveName2(const EC_KEY* key, int32_t* nidName) { + // No error queue impact. + if (!nidName) return 0; diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_err.c b/src/native/libs/System.Security.Cryptography.Native/pal_err.c index e1ca3292fbab2..da0dc6fd2a8be 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_err.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_err.c @@ -9,15 +9,17 @@ void CryptoNative_ErrClearError() ERR_clear_error(); } -uint64_t CryptoNative_ErrGetErrorAlloc(int32_t* isAllocFailure) +uint64_t CryptoNative_ErrGetExceptionError(int32_t* isAllocFailure) { - unsigned long err = ERR_get_error(); + unsigned long err = ERR_peek_last_error(); if (isAllocFailure) { *isAllocFailure = ERR_GET_REASON(err) == ERR_R_MALLOC_FAILURE; } + // We took the one we want, clear the rest. + ERR_clear_error(); return err; } diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_err.h b/src/native/libs/System.Security.Cryptography.Native/pal_err.h index 810cd51c68ffc..77b8c378f6f34 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_err.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_err.h @@ -18,10 +18,11 @@ Shims the ERR_clear_error method. PALEXPORT void CryptoNative_ErrClearError(void); /* -Shim to ERR_get_error which also returns whether the error -was caused by an allocation failure. +Returns the error code to use as the basis for an exception. + +If the error represents an allocation error, *isAllocFailure is set to 1. */ -PALEXPORT uint64_t CryptoNative_ErrGetErrorAlloc(int32_t* isAllocFailure); +PALEXPORT uint64_t CryptoNative_ErrGetExceptionError(int32_t* isAllocFailure); PALEXPORT uint64_t CryptoNative_ErrPeekError(void); diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_evp.c b/src/native/libs/System.Security.Cryptography.Native/pal_evp.c index 40181b2b132b6..fcdd1aef74f55 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_evp.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_evp.c @@ -9,10 +9,16 @@ EVP_MD_CTX* CryptoNative_EvpMdCtxCreate(const EVP_MD* type) { + ERR_clear_error(); + EVP_MD_CTX* ctx = EVP_MD_CTX_new(); + if (ctx == NULL) { - // Allocation failed + // Allocation failed. + // This is one of the few places that don't report the error to the queue, so + // we'll do it here. + ERR_put_error(ERR_LIB_EVP, 0, ERR_R_MALLOC_FAILURE, __FILE__, __LINE__); return NULL; } @@ -36,16 +42,20 @@ void CryptoNative_EvpMdCtxDestroy(EVP_MD_CTX* ctx) int32_t CryptoNative_EvpDigestReset(EVP_MD_CTX* ctx, const EVP_MD* type) { + ERR_clear_error(); return EVP_DigestInit_ex(ctx, type, NULL); } int32_t CryptoNative_EvpDigestUpdate(EVP_MD_CTX* ctx, const void* d, int32_t cnt) { + // No error queue impact return EVP_DigestUpdate(ctx, d, (size_t)cnt); } int32_t CryptoNative_EvpDigestFinalEx(EVP_MD_CTX* ctx, uint8_t* md, uint32_t* s) { + ERR_clear_error(); + unsigned int size; int32_t ret = EVP_DigestFinal_ex(ctx, md, &size); if (ret == SUCCESS) @@ -67,6 +77,10 @@ static EVP_MD_CTX* EvpDup(const EVP_MD_CTX* ctx) if (dup == NULL) { + // Allocation failed. + // This is one of the few places that don't report the error to the queue, so + // we'll do it here. + ERR_put_error(ERR_LIB_EVP, 0, ERR_R_MALLOC_FAILURE, __FILE__, __LINE__); return NULL; } @@ -81,6 +95,8 @@ static EVP_MD_CTX* EvpDup(const EVP_MD_CTX* ctx) int32_t CryptoNative_EvpDigestCurrent(const EVP_MD_CTX* ctx, uint8_t* md, uint32_t* s) { + ERR_clear_error(); + EVP_MD_CTX* dup = EvpDup(ctx); if (dup != NULL) @@ -95,13 +111,19 @@ int32_t CryptoNative_EvpDigestCurrent(const EVP_MD_CTX* ctx, uint8_t* md, uint32 int32_t CryptoNative_EvpDigestOneShot(const EVP_MD* type, const void* source, int32_t sourceSize, uint8_t* md, uint32_t* mdSize) { + ERR_clear_error(); + if (type == NULL || sourceSize < 0 || md == NULL || mdSize == NULL) + { return 0; + } EVP_MD_CTX* ctx = CryptoNative_EvpMdCtxCreate(type); if (ctx == NULL) + { return 0; + } int32_t ret = EVP_DigestUpdate(ctx, source, (size_t)sourceSize); @@ -119,36 +141,43 @@ int32_t CryptoNative_EvpDigestOneShot(const EVP_MD* type, const void* source, in int32_t CryptoNative_EvpMdSize(const EVP_MD* md) { + // No error queue impact. return EVP_MD_get_size(md); } const EVP_MD* CryptoNative_EvpMd5() { + // No error queue impact. return EVP_md5(); } const EVP_MD* CryptoNative_EvpSha1() { + // No error queue impact. return EVP_sha1(); } const EVP_MD* CryptoNative_EvpSha256() { + // No error queue impact. return EVP_sha256(); } const EVP_MD* CryptoNative_EvpSha384() { + // No error queue impact. return EVP_sha384(); } const EVP_MD* CryptoNative_EvpSha512() { + // No error queue impact. return EVP_sha512(); } int32_t CryptoNative_GetMaxMdSize() { + // No error queue impact. return EVP_MAX_MD_SIZE; } @@ -167,6 +196,8 @@ int32_t CryptoNative_Pbkdf2(const char* password, return -1; } + ERR_clear_error(); + const char* empty = ""; if (salt == NULL) diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_evp_cipher.c b/src/native/libs/System.Security.Cryptography.Native/pal_evp_cipher.c index ae2c0df5aae02..672490c5ef495 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_evp_cipher.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_evp_cipher.c @@ -11,10 +11,16 @@ EVP_CIPHER_CTX* CryptoNative_EvpCipherCreate2(const EVP_CIPHER* type, uint8_t* key, int32_t keyLength, unsigned char* iv, int32_t enc) { + ERR_clear_error(); + EVP_CIPHER_CTX* ctx = EVP_CIPHER_CTX_new(); + if (ctx == NULL) { // Allocation failed + // This is one of the few places that don't report the error to the queue, so + // we'll do it here. + ERR_put_error(ERR_LIB_EVP, 0, ERR_R_MALLOC_FAILURE, __FILE__, __LINE__); return NULL; } @@ -75,10 +81,16 @@ CryptoNative_EvpCipherCreate2(const EVP_CIPHER* type, uint8_t* key, int32_t keyL EVP_CIPHER_CTX* CryptoNative_EvpCipherCreatePartial(const EVP_CIPHER* type) { + ERR_clear_error(); + EVP_CIPHER_CTX* ctx = EVP_CIPHER_CTX_new(); + if (ctx == NULL) { // Allocation failed + // This is one of the few places that don't report the error to the queue, so + // we'll do it here. + ERR_put_error(ERR_LIB_EVP, 0, ERR_R_MALLOC_FAILURE, __FILE__, __LINE__); return NULL; } @@ -101,17 +113,21 @@ CryptoNative_EvpCipherCreatePartial(const EVP_CIPHER* type) int32_t CryptoNative_EvpCipherSetKeyAndIV(EVP_CIPHER_CTX* ctx, uint8_t* key, unsigned char* iv, int32_t enc) { + ERR_clear_error(); + // Perform final initialization specifying the remaining arguments return EVP_CipherInit_ex(ctx, NULL, NULL, key, iv, enc); } int32_t CryptoNative_EvpCipherSetGcmNonceLength(EVP_CIPHER_CTX* ctx, int32_t ivLength) { + ERR_clear_error(); return EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_IVLEN, ivLength, NULL); } int32_t CryptoNative_EvpCipherSetCcmNonceLength(EVP_CIPHER_CTX* ctx, int32_t ivLength) { + ERR_clear_error(); return EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_CCM_SET_IVLEN, ivLength, NULL); } @@ -127,18 +143,22 @@ int32_t CryptoNative_EvpCipherReset(EVP_CIPHER_CTX* ctx, uint8_t* pIv, int32_t c { assert(cIv >= 0 && (pIv != NULL || cIv == 0)); (void)cIv; + ERR_clear_error(); return EVP_CipherInit_ex(ctx, NULL, NULL, NULL, pIv, KEEP_CURRENT_DIRECTION); } int32_t CryptoNative_EvpCipherCtxSetPadding(EVP_CIPHER_CTX* x, int32_t padding) { + // No error queue impact. return EVP_CIPHER_CTX_set_padding(x, padding); } int32_t CryptoNative_EvpCipherUpdate(EVP_CIPHER_CTX* ctx, uint8_t* out, int32_t* outl, unsigned char* in, int32_t inl) { + ERR_clear_error(); + int outLength; int32_t ret = EVP_CipherUpdate(ctx, out, &outLength, in, inl); if (ret == SUCCESS) @@ -151,6 +171,8 @@ CryptoNative_EvpCipherUpdate(EVP_CIPHER_CTX* ctx, uint8_t* out, int32_t* outl, u int32_t CryptoNative_EvpCipherFinalEx(EVP_CIPHER_CTX* ctx, uint8_t* outm, int32_t* outl) { + ERR_clear_error(); + int outLength; int32_t ret = EVP_CipherFinal_ex(ctx, outm, &outLength); if (ret == SUCCESS) @@ -163,26 +185,31 @@ int32_t CryptoNative_EvpCipherFinalEx(EVP_CIPHER_CTX* ctx, uint8_t* outm, int32_ int32_t CryptoNative_EvpCipherGetGcmTag(EVP_CIPHER_CTX* ctx, uint8_t* tag, int32_t tagLength) { + ERR_clear_error(); return EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, tagLength, tag); } int32_t CryptoNative_EvpCipherSetGcmTag(EVP_CIPHER_CTX* ctx, uint8_t* tag, int32_t tagLength) { + ERR_clear_error(); return EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, tagLength, tag); } int32_t CryptoNative_EvpCipherGetCcmTag(EVP_CIPHER_CTX* ctx, uint8_t* tag, int32_t tagLength) { + ERR_clear_error(); return EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_CCM_GET_TAG, tagLength, tag); } int32_t CryptoNative_EvpCipherSetCcmTag(EVP_CIPHER_CTX* ctx, uint8_t* tag, int32_t tagLength) { + ERR_clear_error(); return EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_CCM_SET_TAG, tagLength, tag); } int32_t CryptoNative_EvpCipherGetAeadTag(EVP_CIPHER_CTX* ctx, uint8_t* tag, int32_t tagLength) { + ERR_clear_error(); #if HAVE_OPENSSL_CHACHA20POLY1305 return EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_GET_TAG, tagLength, tag); #else @@ -195,6 +222,7 @@ int32_t CryptoNative_EvpCipherGetAeadTag(EVP_CIPHER_CTX* ctx, uint8_t* tag, int3 int32_t CryptoNative_EvpCipherSetAeadTag(EVP_CIPHER_CTX* ctx, uint8_t* tag, int32_t tagLength) { + ERR_clear_error(); #if HAVE_OPENSSL_CHACHA20POLY1305 return EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_SET_TAG, tagLength, tag); #else @@ -207,141 +235,169 @@ int32_t CryptoNative_EvpCipherSetAeadTag(EVP_CIPHER_CTX* ctx, uint8_t* tag, int3 const EVP_CIPHER* CryptoNative_EvpAes128Ecb() { + // No error queue impact. return EVP_aes_128_ecb(); } const EVP_CIPHER* CryptoNative_EvpAes128Cbc() { + // No error queue impact. return EVP_aes_128_cbc(); } const EVP_CIPHER* CryptoNative_EvpAes128Gcm() { + // No error queue impact. return EVP_aes_128_gcm(); } const EVP_CIPHER* CryptoNative_EvpAes128Cfb128() { + // No error queue impact. return EVP_aes_128_cfb128(); } const EVP_CIPHER* CryptoNative_EvpAes128Cfb8() { + // No error queue impact. return EVP_aes_128_cfb8(); } const EVP_CIPHER* CryptoNative_EvpAes128Ccm() { + // No error queue impact. return EVP_aes_128_ccm(); } const EVP_CIPHER* CryptoNative_EvpAes192Ecb() { + // No error queue impact. return EVP_aes_192_ecb(); } const EVP_CIPHER* CryptoNative_EvpAes192Cfb128() { + // No error queue impact. return EVP_aes_192_cfb128(); } const EVP_CIPHER* CryptoNative_EvpAes192Cfb8() { + // No error queue impact. return EVP_aes_192_cfb8(); } const EVP_CIPHER* CryptoNative_EvpAes192Cbc() { + // No error queue impact. return EVP_aes_192_cbc(); } const EVP_CIPHER* CryptoNative_EvpAes192Gcm() { + // No error queue impact. return EVP_aes_192_gcm(); } const EVP_CIPHER* CryptoNative_EvpAes192Ccm() { + // No error queue impact. return EVP_aes_192_ccm(); } const EVP_CIPHER* CryptoNative_EvpAes256Ecb() { + // No error queue impact. return EVP_aes_256_ecb(); } const EVP_CIPHER* CryptoNative_EvpAes256Cfb128() { + // No error queue impact. return EVP_aes_256_cfb128(); } const EVP_CIPHER* CryptoNative_EvpAes256Cfb8() { + // No error queue impact. return EVP_aes_256_cfb8(); } const EVP_CIPHER* CryptoNative_EvpAes256Cbc() { + // No error queue impact. return EVP_aes_256_cbc(); } const EVP_CIPHER* CryptoNative_EvpAes256Gcm() { + // No error queue impact. return EVP_aes_256_gcm(); } const EVP_CIPHER* CryptoNative_EvpAes256Ccm() { + // No error queue impact. return EVP_aes_256_ccm(); } const EVP_CIPHER* CryptoNative_EvpDesEcb() { + // No error queue impact. return EVP_des_ecb(); } const EVP_CIPHER* CryptoNative_EvpDesCfb8() { + // No error queue impact. return EVP_des_cfb8(); } const EVP_CIPHER* CryptoNative_EvpDesCbc() { + // No error queue impact. return EVP_des_cbc(); } const EVP_CIPHER* CryptoNative_EvpDes3Ecb() { + // No error queue impact. return EVP_des_ede3(); } const EVP_CIPHER* CryptoNative_EvpDes3Cfb8() { + // No error queue impact. return EVP_des_ede3_cfb8(); } const EVP_CIPHER* CryptoNative_EvpDes3Cfb64() { + // No error queue impact. return EVP_des_ede3_cfb64(); } const EVP_CIPHER* CryptoNative_EvpDes3Cbc() { + // No error queue impact. return EVP_des_ede3_cbc(); } const EVP_CIPHER* CryptoNative_EvpRC2Ecb() { + // No error queue impact. return EVP_rc2_ecb(); } const EVP_CIPHER* CryptoNative_EvpRC2Cbc() { + // No error queue impact. return EVP_rc2_cbc(); } const EVP_CIPHER* CryptoNative_EvpChaCha20Poly1305() { + // No error queue impact. #if HAVE_OPENSSL_CHACHA20POLY1305 if (API_EXISTS(EVP_chacha20_poly1305)) { diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.c b/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.c index 79de386f09721..381711b5c7a07 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey.c @@ -6,6 +6,7 @@ EVP_PKEY* CryptoNative_EvpPkeyCreate() { + ERR_clear_error(); return EVP_PKEY_new(); } @@ -13,6 +14,8 @@ EVP_PKEY* CryptoNative_EvpPKeyDuplicate(EVP_PKEY* currentKey, int32_t algId) { assert(currentKey != NULL); + ERR_clear_error(); + int currentAlgId = EVP_PKEY_get_base_id(currentKey); if (algId != NID_undef && algId != currentAlgId) @@ -67,6 +70,10 @@ void CryptoNative_EvpPkeyDestroy(EVP_PKEY* pkey) int32_t CryptoNative_EvpPKeySize(EVP_PKEY* pkey) { + // This function is not expected to populate the error queue with + // any errors, but it's technically possible that an external + // ENGINE or OSSL_PROVIDER populate the queue in their implmenetation, + // but the calling code does not check for one. assert(pkey != NULL); return EVP_PKEY_get_size(pkey); } @@ -78,6 +85,7 @@ int32_t CryptoNative_UpRefEvpPkey(EVP_PKEY* pkey) return 0; } + // No error queue impact. return EVP_PKEY_up_ref(pkey); } @@ -117,6 +125,8 @@ EVP_PKEY* CryptoNative_DecodeSubjectPublicKeyInfo(const uint8_t* buf, int32_t le assert(buf != NULL); assert(len > 0); + ERR_clear_error(); + EVP_PKEY* key = d2i_PUBKEY(NULL, &buf, len); if (key != NULL && !CheckKey(key, algId, EVP_PKEY_public_check)) @@ -133,6 +143,8 @@ EVP_PKEY* CryptoNative_DecodePkcs8PrivateKey(const uint8_t* buf, int32_t len, in assert(buf != NULL); assert(len > 0); + ERR_clear_error(); + PKCS8_PRIV_KEY_INFO* p8info = d2i_PKCS8_PRIV_KEY_INFO(NULL, &buf, len); if (p8info == NULL) @@ -198,6 +210,7 @@ int32_t CryptoNative_GetPkcs8PrivateKeySize(EVP_PKEY* pkey, int32_t* p8size) ERR_clear_error(); ERR_put_error(ERR_GET_LIB(error), 0, ERR_R_MALLOC_FAILURE, file, line); + // Since ERR_peek_error() matches what exception is thrown, leave the OOM on top. return -1; } @@ -212,6 +225,8 @@ int32_t CryptoNative_EncodePkcs8PrivateKey(EVP_PKEY* pkey, uint8_t* buf) assert(pkey != NULL); assert(buf != NULL); + ERR_clear_error(); + PKCS8_PRIV_KEY_INFO* p8 = EVP_PKEY2PKCS8(pkey); if (p8 == NULL) @@ -228,6 +243,7 @@ int32_t CryptoNative_GetSubjectPublicKeyInfoSize(EVP_PKEY* pkey) { assert(pkey != NULL); + ERR_clear_error(); return i2d_PUBKEY(pkey, NULL); } @@ -236,5 +252,6 @@ int32_t CryptoNative_EncodeSubjectPublicKeyInfo(EVP_PKEY* pkey, uint8_t* buf) assert(pkey != NULL); assert(buf != NULL); + ERR_clear_error(); return i2d_PUBKEY(pkey, &buf); } diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_dsa.c b/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_dsa.c index 1b95369e84264..2f649db62a58c 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_dsa.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_dsa.c @@ -5,10 +5,12 @@ DSA* CryptoNative_EvpPkeyGetDsa(EVP_PKEY* pkey) { + ERR_clear_error(); return EVP_PKEY_get1_DSA(pkey); } int32_t CryptoNative_EvpPkeySetDsa(EVP_PKEY* pkey, DSA* dsa) { + ERR_clear_error(); return EVP_PKEY_set1_DSA(pkey, dsa); } diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_ecdh.c b/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_ecdh.c index 6dbe655e6c79a..b6a9daf715998 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_ecdh.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_ecdh.c @@ -8,6 +8,8 @@ EVP_PKEY_CTX* CryptoNative_EvpPKeyCtxCreate(EVP_PKEY* pkey, EVP_PKEY* peerkey, u if (secretLength != NULL) *secretLength = 0; + ERR_clear_error(); + if (pkey == NULL || peerkey == NULL || secretLength == NULL) { return NULL; @@ -40,6 +42,8 @@ int32_t CryptoNative_EvpPKeyDeriveSecretAgreement(uint8_t* secret, uint32_t secr size_t tmpSize = (size_t)secretLength; int ret = 0; + ERR_clear_error(); + if (secret != NULL && ctx != NULL) { ret = EVP_PKEY_derive(ctx, secret, &tmpSize); diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_eckey.c b/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_eckey.c index 65d7cfb8d3f0c..e7c4389b05308 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_eckey.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_eckey.c @@ -5,10 +5,12 @@ EC_KEY* CryptoNative_EvpPkeyGetEcKey(EVP_PKEY* pkey) { + ERR_clear_error(); return EVP_PKEY_get1_EC_KEY(pkey); } int32_t CryptoNative_EvpPkeySetEcKey(EVP_PKEY* pkey, EC_KEY* key) { + ERR_clear_error(); return EVP_PKEY_set1_EC_KEY(pkey, key); } diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_rsa.c b/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_rsa.c index 36924abb50581..51e0b055f1ea6 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_rsa.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_rsa.c @@ -11,6 +11,8 @@ EVP_PKEY* CryptoNative_EvpPKeyCreateRsa(RSA* currentKey) { assert(currentKey != NULL); + ERR_clear_error(); + EVP_PKEY* pkey = EVP_PKEY_new(); if (pkey == NULL) @@ -29,6 +31,8 @@ EVP_PKEY* CryptoNative_EvpPKeyCreateRsa(RSA* currentKey) EVP_PKEY* CryptoNative_RsaGenerateKey(int keySize) { + ERR_clear_error(); + EVP_PKEY_CTX* ctx = EVP_PKEY_CTX_new_id(EVP_PKEY_RSA, NULL); if (ctx == NULL) @@ -99,6 +103,8 @@ int32_t CryptoNative_RsaDecrypt(EVP_PKEY* pkey, assert(padding >= RsaPaddingPkcs1 && padding <= RsaPaddingOaepOrPss); assert(digest != NULL || padding == RsaPaddingPkcs1); + ERR_clear_error(); + EVP_PKEY_CTX* ctx = EVP_PKEY_CTX_new(pkey, NULL); int ret = -1; @@ -153,6 +159,8 @@ int32_t CryptoNative_RsaEncrypt(EVP_PKEY* pkey, assert(padding >= RsaPaddingPkcs1 && padding <= RsaPaddingOaepOrPss); assert(digest != NULL || padding == RsaPaddingPkcs1); + ERR_clear_error(); + EVP_PKEY_CTX* ctx = EVP_PKEY_CTX_new(pkey, NULL); int ret = -1; @@ -227,6 +235,8 @@ int32_t CryptoNative_RsaSignHash(EVP_PKEY* pkey, assert(padding >= RsaPaddingPkcs1 && padding <= RsaPaddingOaepOrPss); assert(digest != NULL || padding == RsaPaddingPkcs1); + ERR_clear_error(); + EVP_PKEY_CTX* ctx = EVP_PKEY_CTX_new(pkey, NULL); int ret = -1; @@ -281,6 +291,8 @@ int32_t CryptoNative_RsaVerifyHash(EVP_PKEY* pkey, assert(padding >= RsaPaddingPkcs1 && padding <= RsaPaddingOaepOrPss); assert(digest != NULL || padding == RsaPaddingPkcs1); + ERR_clear_error(); + EVP_PKEY_CTX* ctx = EVP_PKEY_CTX_new(pkey, NULL); int ret = -1; diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_hmac.c b/src/native/libs/System.Security.Cryptography.Native/pal_hmac.c index 3b6f8166dac6c..ac35430692ab8 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_hmac.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_hmac.c @@ -13,10 +13,16 @@ HMAC_CTX* CryptoNative_HmacCreate(const uint8_t* key, int32_t keyLen, const EVP_ assert(keyLen >= 0); assert(md != NULL); + ERR_clear_error(); + HMAC_CTX* ctx = HMAC_CTX_new(); + if (ctx == NULL) { // Allocation failed + // This is one of the few places that don't report the error to the queue, so + // we'll do it here. + ERR_put_error(ERR_LIB_EVP, 0, ERR_R_MALLOC_FAILURE, __FILE__, __LINE__); return NULL; } @@ -49,6 +55,8 @@ int32_t CryptoNative_HmacReset(HMAC_CTX* ctx) { assert(ctx != NULL); + ERR_clear_error(); + return HMAC_Init_ex(ctx, NULL, 0, NULL, NULL); } @@ -58,6 +66,8 @@ int32_t CryptoNative_HmacUpdate(HMAC_CTX* ctx, const uint8_t* data, int32_t len) assert(data != NULL || len == 0); assert(len >= 0); + ERR_clear_error(); + if (len < 0) { return 0; @@ -73,6 +83,8 @@ int32_t CryptoNative_HmacFinal(HMAC_CTX* ctx, uint8_t* md, int32_t* len) assert(md != NULL || *len == 0); assert(*len >= 0); + ERR_clear_error(); + if (len == NULL || *len < 0) { return 0; @@ -88,10 +100,15 @@ static HMAC_CTX* HmacDup(const HMAC_CTX* ctx) { assert(ctx != NULL); + ERR_clear_error(); + HMAC_CTX* dup = HMAC_CTX_new(); if (dup == NULL) { + // This is one of the few places that don't report the error to the queue, so + // we'll do it here. + ERR_put_error(ERR_LIB_EVP, 0, ERR_R_MALLOC_FAILURE, __FILE__, __LINE__); return NULL; } @@ -114,6 +131,8 @@ int32_t CryptoNative_HmacCurrent(const HMAC_CTX* ctx, uint8_t* md, int32_t* len) assert(md != NULL || *len == 0); assert(*len >= 0); + ERR_clear_error(); + if (len == NULL || *len < 0) { return 0; @@ -144,6 +163,8 @@ int32_t CryptoNative_HmacOneShot(const EVP_MD* type, assert(key != NULL || keySize == 0); assert(source != NULL || sourceSize == 0); + ERR_clear_error(); + uint8_t empty = 0; if (key == NULL) diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ocsp.c b/src/native/libs/System.Security.Cryptography.Native/pal_ocsp.c index dc4137ddddb2e..84e1a5b72b196 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ocsp.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ocsp.c @@ -15,16 +15,20 @@ void CryptoNative_OcspRequestDestroy(OCSP_REQUEST* request) int32_t CryptoNative_GetOcspRequestDerSize(OCSP_REQUEST* req) { + ERR_clear_error(); return i2d_OCSP_REQUEST(req, NULL); } int32_t CryptoNative_EncodeOcspRequest(OCSP_REQUEST* req, uint8_t* buf) { + ERR_clear_error(); return i2d_OCSP_REQUEST(req, &buf); } OCSP_RESPONSE* CryptoNative_DecodeOcspResponse(const uint8_t* buf, int32_t len) { + ERR_clear_error(); + if (buf == NULL || len == 0) { return NULL; diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_pkcs7.c b/src/native/libs/System.Security.Cryptography.Native/pal_pkcs7.c index ae1b33d8b4782..efb0a738966f5 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_pkcs7.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_pkcs7.c @@ -5,11 +5,14 @@ PKCS7* CryptoNative_PemReadBioPkcs7(BIO* bp) { + ERR_clear_error(); return PEM_read_bio_PKCS7(bp, NULL, NULL, NULL); } PKCS7* CryptoNative_DecodePkcs7(const uint8_t* buf, int32_t len) { + ERR_clear_error(); + if (!buf || !len) { return NULL; @@ -20,11 +23,13 @@ PKCS7* CryptoNative_DecodePkcs7(const uint8_t* buf, int32_t len) PKCS7* CryptoNative_D2IPkcs7Bio(BIO* bp) { + ERR_clear_error(); return d2i_PKCS7_bio(bp, NULL); } PKCS7* CryptoNative_Pkcs7CreateCertificateCollection(X509Stack* certs) { + ERR_clear_error(); return PKCS7_sign(NULL, NULL, certs, NULL, PKCS7_PARTIAL); } @@ -38,6 +43,8 @@ void CryptoNative_Pkcs7Destroy(PKCS7* p7) int32_t CryptoNative_GetPkcs7Certificates(PKCS7* p7, X509Stack** certs) { + // No error queue impact. + if (!p7 || !certs) { return 0; @@ -58,10 +65,12 @@ int32_t CryptoNative_GetPkcs7Certificates(PKCS7* p7, X509Stack** certs) int32_t CryptoNative_GetPkcs7DerSize(PKCS7* p7) { + ERR_clear_error(); return i2d_PKCS7(p7, NULL); } int32_t CryptoNative_EncodePkcs7(PKCS7* p7, uint8_t* buf) { + ERR_clear_error(); return i2d_PKCS7(p7, &buf); } diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 5b5e2028c0203..5ef0e681182d3 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -144,6 +144,7 @@ void CryptoNative_EnsureLibSslInitialized() const SSL_METHOD* CryptoNative_SslV2_3Method() { + // No error queue impact. const SSL_METHOD* method = TLS_method(); assert(method != NULL); return method; @@ -151,6 +152,8 @@ const SSL_METHOD* CryptoNative_SslV2_3Method() SSL_CTX* CryptoNative_SslCtxCreate(const SSL_METHOD* method) { + ERR_clear_error(); + SSL_CTX* ctx = SSL_CTX_new(method); if (ctx != NULL) @@ -246,6 +249,8 @@ static void ResetCtxProtocolRestrictions(SSL_CTX* ctx) void CryptoNative_SslCtxSetProtocolOptions(SSL_CTX* ctx, SslProtocols protocols) { + // void shim functions don't lead to exceptions, so skip the unconditional error clearing. + // Ensure that ECDHE is available if (TrySetECDHNamedCurve(ctx) == 0) { @@ -303,19 +308,12 @@ void CryptoNative_SslCtxSetProtocolOptions(SSL_CTX* ctx, SslProtocols protocols) SSL* CryptoNative_SslCreate(SSL_CTX* ctx) { + ERR_clear_error(); return SSL_new(ctx); } int32_t CryptoNative_SslGetError(SSL* ssl, int32_t ret) { - // This pops off "old" errors left by other operations - // until the first error is equal to the last one, - // this should be looked at again when OpenSsl 1.1 is migrated to - while (ERR_peek_error() != ERR_peek_last_error()) - { - ERR_get_error(); - } - // The error queue should be cleaned outside, if done here there will be no info // for managed exception. return SSL_get_error(ssl, ret); @@ -339,21 +337,26 @@ void CryptoNative_SslCtxDestroy(SSL_CTX* ctx) void CryptoNative_SslSetConnectState(SSL* ssl) { + // void shim functions don't lead to exceptions, so skip the unconditional error clearing. SSL_set_connect_state(ssl); } void CryptoNative_SslSetAcceptState(SSL* ssl) { + // void shim functions don't lead to exceptions, so skip the unconditional error clearing. SSL_set_accept_state(ssl); } const char* CryptoNative_SslGetVersion(SSL* ssl) { + // No error queue impact. return SSL_get_version(ssl); } int32_t CryptoNative_SslGetFinished(SSL* ssl, void* buf, int32_t count) { + // No error queue impact. + size_t result = SSL_get_finished(ssl, buf, (size_t)count); assert(result <= INT32_MAX); return (int32_t)result; @@ -361,6 +364,8 @@ int32_t CryptoNative_SslGetFinished(SSL* ssl, void* buf, int32_t count) int32_t CryptoNative_SslGetPeerFinished(SSL* ssl, void* buf, int32_t count) { + // No error queue impact. + size_t result = SSL_get_peer_finished(ssl, buf, (size_t)count); assert(result <= INT32_MAX); return (int32_t)result; @@ -368,12 +373,17 @@ int32_t CryptoNative_SslGetPeerFinished(SSL* ssl, void* buf, int32_t count) int32_t CryptoNative_SslSessionReused(SSL* ssl) { + // No error queue impact. + return SSL_session_reused(ssl) == 1; } int32_t CryptoNative_SslWrite(SSL* ssl, const void* buf, int32_t num, int32_t* error) { + ERR_clear_error(); + int32_t result = SSL_write(ssl, buf, num); + if (result > 0) { *error = SSL_ERROR_NONE; @@ -388,7 +398,10 @@ int32_t CryptoNative_SslWrite(SSL* ssl, const void* buf, int32_t num, int32_t* e int32_t CryptoNative_SslRead(SSL* ssl, void* buf, int32_t num, int32_t* error) { + ERR_clear_error(); + int32_t result = SSL_read(ssl, buf, num); + if (result > 0) { *error = SSL_ERROR_NONE; @@ -411,6 +424,8 @@ static int verify_callback(int preverify_ok, X509_STORE_CTX* store) int32_t CryptoNative_SslRenegotiate(SSL* ssl, int32_t* error) { + ERR_clear_error(); + #ifdef NEED_OPENSSL_1_1 // TLS1.3 uses different API for renegotiation/delayed client cert request #ifndef TLS1_3_VERSION @@ -455,6 +470,8 @@ int32_t CryptoNative_SslRenegotiate(SSL* ssl, int32_t* error) int32_t CryptoNative_IsSslRenegotiatePending(SSL* ssl) { + ERR_clear_error(); + SSL_peek(ssl, NULL, 0); return SSL_renegotiate_pending(ssl) != 0; } @@ -467,6 +484,7 @@ int32_t CryptoNative_SslShutdown(SSL* ssl) void CryptoNative_SslSetBio(SSL* ssl, BIO* rbio, BIO* wbio) { + // void shim functions don't lead to exceptions, so skip the unconditional error clearing. SSL_set_bio(ssl, rbio, wbio); } @@ -488,66 +506,80 @@ int32_t CryptoNative_SslDoHandshake(SSL* ssl, int32_t* error) int32_t CryptoNative_IsSslStateOK(SSL* ssl) { + // No error queue impact. return SSL_is_init_finished(ssl); } X509* CryptoNative_SslGetPeerCertificate(SSL* ssl) { + // No error queue impact. return SSL_get1_peer_certificate(ssl); } X509Stack* CryptoNative_SslGetPeerCertChain(SSL* ssl) { + // No error queue impact. return SSL_get_peer_cert_chain(ssl); } int32_t CryptoNative_SslUseCertificate(SSL* ssl, X509* x) { + ERR_clear_error(); return SSL_use_certificate(ssl, x); } int32_t CryptoNative_SslUsePrivateKey(SSL* ssl, EVP_PKEY* pkey) { + ERR_clear_error(); return SSL_use_PrivateKey(ssl, pkey); } int32_t CryptoNative_SslCtxUseCertificate(SSL_CTX* ctx, X509* x) { + ERR_clear_error(); return SSL_CTX_use_certificate(ctx, x); } int32_t CryptoNative_SslCtxUsePrivateKey(SSL_CTX* ctx, EVP_PKEY* pkey) { + ERR_clear_error(); return SSL_CTX_use_PrivateKey(ctx, pkey); } int32_t CryptoNative_SslCtxCheckPrivateKey(SSL_CTX* ctx) { + ERR_clear_error(); return SSL_CTX_check_private_key(ctx); } void CryptoNative_SslCtxSetQuietShutdown(SSL_CTX* ctx) { + // void shim functions don't lead to exceptions, so skip the unconditional error clearing. SSL_CTX_set_quiet_shutdown(ctx, 1); } void CryptoNative_SslSetQuietShutdown(SSL* ssl, int mode) { + // void shim functions don't lead to exceptions, so skip the unconditional error clearing. SSL_set_quiet_shutdown(ssl, mode); } X509NameStack* CryptoNative_SslGetClientCAList(SSL* ssl) { + // No error queue impact. return SSL_get_client_CA_list(ssl); } void CryptoNative_SslSetVerifyPeer(SSL* ssl) { + // void shim functions don't lead to exceptions, so skip the unconditional error clearing. SSL_set_verify(ssl, SSL_VERIFY_PEER, verify_callback); } void CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode) { + // void shim functions don't lead to exceptions, so skip the unconditional error clearing. + // We never reuse same CTX for both client and server SSL_CTX_ctrl(ctx, SSL_CTRL_SET_SESS_CACHE_MODE, mode ? SSL_SESS_CACHE_BOTH : SSL_SESS_CACHE_OFF, NULL); if (mode == 0) @@ -558,6 +590,8 @@ void CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode) int32_t CryptoNative_SslCtxSetEncryptionPolicy(SSL_CTX* ctx, EncryptionPolicy policy) { + // No error queue impact. + switch (policy) { case AllowNoEncryption: @@ -575,6 +609,8 @@ int32_t CryptoNative_SslCtxSetEncryptionPolicy(SSL_CTX* ctx, EncryptionPolicy po int32_t CryptoNative_SslCtxSetCiphers(SSL_CTX* ctx, const char* cipherList, const char* cipherSuites) { + ERR_clear_error(); + int32_t ret = true; // for < TLS 1.3 @@ -602,6 +638,8 @@ int32_t CryptoNative_SslCtxSetCiphers(SSL_CTX* ctx, const char* cipherList, cons int32_t CryptoNative_SetCiphers(SSL* ssl, const char* cipherList, const char* cipherSuites) { + ERR_clear_error(); + int32_t ret = true; // for < TLS 1.3 @@ -629,6 +667,8 @@ int32_t CryptoNative_SetCiphers(SSL* ssl, const char* cipherList, const char* ci const char* CryptoNative_GetOpenSslCipherSuiteName(SSL* ssl, int32_t cipherSuite, int32_t* isTls12OrLower) { + // No error queue impact. + #if HAVE_OPENSSL_SET_CIPHERSUITES unsigned char cs[2]; const SSL_CIPHER* cipher; @@ -689,6 +729,8 @@ const char* CryptoNative_GetOpenSslCipherSuiteName(SSL* ssl, int32_t cipherSuite int32_t CryptoNative_Tls13Supported() { + // No error queue impact. + #if HAVE_OPENSSL_SET_CIPHERSUITES return API_EXISTS(SSL_CTX_set_ciphersuites); #else @@ -698,6 +740,8 @@ int32_t CryptoNative_Tls13Supported() int32_t CryptoNative_SslCtxAddExtraChainCert(SSL_CTX* ctx, X509* x509) { + ERR_clear_error(); + if (!x509 || !ctx) { return 0; @@ -713,6 +757,8 @@ int32_t CryptoNative_SslCtxAddExtraChainCert(SSL_CTX* ctx, X509* x509) int32_t CryptoNative_SslAddExtraChainCert(SSL* ssl, X509* x509) { + ERR_clear_error(); + if (!x509 || !ssl) { return 0; @@ -728,6 +774,8 @@ int32_t CryptoNative_SslAddExtraChainCert(SSL* ssl, X509* x509) void CryptoNative_SslCtxSetAlpnSelectCb(SSL_CTX* ctx, SslCtxSetAlpnCallback cb, void* arg) { + // void shim functions don't lead to exceptions, so skip the unconditional error clearing. + #if HAVE_OPENSSL_ALPN if (API_EXISTS(SSL_CTX_set_alpn_select_cb)) { @@ -751,6 +799,8 @@ static int client_certificate_cb(SSL *ssl, void* state) void CryptoNative_SslSetClientCertCallback(SSL* ssl, int set) { + // void shim functions don't lead to exceptions, so skip the unconditional error clearing. + SSL_set_cert_cb(ssl, set ? client_certificate_cb : NULL, NULL); } @@ -769,16 +819,20 @@ void CryptoNative_SslSetPostHandshakeAuth(SSL* ssl, int32_t val) int32_t CryptoNative_SslSetData(SSL* ssl, void *ptr) { + ERR_clear_error(); return SSL_set_ex_data(ssl, 0, ptr); } void* CryptoNative_SslGetData(SSL* ssl) { + // No error queue impact. return SSL_get_ex_data(ssl, 0); } int32_t CryptoNative_SslSetAlpnProtos(SSL* ssl, const uint8_t* protos, uint32_t protos_len) { + ERR_clear_error(); + #if HAVE_OPENSSL_ALPN if (API_EXISTS(SSL_CTX_set_alpn_protos)) { @@ -797,6 +851,8 @@ int32_t CryptoNative_SslSetAlpnProtos(SSL* ssl, const uint8_t* protos, uint32_t void CryptoNative_SslGet0AlpnSelected(SSL* ssl, const uint8_t** protocol, uint32_t* len) { + // void shim functions don't lead to exceptions, so skip the unconditional error clearing. + #if HAVE_OPENSSL_ALPN if (API_EXISTS(SSL_get0_alpn_selected)) { @@ -814,11 +870,14 @@ void CryptoNative_SslGet0AlpnSelected(SSL* ssl, const uint8_t** protocol, uint32 int32_t CryptoNative_SslSetTlsExtHostName(SSL* ssl, uint8_t* name) { + ERR_clear_error(); return (int32_t)SSL_set_tlsext_host_name(ssl, name); } int32_t CryptoNative_SslGetCurrentCipherId(SSL* ssl, int32_t* cipherId) { + // No error queue impact. + const SSL_CIPHER* cipher = SSL_get_current_cipher(ssl); if (!cipher) { @@ -888,6 +947,9 @@ static int MakeSelfSignedCertificate(X509 * cert, EVP_PKEY* evp) int32_t CryptoNative_OpenSslGetProtocolSupport(SslProtocols protocol) { + // Many of these helpers already clear the error queue, and we unconditionally + // clear it at the end. + int ret = 0; SSL_CTX* clientCtx = CryptoNative_SslCtxCreate(TLS_method()); diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_x509.c b/src/native/libs/System.Security.Cryptography.Native/pal_x509.c index be20da8a4a4ff..549e6101d2fe6 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_x509.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_x509.c @@ -69,10 +69,9 @@ c_static_assert(PAL_X509_V_ERR_IP_ADDRESS_MISMATCH == X509_V_ERR_IP_ADDRESS_MISM EVP_PKEY* CryptoNative_GetX509EvpPublicKey(X509* x509) { - if (!x509) - { - return NULL; - } + assert(x509 != NULL); + + ERR_clear_error(); // X509_get_X509_PUBKEY returns an interior pointer, so should not be freed return X509_PUBKEY_get(X509_get_X509_PUBKEY(x509)); @@ -80,7 +79,9 @@ EVP_PKEY* CryptoNative_GetX509EvpPublicKey(X509* x509) X509_CRL* CryptoNative_DecodeX509Crl(const uint8_t* buf, int32_t len) { - if (!buf || !len) + ERR_clear_error(); + + if (buf == NULL || len == 0) { return NULL; } @@ -90,7 +91,9 @@ X509_CRL* CryptoNative_DecodeX509Crl(const uint8_t* buf, int32_t len) X509* CryptoNative_DecodeX509(const uint8_t* buf, int32_t len) { - if (!buf || !len) + ERR_clear_error(); + + if (buf == NULL || len == 0) { return NULL; } @@ -100,11 +103,13 @@ X509* CryptoNative_DecodeX509(const uint8_t* buf, int32_t len) int32_t CryptoNative_GetX509DerSize(X509* x) { + ERR_clear_error(); return i2d_X509(x, NULL); } int32_t CryptoNative_EncodeX509(X509* x, uint8_t* buf) { + ERR_clear_error(); return i2d_X509(x, &buf); } @@ -118,71 +123,86 @@ void CryptoNative_X509Destroy(X509* a) X509* CryptoNative_X509Duplicate(X509* x509) { + ERR_clear_error(); return X509_dup(x509); } X509* CryptoNative_PemReadX509FromBio(BIO* bio) { + ERR_clear_error(); return PEM_read_bio_X509(bio, NULL, NULL, NULL); } X509* CryptoNative_PemReadX509FromBioAux(BIO* bio) { + ERR_clear_error(); return PEM_read_bio_X509_AUX(bio, NULL, NULL, NULL); } ASN1_INTEGER* CryptoNative_X509GetSerialNumber(X509* x509) { + // Just a field accessor, no error queue interactions apply. return X509_get_serialNumber(x509); } X509_NAME* CryptoNative_X509GetIssuerName(X509* x509) { + // Just a field accessor, no error queue interactions apply. return X509_get_issuer_name(x509); } X509_NAME* CryptoNative_X509GetSubjectName(X509* x509) { + // Just a field accessor, no error queue interactions apply. return X509_get_subject_name(x509); } int32_t CryptoNative_X509CheckPurpose(X509* x, int32_t id, int32_t ca) { + ERR_clear_error(); return X509_check_purpose(x, id, ca); } uint64_t CryptoNative_X509IssuerNameHash(X509* x) { + ERR_clear_error(); return X509_issuer_name_hash(x); } int32_t CryptoNative_X509GetExtCount(X509* x) { + // Just a field accessor, no error queue interactions apply. return X509_get_ext_count(x); } X509_EXTENSION* CryptoNative_X509GetExt(X509* x, int32_t loc) { + // Just a field accessor, no error queue interactions apply. return X509_get_ext(x, loc); } ASN1_OBJECT* CryptoNative_X509ExtensionGetOid(X509_EXTENSION* x) { + // Just a field accessor, no error queue interactions apply. return X509_EXTENSION_get_object(x); } ASN1_OCTET_STRING* CryptoNative_X509ExtensionGetData(X509_EXTENSION* x) { + // Just a field accessor, no error queue interactions apply. return X509_EXTENSION_get_data(x); } int32_t CryptoNative_X509ExtensionGetCritical(X509_EXTENSION* x) { + // Just a field accessor, no error queue interactions apply. return X509_EXTENSION_get_critical(x); } ASN1_OCTET_STRING* CryptoNative_X509FindExtensionData(X509* x, int32_t nid) { + ERR_clear_error(); + if (x == NULL || nid == NID_undef) { return NULL; @@ -215,6 +235,7 @@ void CryptoNative_X509StoreDestory(X509_STORE* v) int32_t CryptoNative_X509StoreAddCrl(X509_STORE* ctx, X509_CRL* x) { + ERR_clear_error(); return X509_STORE_add_crl(ctx, x); } @@ -227,11 +248,13 @@ int32_t CryptoNative_X509StoreSetRevocationFlag(X509_STORE* ctx, X509RevocationF verifyFlags |= X509_V_FLAG_CRL_CHECK_ALL; } + // Just a field mutator, no error queue interactions apply. return X509_STORE_set_flags(ctx, verifyFlags); } X509_STORE_CTX* CryptoNative_X509StoreCtxCreate() { + ERR_clear_error(); return X509_STORE_CTX_new(); } @@ -245,6 +268,8 @@ void CryptoNative_X509StoreCtxDestroy(X509_STORE_CTX* v) int32_t CryptoNative_X509StoreCtxInit(X509_STORE_CTX* ctx, X509_STORE* store, X509* x509, X509Stack* extraStore) { + ERR_clear_error(); + int32_t val = X509_STORE_CTX_init(ctx, store, x509, extraStore); if (val != 0) @@ -257,11 +282,13 @@ int32_t CryptoNative_X509StoreCtxInit(X509_STORE_CTX* ctx, X509_STORE* store, X5 int32_t CryptoNative_X509VerifyCert(X509_STORE_CTX* ctx) { + ERR_clear_error(); return X509_verify_cert(ctx); } X509Stack* CryptoNative_X509StoreCtxGetChain(X509_STORE_CTX* ctx) { + ERR_clear_error(); return X509_STORE_CTX_get1_chain(ctx); } @@ -272,6 +299,7 @@ X509* CryptoNative_X509StoreCtxGetCurrentCert(X509_STORE_CTX* ctx) return NULL; } + // Just a field accessor, no error queue interactions apply. X509* cert = X509_STORE_CTX_get_current_cert(ctx); if (cert != NULL) @@ -286,6 +314,7 @@ X509Stack* CryptoNative_X509StoreCtxGetSharedUntrusted(X509_STORE_CTX* ctx) { if (ctx) { + // Just a field accessor, no error queue interactions apply. return X509_STORE_CTX_get0_untrusted(ctx); } @@ -294,11 +323,14 @@ X509Stack* CryptoNative_X509StoreCtxGetSharedUntrusted(X509_STORE_CTX* ctx) int32_t CryptoNative_X509StoreCtxGetError(X509_STORE_CTX* ctx) { + // Just a field accessor, no error queue interactions apply. return (int32_t)X509_STORE_CTX_get_error(ctx); } int32_t CryptoNative_X509StoreCtxReset(X509_STORE_CTX* ctx) { + ERR_clear_error(); + X509* leaf = X509_STORE_CTX_get0_cert(ctx); X509Stack* untrusted = X509_STORE_CTX_get0_untrusted(ctx); X509_STORE* store = X509_STORE_CTX_get0_store(ctx); @@ -309,6 +341,7 @@ int32_t CryptoNative_X509StoreCtxReset(X509_STORE_CTX* ctx) int32_t CryptoNative_X509StoreCtxRebuildChain(X509_STORE_CTX* ctx) { + // Callee clears the error queue already if (!CryptoNative_X509StoreCtxReset(ctx)) { return -1; @@ -319,16 +352,19 @@ int32_t CryptoNative_X509StoreCtxRebuildChain(X509_STORE_CTX* ctx) void CryptoNative_X509StoreCtxSetVerifyCallback(X509_STORE_CTX* ctx, X509StoreVerifyCallback callback) { + // Just a field mutator, no error queue interactions apply. X509_STORE_CTX_set_verify_cb(ctx, callback); } int32_t CryptoNative_X509StoreCtxGetErrorDepth(X509_STORE_CTX* ctx) { + // Just a field accessor, no error queue interactions apply. return X509_STORE_CTX_get_error_depth(ctx); } const char* CryptoNative_X509VerifyCertErrorString(int32_t n) { + // Called function is a hard-coded lookup table, no error queue interactions apply. return X509_verify_cert_error_string((long)n); } @@ -342,16 +378,20 @@ void CryptoNative_X509CrlDestroy(X509_CRL* a) int32_t CryptoNative_PemWriteBioX509Crl(BIO* bio, X509_CRL* crl) { + ERR_clear_error(); return PEM_write_bio_X509_CRL(bio, crl); } X509_CRL* CryptoNative_PemReadBioX509Crl(BIO* bio) { + ERR_clear_error(); return PEM_read_bio_X509_CRL(bio, NULL, NULL, NULL); } int32_t CryptoNative_GetX509SubjectPublicKeyInfoDerSize(X509* x509) { + ERR_clear_error(); + if (!x509) { return 0; @@ -363,6 +403,8 @@ int32_t CryptoNative_GetX509SubjectPublicKeyInfoDerSize(X509* x509) int32_t CryptoNative_EncodeX509SubjectPublicKeyInfo(X509* x509, uint8_t* buf) { + ERR_clear_error(); + if (!x509) { return 0; @@ -376,6 +418,7 @@ X509* CryptoNative_X509UpRef(X509* x509) { if (x509 != NULL) { + // Just a field mutator, no error queue interactions apply. X509_up_ref(x509); } @@ -410,6 +453,8 @@ static DIR* OpenUserStore(const char* storePath, char** pathTmp, size_t* pathTmp static X509* ReadNextPublicCert(DIR* dir, X509Stack* tmpStack, char* pathTmp, size_t pathTmpSize, char* nextFileWrite) { + // Callers of this routine are responsible for appropriately clearing the error queue. + struct dirent* next; ptrdiff_t offset = nextFileWrite - pathTmp; assert(offset > 0); @@ -475,6 +520,7 @@ static X509* ReadNextPublicCert(DIR* dir, X509Stack* tmpStack, char* pathTmp, si X509_STORE* CryptoNative_X509ChainNew(X509Stack* systemTrust, X509Stack* userTrust) { + ERR_clear_error(); X509_STORE* store = X509_STORE_new(); if (store == NULL) @@ -534,6 +580,8 @@ int32_t CryptoNative_X509StackAddDirectoryStore(X509Stack* stack, char* storePat return -1; } + ERR_clear_error(); + int clearError = 1; char* pathTmp; size_t pathTmpSize; @@ -545,6 +593,11 @@ int32_t CryptoNative_X509StackAddDirectoryStore(X509Stack* stack, char* storePat X509* cert; X509Stack* tmpStack = sk_X509_new_null(); + if (tmpStack == NULL) + { + return 0; + } + while ((cert = ReadNextPublicCert(storeDir, tmpStack, pathTmp, pathTmpSize, nextFileWrite)) != NULL) { if (!sk_X509_push(stack, cert)) @@ -580,6 +633,8 @@ int32_t CryptoNative_X509StackAddMultiple(X509Stack* dest, X509Stack* src) return -1; } + ERR_clear_error(); + int success = 1; if (src != NULL) @@ -609,6 +664,7 @@ int32_t CryptoNative_X509StoreCtxCommitToChain(X509_STORE_CTX* storeCtx) return -1; } + ERR_clear_error(); X509Stack* chain = X509_STORE_CTX_get1_chain(storeCtx); if (chain == NULL) @@ -664,6 +720,8 @@ int32_t CryptoNative_X509StoreCtxResetForSignatureError(X509_STORE_CTX* storeCtx *newStore = NULL; + ERR_clear_error(); + int errorDepth = X509_STORE_CTX_get_error_depth(storeCtx); X509Stack* chain = X509_STORE_CTX_get0_chain(storeCtx); int chainLength = sk_X509_num(chain); @@ -946,6 +1004,8 @@ int32_t CryptoNative_X509ChainGetCachedOcspStatus(X509_STORE_CTX* storeCtx, char return -1; } + ERR_clear_error(); + X509* subject; X509* issuer; @@ -1031,6 +1091,8 @@ OCSP_REQUEST* CryptoNative_X509ChainBuildOcspRequest(X509_STORE_CTX* storeCtx, i return NULL; } + ERR_clear_error(); + X509* subject; X509* issuer; @@ -1077,6 +1139,8 @@ CryptoNative_X509ChainVerifyOcsp(X509_STORE_CTX* storeCtx, OCSP_REQUEST* req, OC return -1; } + ERR_clear_error(); + X509* subject; X509* issuer; diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_x509_name.c b/src/native/libs/System.Security.Cryptography.Native/pal_x509_name.c index c18b890caf79a..20d3bafc63a87 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_x509_name.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_x509_name.c @@ -5,10 +5,12 @@ int32_t CryptoNative_GetX509NameStackFieldCount(X509NameStack* sk) { + // No error queie impact. return sk_X509_NAME_num(sk); } X509_NAME* CryptoNative_GetX509NameStackField(X509NameStack* sk, int32_t loc) { + // No error queue impact. return sk_X509_NAME_value(sk, loc); } diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_x509_root.c b/src/native/libs/System.Security.Cryptography.Native/pal_x509_root.c index 81b3d80d9d8f5..4454b19945c75 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_x509_root.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_x509_root.c @@ -10,6 +10,8 @@ const char* CryptoNative_GetX509RootStorePath(uint8_t* defaultPath) { assert(defaultPath != NULL); + // No error queue impact. + const char* dir = getenv(X509_get_default_cert_dir_env()); *defaultPath = 0; @@ -26,6 +28,8 @@ const char* CryptoNative_GetX509RootStoreFile(uint8_t* defaultPath) { assert(defaultPath != NULL); + // No error queue impact. + const char* file = getenv(X509_get_default_cert_file_env()); *defaultPath = 0; diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_x509ext.c b/src/native/libs/System.Security.Cryptography.Native/pal_x509ext.c index cebc3b5310c85..908477ae5ecd8 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_x509ext.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_x509ext.c @@ -9,6 +9,7 @@ X509_EXTENSION* CryptoNative_X509ExtensionCreateByObj(ASN1_OBJECT* obj, int32_t isCritical, ASN1_OCTET_STRING* data) { + ERR_clear_error(); return X509_EXTENSION_create_by_OBJ(NULL, obj, isCritical, data); } @@ -22,6 +23,7 @@ void CryptoNative_X509ExtensionDestroy(X509_EXTENSION* a) int32_t CryptoNative_X509V3ExtPrint(BIO* out, X509_EXTENSION* ext) { + ERR_clear_error(); return X509V3_EXT_print(out, ext, X509V3_EXT_DEFAULT, /*indent*/ 0); } @@ -41,6 +43,8 @@ int32_t CryptoNative_DecodeX509BasicConstraints2Extension(const uint8_t* encoded *pathLengthConstraint = 0; int32_t result = false; + ERR_clear_error(); + BASIC_CONSTRAINTS* constraints = d2i_BASIC_CONSTRAINTS(NULL, &encoded, encodedLength); if (constraints) { @@ -70,6 +74,8 @@ int32_t CryptoNative_DecodeX509BasicConstraints2Extension(const uint8_t* encoded EXTENDED_KEY_USAGE* CryptoNative_DecodeExtendedKeyUsage(const uint8_t* buf, int32_t len) { + ERR_clear_error(); + if (!buf || !len) { return NULL;