NCryptSetProperty - The security descriptor structure is invalid
Asked Answered
S

2

6

In Window, I'm trying to create a key in the TPM with the help of the NCrypt library and restrict the access to only my application in C++, but I get the error: "The security descriptor structure is invalid"

To help with readability I've removed all error controls from the code

// Get App SID
DWORD pid = GetCurrentProcessId();
HANDLE hProcess = OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, pid);

HANDLE hToken;
OpenProcessToken(hProcess, TOKEN_QUERY, &hToken);

DWORD bufferSize = 0;
GetTokenInformation(hToken, TokenUser, NULL, 0, &bufferSize);

PTOKEN_USER tokenUser = reinterpret_cast<PTOKEN_USER>(malloc(bufferSize));

// I suspect that the tokenUser->User.Sid is related to the current user logged in,
// but I couldn't find any other way to get and identifier related to the running
// process that is consistent across multiple executions
GetTokenInformation(hToken, TokenUser, tokenUser, bufferSize, &bufferSize);

EXPLICIT_ACCESS ea[1];
ZeroMemory(&ea, sizeof(EXPLICIT_ACCESS));
ea[0].grfAccessPermissions = WRITE_OWNER;
ea[0].grfAccessMode = SET_ACCESS;
ea[0].grfInheritance = NO_INHERITANCE;
ea[0].Trustee.TrusteeForm = TRUSTEE_IS_SID;
ea[0].Trustee.TrusteeType = TRUSTEE_IS_USER;
ea[0].Trustee.ptstrName = (LPTSTR)tokenUser->User.Sid;

PACL pACL = NULL;
SetEntriesInAcl(1, ea, NULL, &pACL);

PSECURITY_DESCRIPTOR pSD = NULL;
pSD = (PSECURITY_DESCRIPTOR)LocalAlloc(LPTR, SECURITY_DESCRIPTOR_MIN_LENGTH);
InitializeSecurityDescriptor(pSD, SECURITY_DESCRIPTOR_REVISION);

SetSecurityDescriptorDacl(pSD, TRUE, pACL, FALSE);
SECURITY_ATTRIBUTES sa;
sa.nLength = sizeof(SECURITY_ATTRIBUTES);
sa.lpSecurityDescriptor = pSD;
sa.bInheritHandle = FALSE;

NCryptSetProperty(hKey, NCRYPT_SECURITY_DESCR_PROPERTY, reinterpret_cast<PBYTE>(&sa), sizeof(SECURITY_ATTRIBUTES), NCRYPT_PERSIST_FLAG | DACL_SECURITY_INFORMATION);

I also tried the approach suggested here: https://forums.codeguru.com/showthread.php?301326-CreateDirectory-and-SECURITY_ATTRIBUTES

Is the flag NCRYPT_SECURITY_DESCR_PROPERTY not supported yet?

Sumpter answered 30/4, 2024 at 20:46 Comment(0)
G
0

There are a couple of issues in your code that I noted should be modified for the NCryptSetProperty function to work properly.

One is that the function does not receive a pointer to a SECURITY_ATTRIBUTES structure, but a pointer to a SECURITY_DESCRIPTOR. You can get the size of the built security descriptor by using the function GetSecurityDescriptorLength. Please, note that there is a MINIMUM security descriptor size for a security descriptor without entries in the control access list. However, the size grows as you add entries to the control access list of the security descriptor.

Another issue that I noted in your code is that the permissions added to the EXPLICIT_ACCESS only includes WRITE_OWNER. You do not include the WRITE_DACL permissions. You might not be able to create the key, specially if it exists with that only permission in the access control list. If the key exists and it was created with broader access, it would allow you to modify it. However, if you try to rewrite it with NCRYPT_OVERWRITE_KEY_FLAG flag, and you only take the WRITE_OWNER flag with you while that is exactly the only permission already in the key, you might not be able to modify the DACL. You need to save the key with the WRITE_DACL permission and take it with you next time if you rewrite the key.

If you already locked the key with insufficient permissions while debugging, you go to %ALLUSERSPROFILE%\Application Data\Microsoft\Crypto\Keys, locate the key, and change its permissions in such a way that you can rewrite the key with your code using an appropriate permission flags in the EXPLICIT_ACCESS structure.

