Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move most of the remaining Strong Name code to internal to the C++ file and remove duplicate validation #95536

Merged
merged 10 commits into from
Dec 6, 2023
10 changes: 0 additions & 10 deletions src/coreclr/inc/strongnameinternal.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,6 @@ typedef struct {
BYTE PublicKey[1]; // variable length byte array containing the key value in format output by CryptoAPI
} PublicKeyBlob;

// Determine the number of bytes in a public key
DWORD StrongNameSizeOfPublicKey(const PublicKeyBlob &keyPublicKey);

bool StrongNameIsValidPublicKey(_In_reads_(cbPublicKeyBlob) const BYTE *pbPublicKeyBlob, DWORD cbPublicKeyBlob);
bool StrongNameIsValidPublicKey(const PublicKeyBlob &keyPublicKey);

// Determine if a public key is the ECMA key
bool StrongNameIsEcmaKey(_In_reads_(cbKey) const BYTE *pbKey, DWORD cbKey);
bool StrongNameIsEcmaKey(const PublicKeyBlob &keyPublicKey);

HRESULT StrongNameTokenFromPublicKey(BYTE* pbPublicKeyBlob, // [in] public key blob
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
ULONG cbPublicKeyBlob,
BYTE** ppbStrongNameToken, // [out] strong name token
Expand Down
112 changes: 42 additions & 70 deletions src/coreclr/md/runtime/strongnameinternal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,28 @@

#include "stdafx.h"
#include "strongnameinternal.h"
#include "sha1.h"

// We allow a special abbreviated form of the Microsoft public key (16 bytes
// long: 0 for both alg ids, 4 for key length and 4 bytes of 0 for the key
// itself). This allows us to build references to system libraries that are
// platform neutral (so a 3rd party can build SPCL replacements). The
// special zero PK is just shorthand for the local runtime's real system PK,
// which is always used to perform the signature verification, so no security
// hole is opened by this. Therefore we need to store a copy of the real PK (for
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
// this platform) here.
#include "thekey.h"
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
#include "ecmakey.h"
#include "sha1.h"

//---------------------------------------------------------------------------------------
//
// Check to see if a public key blob is the ECMA public key blob
// Determine the number of bytes that a public key blob occupies, including the key portion
//
// Arguments:
// pbKey - public key blob to check
// cbKey - size in bytes of pbKey
// keyPublicKey - key blob to calculate the size of
//

bool StrongNameIsEcmaKey(_In_reads_(cbKey) const BYTE *pbKey, DWORD cbKey)
DWORD StrongNameSizeOfPublicKey(const PublicKeyBlob &keyPublicKey)
{
CONTRACTL
{
Expand All @@ -28,14 +36,8 @@ bool StrongNameIsEcmaKey(_In_reads_(cbKey) const BYTE *pbKey, DWORD cbKey)
}
CONTRACTL_END;

// The key should be the same size as the ECMA key
if (cbKey != sizeof(g_rbNeutralPublicKey))
{
return false;
}

const PublicKeyBlob *pKeyBlob = reinterpret_cast<const PublicKeyBlob *>(pbKey);
return StrongNameIsEcmaKey(*pKeyBlob);
return offsetof(PublicKeyBlob, PublicKey) + // Size of the blob header plus
GET_UNALIGNED_VAL32(&keyPublicKey.cbPublicKey); // the number of bytes in the key
}

//---------------------------------------------------------------------------------------
Expand All @@ -61,38 +63,30 @@ bool StrongNameIsEcmaKey(const PublicKeyBlob &keyPublicKey)

//---------------------------------------------------------------------------------------
//
// Verify that a public key blob looks like a reasonable public key
// Check to see if a public key blob is the ECMA public key blob
//
// Arguments:
// pbBuffer - buffer to verify the format of
// cbBuffer - size of pbBuffer
// pbKey - public key blob to check
// cbKey - size in bytes of pbKey
//

bool StrongNameIsValidPublicKey(_In_reads_(cbBuffer) const BYTE *pbBuffer, DWORD cbBuffer)
bool StrongNameIsEcmaKey(_In_reads_(cbKey) const BYTE *pbKey, DWORD cbKey)
{
CONTRACTL
{
PRECONDITION(CheckPointer(pbBuffer));
NOTHROW;
GC_NOTRIGGER;
}
CONTRACTL_END;

// The buffer must be at least as large as the public key structure
if (cbBuffer < sizeof(PublicKeyBlob))
{
return false;
}

// The buffer must be the same size as the structure header plus the trailing key data
const PublicKeyBlob *pkeyPublicKey = reinterpret_cast<const PublicKeyBlob *>(pbBuffer);
if (GET_UNALIGNED_VAL32(&pkeyPublicKey->cbPublicKey) != cbBuffer - offsetof(PublicKeyBlob, PublicKey))
// The key should be the same size as the ECMA key
if (cbKey != sizeof(g_rbNeutralPublicKey))
{
return false;
}

// The buffer itself looks reasonable, but the public key structure needs to be validated as well
return StrongNameIsValidPublicKey(*pkeyPublicKey);
const PublicKeyBlob *pKeyBlob = reinterpret_cast<const PublicKeyBlob *>(pbKey);
return StrongNameIsEcmaKey(*pKeyBlob);
}

//---------------------------------------------------------------------------------------
Expand Down Expand Up @@ -146,30 +140,41 @@ bool StrongNameIsValidPublicKey(const PublicKeyBlob &keyPublicKey)
return true;
}


