Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 82485a26 authored by Nathan Harold's avatar Nathan Harold
Browse files

Check mOwnedByTransform to avoid DELSA on SPI 

The owned by transform flag prevents the removal
of an SPI from accidentally deleting an associated
SA in the kernel. That flag wasn't actually being
checked, so deleting an SPI would result in the
transform being removed.

The existing code already guarantees that the SA is
deleted when the transform is deleted

Bug: 73258845
Test: runtest frameworks-net
Merged-In: I4c26aea7af817a5d9e54da5db1cdf4f943bcae06
Change-Id: I4c26aea7af817a5d9e54da5db1cdf4f943bcae06
(cherry picked from commit 22795302)
parent c8f63060

services/core/java/com/android/server/IpSecService.java

+6 −4
Original line number Diff line number Diff line
@@ -676,10 +676,12 @@ public class IpSecService extends IIpSecService.Stub { 
        @Override

        public void freeUnderlyingResources() {

            try {

                if (!mOwnedByTransform) {

                    mSrvConfig

                            .getNetdInstance()

                            .ipSecDeleteSecurityAssociation(

                                    mResourceId, mSourceAddress, mDestinationAddress, mSpi, 0, 0);

                }

            } catch (ServiceSpecificException | RemoteException e) {

                Log.e(TAG, "Failed to delete SPI reservation with ID: " + mResourceId, e);

            }

tests/net/java/com/android/server/IpSecServiceParameterizedTest.java

+41 −1
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ import static org.mockito.Matchers.anyInt; 
import static org.mockito.Matchers.anyString;

import static org.mockito.Matchers.eq;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.times;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;
@@ -319,6 +320,30 @@ public class IpSecServiceParameterizedTest { 
        }

    }



    @Test

    public void testReleaseOwnedSpi() throws Exception {

        IpSecConfig ipSecConfig = new IpSecConfig();

        addDefaultSpisAndRemoteAddrToIpSecConfig(ipSecConfig);

        addAuthAndCryptToIpSecConfig(ipSecConfig);



        IpSecTransformResponse createTransformResp =

                mIpSecService.createTransform(ipSecConfig, new Binder());

        IpSecService.UserRecord userRecord =

                mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid());

        assertEquals(1, userRecord.mSpiQuotaTracker.mCurrent);

        mIpSecService.releaseSecurityParameterIndex(ipSecConfig.getSpiResourceId());

        verify(mMockNetd, times(0))

                .ipSecDeleteSecurityAssociation(

                        eq(createTransformResp.resourceId),

                        anyString(),

                        anyString(),

                        eq(TEST_SPI),

                        anyInt(),

                        anyInt());

        // quota is not released until the SPI is released by the Transform

        assertEquals(1, userRecord.mSpiQuotaTracker.mCurrent);

    }



    @Test

    public void testDeleteTransform() throws Exception {

        IpSecConfig ipSecConfig = new IpSecConfig();
@@ -329,7 +354,7 @@ public class IpSecServiceParameterizedTest { 
                mIpSecService.createTransform(ipSecConfig, new Binder());

        mIpSecService.deleteTransform(createTransformResp.resourceId);



        verify(mMockNetd)

        verify(mMockNetd, times(1))

                .ipSecDeleteSecurityAssociation(

                        eq(createTransformResp.resourceId),

                        anyString(),
@@ -342,6 +367,21 @@ public class IpSecServiceParameterizedTest { 
        IpSecService.UserRecord userRecord =

                mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid());

        assertEquals(0, userRecord.mTransformQuotaTracker.mCurrent);

        assertEquals(1, userRecord.mSpiQuotaTracker.mCurrent);



        mIpSecService.releaseSecurityParameterIndex(ipSecConfig.getSpiResourceId());

        // Verify that ipSecDeleteSa was not called when the SPI was released because the

        // ownedByTransform property should prevent it; (note, the called count is cumulative).

        verify(mMockNetd, times(1))

                .ipSecDeleteSecurityAssociation(

                        anyInt(),

                        anyString(),

                        anyString(),

                        anyInt(),

                        anyInt(),

                        anyInt());

        assertEquals(0, userRecord.mSpiQuotaTracker.mCurrent);



        try {

            userRecord.mTransformRecords.getRefcountedResourceOrThrow(

                    createTransformResp.resourceId);