Another issue, the result of the function SetEntriesInAcl has not been checked. The function expects a pointer of an EXCLICIT_ACCESS_W. Make sure that you are using an EXPLICIT_ACCESS_W, a consistent version with the UNICODE version of the function SetEntriesInAcl. If you the result of that call is not checked, you do not know if you are actually building a valid and appropriate access control list inside the security descriptor.

You can verify if you build correctly the security descriptor by using IsValidSecurityDescriptor function. That is a primary test. The NCryptSetProperty function will again taste your security descriptor against the flags passed in. If the security descriptor does not contain what the flags passed in say, the function NCryptFinalizeKey will return Win32 error 1338 ERROR_INVALID_SECURITY_DESCR.

Here I have a code I had wrote some time ago to build a security descriptor from the entries in the pMdtsAccessControl parameter:

if ((dwFlags & AC_SECURITY_DESCRIPTOR_FROM_SYSTEM) == AC_SECURITY_DESCRIPTOR_FROM_SYSTEM) {
    if (cMdtsAccessControl <= 6) {

        pEa = calloc(cMdtsAccessControl, sizeof(EXPLICIT_ACCESSW));
        if (pEa == NULL) {
            dwError = ERROR_OUTOFMEMORY;
            goto cleanup;
        }

        int i = 0;
        for (; i < cMdtsAccessControl; i++) {

            switch (pMdtsAccessControl[i].ObjectUser) {

            case AC_USER:                   
            {
                if (pUserSID == NULL) {

                    SID_IDENTIFIER_AUTHORITY SIDAuthUsers = SECURITY_WORLD_SID_AUTHORITY;
                    if (AllocateAndInitializeSid(&SIDAuthUsers, 1, SECURITY_WORLD_RID, 0, 0, 0, 0, 0, 0, 0, &pUserSID) == FALSE) {
                        dwError = GetLastError();
                        goto cleanup;
                    }
                }
            
                pEa[i].grfAccessPermissions             = pMdtsAccessControl[i].grfAccessPermissions;
                pEa[i].grfAccessMode                    = pMdtsAccessControl[i].ObjectAccessMode;
                pEa[i].grfInheritance                   = pMdtsAccessControl[i].grfInheritance;             
                pEa[i].Trustee.TrusteeForm              = TRUSTEE_IS_SID;
                pEa[i].Trustee.TrusteeType              = TRUSTEE_IS_WELL_KNOWN_GROUP;
                pEa[i].Trustee.ptstrName                = (LPWCH)pUserSID;
                pEa[i].Trustee.MultipleTrusteeOperation = NO_MULTIPLE_TRUSTEE;
                pEa[i].Trustee.pMultipleTrustee         = NULL;

                break;
            }

            case AC_OWNER:
            {
                if (pOwnerSid == NULL) {
                    dwError = ERROR_INVALID_PARAMETER;
                    goto cleanup;
                }

                if (IsValidSid(pOwnerSid->pOwnerSid) == FALSE) {
                    dwError = ERROR_INVALID_SID;
                    goto cleanup;
                }
                                
                pEa[i].grfAccessPermissions             = pMdtsAccessControl[i].grfAccessPermissions;
                pEa[i].grfAccessMode                    = pMdtsAccessControl[i].ObjectAccessMode;
                pEa[i].grfInheritance                   = pMdtsAccessControl[i].grfInheritance;
                pEa[i].Trustee.TrusteeForm              = TRUSTEE_IS_SID;
                pEa[i].Trustee.TrusteeType              = TRUSTEE_IS_USER;
                pEa[i].Trustee.ptstrName                = pOwnerSid->pOwnerSid;
                pEa[i].Trustee.MultipleTrusteeOperation = NO_MULTIPLE_TRUSTEE;
                pEa[i].Trustee.pMultipleTrustee         = NULL;

                break;
            }
            
            case AC_ADMIN:
            {

                if (pAdminSID == NULL) {
                    SID_IDENTIFIER_AUTHORITY  SIDAuthNT = SECURITY_NT_AUTHORITY;
                    if (AllocateAndInitializeSid(&SIDAuthNT, 2, SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_ADMINS, 0, 0, 0, 0, 0, 0, &pAdminSID) == FALSE) {
                        dwError = GetLastError();
                        goto cleanup;
                    }
                }

                pEa[i].grfAccessMode                    = pMdtsAccessControl[i].ObjectAccessMode;
                pEa[i].grfAccessPermissions             = pMdtsAccessControl[i].grfAccessPermissions;
                pEa[i].grfInheritance                   = pMdtsAccessControl[i].grfInheritance;
                pEa[i].Trustee.TrusteeForm              = TRUSTEE_IS_SID;
                pEa[i].Trustee.TrusteeType              = TRUSTEE_IS_GROUP;
                pEa[i].Trustee.ptstrName                = (LPWCH)pAdminSID;
                pEa[i].Trustee.MultipleTrusteeOperation = NO_MULTIPLE_TRUSTEE;
                pEa[i].Trustee.pMultipleTrustee         = NULL;

                break;
            }
            default:
                dwError = ERROR_INVALID_HANDLE;
                goto cleanup;
                break;
            }
        }

        // Set the entries in the EXPLICIT_ACCESSW
        if (i > 0) {
            if (SetEntriesInAcl(i, (PEXPLICIT_ACCESSW)pEa, NULL, &pAcl) != ERROR_SUCCESS) {
                dwError = GetLastError();
                goto cleanup;
            }
        }           
    }

    pSecurityDesc = (PSECURITY_DESCRIPTOR)LocalAlloc(LPTR, SECURITY_DESCRIPTOR_MIN_LENGTH);
    if (pSecurityDesc == NULL) {
        dwError = ERROR_OUTOFMEMORY;
        goto cleanup;
    }

    if (InitializeSecurityDescriptor(pSecurityDesc, SECURITY_DESCRIPTOR_REVISION1) == FALSE) {
        dwError = GetLastError();
        goto cleanup;
    }

    if (pOwnerSid) {
        if (SetSecurityDescriptorOwner(pSecurityDesc, pOwnerSid->pOwnerSid, FALSE) == FALSE) {
            dwError = GetLastError();
            goto cleanup;
        }

        if (SetSecurityDescriptorGroup(pSecurityDesc, pOwnerSid->pOwnerSid, FALSE) == FALSE) {
            dwError = GetLastError();
            goto cleanup;
        }
    }
    else {
        if (pAdminSID) {
            if (SetSecurityDescriptorOwner(pSecurityDesc, pAdminSID, FALSE) == FALSE) {
                dwError = GetLastError();
                goto cleanup;
            }

            if (SetSecurityDescriptorGroup(pSecurityDesc, pAdminSID, FALSE) == FALSE) {
                dwError = GetLastError();
                goto cleanup;
            }
        }
    }

    if (pAcl) {
        if (SetSecurityDescriptorDacl(pSecurityDesc, TRUE, pAcl, FALSE) == FALSE) {
            dwError = GetLastError();
            goto cleanup;
        }
    }
        
} // END OF AC_SECURITY_DESCRIPTOR_FROM_SYSTEM