//---------------------------------------------------------------------------------------
//
// Determine the number of bytes that a public key blob occupies, including the key portion
// Verify that a public key blob looks like a reasonable public key
//
// Arguments:
// keyPublicKey - key blob to calculate the size of
// pbBuffer - buffer to verify the format of
// cbBuffer - size of pbBuffer
//

DWORD StrongNameSizeOfPublicKey(const PublicKeyBlob &keyPublicKey)
bool StrongNameIsValidPublicKey(_In_reads_(cbBuffer) const BYTE *pbBuffer, DWORD cbBuffer)
{
CONTRACTL
{
PRECONDITION(CheckPointer(pbBuffer));
NOTHROW;
GC_NOTRIGGER;
}
CONTRACTL_END;

return offsetof(PublicKeyBlob, PublicKey) + // Size of the blob header plus
GET_UNALIGNED_VAL32(&keyPublicKey.cbPublicKey); // the number of bytes in the key
}

// The buffer must be at least as large as the public key structure
if (cbBuffer < sizeof(PublicKeyBlob))
{
return false;
}

// The buffer must be the same size as the structure header plus the trailing key data
const PublicKeyBlob *pkeyPublicKey = reinterpret_cast<const PublicKeyBlob *>(pbBuffer);
if (GET_UNALIGNED_VAL32(&pkeyPublicKey->cbPublicKey) != cbBuffer - offsetof(PublicKeyBlob, PublicKey))
{
return false;
}

// The buffer itself looks reasonable, but the public key structure needs to be validated as well
return StrongNameIsValidPublicKey(*pkeyPublicKey);
}

// Size in bytes of strong name token.
#define SN_SIZEOF_TOKEN 8
Expand Down Expand Up @@ -277,34 +282,6 @@ HRESULT StrongNameTokenFromPublicKey(BYTE *pbPublicKeyBlob, // [in] pu
goto Exit;
}

// To compute the correct public key token, we need to make sure the public key blob
// was not padded with extra bytes that CAPI CryptImportKey would've ignored.
// Without this round trip, we would blindly compute the hash over the padded bytes
// which could make finding a public key token collision a significantly easier task
// since an attacker wouldn't need to work hard on generating valid key pairs before hashing.
if (cbPublicKeyBlob <= sizeof(PublicKeyBlob)) {
hr = CORSEC_E_INVALID_PUBLICKEY;
goto Error;
}

// Check that the blob type is PUBLICKEYBLOB.
pPublicKey = (PublicKeyBlob*) pbPublicKeyBlob;

if (GET_UNALIGNED_VAL32(&pPublicKey->cbPublicKey) > cbPublicKeyBlob) {
hr = CORSEC_E_INVALID_PUBLICKEY;
goto Error;
}

if (cbPublicKeyBlob < SN_SIZEOF_KEY(pPublicKey)) {
hr = CORSEC_E_INVALID_PUBLICKEY;
goto Error;
}

if (*(BYTE*) pPublicKey->PublicKey /* PUBLICKEYSTRUC->bType */ != PUBLICKEYBLOB) {
hr = CORSEC_E_INVALID_PUBLICKEY;
goto Error;
}

// Compute a hash over the public key.
sha1.AddData(pbPublicKeyBlob, cbPublicKeyBlob);
pHash = sha1.GetHash();
Expand All @@ -319,11 +296,6 @@ HRESULT StrongNameTokenFromPublicKey(BYTE *pbPublicKeyBlob, // [in] pu

goto Exit;

Error:
if (*ppbStrongNameToken) {
delete [] *ppbStrongNameToken;
*ppbStrongNameToken = NULL;
}
Exit:
#else
DacNotImpl();
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/vm/methodtablebuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include "dllimport.h"
#include "fieldmarshaler.h"
#include "encee.h"
#include "ecmakey.h"
#include "customattribute.h"
#include "typestring.h"

Expand Down