if (dwError == NOERROR && pSecurityDesc && IsValidSecurityDescriptor(pSecurityDesc)) {
    pSecAttr->nLength              = sizeof(SECURITY_ATTRIBUTES);
    pSecAttr->bInheritHandle       = bInheritHandle;
    pSecAttr->lpSecurityDescriptor = pSecurityDesc;
}

// Then the NCryptSetProperty part of the code

if (IsValidSecurityDescriptor(pSecurityDesc) == FALSE) {
            secStatus = HRESULT_FROM_WIN32(GetLsstError());
            goto cleanup;
}

DWORD dwSecurityDescriptorLength = GetSecurityDescriptorLength(pSecurityDesc);

secStatus = NCryptSetProperty(
            hNKey,
            NCRYPT_SECURITY_DESCR_PROPERTY,
            pSecurityDesc,
            dwSecurityDescriptorLength,
            OWNER_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION | GROUP_SECURITY_INFORMATION);

if (secStatus != ERROR_SUCCESS) {
    goto cleanup;
}
Gula answered 8/7, 2024 at 3:55 Comment(0)
G
0

Please, check the return codes from the function calls in your code. Almost none of the return codes from the function calls are checked in your provided code. That way the invalid portion of the process of building a security descriptor can be exposed.

Gula answered 8/7, 2024 at 4:35 Comment(2)
This does not provide an answer to the question. Once you have sufficient reputation you will be able to comment on any post; instead, provide answers that don't require clarification from the asker. - From ReviewBadajoz
Hi @opena, thanks for your reply. As I mentioned in the question, I intentionally removed all error controls from the code to help with readability as I don't think they add value to the question added. The original code had all the controls to avoid invalid values to go throughSumpter

© 2022 - 2025 — McMap. All rights reserved.