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

Commit b358c677 authored by Lorenzo Colitti's avatar Lorenzo Colitti
Browse files

Fix parsing a NduseroptMessage inside a NetlinkMessage. 

The parsing code inside NduseroptMessage assumed that the message
started at the beginning of the buffer. Fix this, and also
restore the original endianness and limit of the buffer after
parsing.

Add tests for this, and make the existing tests a bit more
readable.

Bug: 153694684
Test: new unit tests
Change-Id: Id0b4dc18dd219b4d35846e161022a37c8de3e3bb
parent cc4bb60a

common/netlinkclient/src/android/net/netlink/NduseroptMessage.java

+8 −4
Original line number Diff line number Diff line
@@ -66,22 +66,23 @@ public class NduseroptMessage extends NetlinkMessage { 
        super(header);



        // The structure itself.

        buf.order(ByteOrder.nativeOrder());

        buf.order(ByteOrder.nativeOrder());  // Restored in the finally clause inside parse().

        final int start = buf.position();

        family = buf.get();

        buf.get();  // Skip 1 byte of padding.

        opts_len = Short.toUnsignedInt(buf.getShort());

        ifindex = buf.getInt();

        icmp_type = buf.get();

        icmp_code = buf.get();

        buf.order(ByteOrder.BIG_ENDIAN);

        buf.position(buf.position() + 6);  // Skip 6 bytes of padding.



        // The ND option.

        // Ensure we don't read past opts_len even if the option length is invalid.

        // Note that this check is not really necessary since if the option length is not valid,

        // this struct won't be very useful to the caller.

        buf.order(ByteOrder.BIG_ENDIAN);

        int oldLimit = buf.limit();

        buf.limit(STRUCT_SIZE + opts_len);

        buf.limit(start + STRUCT_SIZE + opts_len);

        try {

            option = NdOption.parse(buf);

        } finally {
@@ -89,7 +90,7 @@ public class NduseroptMessage extends NetlinkMessage { 
        }



        // The source address.

        int newPosition = STRUCT_SIZE + opts_len;

        int newPosition = start + STRUCT_SIZE + opts_len;

        if (newPosition >= buf.limit()) {

            throw new IllegalArgumentException("ND options extend past end of buffer");

        }
@@ -118,6 +119,7 @@ public class NduseroptMessage extends NetlinkMessage { 
     */

    public static NduseroptMessage parse(@NonNull StructNlMsgHdr header, @NonNull ByteBuffer buf) {

        if (buf == null || buf.remaining() < STRUCT_SIZE) return null;

        ByteOrder oldOrder = buf.order();

        try {

            return new NduseroptMessage(header, buf);

        } catch (IllegalArgumentException | UnknownHostException | BufferUnderflowException e) {
@@ -125,6 +127,8 @@ public class NduseroptMessage extends NetlinkMessage { 
            // Convention in this package is that null indicates that the option was truncated, so

            // callers must already handle it.

            return null;

        } finally {

            buf.order(oldOrder);

        }

    }

tests/unit/src/android/net/netlink/NduseroptMessageTest.java

+72 −15
Original line number Diff line number Diff line
@@ -16,6 +16,9 @@ 


package android.net.netlink;



import static android.net.InetAddresses.parseNumericAddress;

import static android.system.OsConstants.AF_INET6;



import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertNotNull;

import static org.junit.Assert.assertNull;
@@ -31,16 +34,16 @@ import libcore.util.HexEncoding; 
import org.junit.Test;

import org.junit.runner.RunWith;



import java.net.InetAddress;

import java.nio.ByteBuffer;

import java.nio.ByteOrder;



@RunWith(AndroidJUnit4.class)

@SmallTest

public class NduseroptMessageTest {



    // Pick ifindices that are high enough that they will "never" be an existing interface index,

    // and always be represented numerically in the address. That way, the test will never need to

    // determine the interface names corresponding to these indices. That simplifies the code and

    // makes the test more useful because determining interface names might require permissions.

    private static final byte ICMP_TYPE_RA = (byte) 134;



    private static final int IFINDEX1 = 15715755;

    private static final int IFINDEX2 = 1431655765;
@@ -59,8 +62,8 @@ public class NduseroptMessageTest { 
    // Length 20, NDUSEROPT_SRCADDR, fe80:2:3:4:5:6:7:8

    private static final String NLA_SRCADDR = "1400" + "0100" + "fe800002000300040005000600070008";



    private static final String SRCADDR1 = "fe80:2:3:4:5:6:7:8%" + IFINDEX1;

    private static final String SRCADDR2 = "fe80:2:3:4:5:6:7:8%" + IFINDEX2;

    private static final InetAddress SADDR1 = parseNumericAddress("fe80:2:3:4:5:6:7:8%" + IFINDEX1);

    private static final InetAddress SADDR2 = parseNumericAddress("fe80:2:3:4:5:6:7:8%" + IFINDEX2);



    private static final String MSG_EMPTY = HDR_EMPTY + NLA_SRCADDR;

    private static final String MSG_PREF64 = HDR_16BYTE + OPT_PREF64 + NLA_SRCADDR;
@@ -68,14 +71,68 @@ public class NduseroptMessageTest { 
    @Test

    public void testParsing() {

        NduseroptMessage msg = parseNduseroptMessage(toBuffer(MSG_EMPTY));

        assertMatches((byte) 10, 0, IFINDEX1, (byte) 134, (byte) 0, SRCADDR1, msg);

        assertMatches(AF_INET6, 0, IFINDEX1, ICMP_TYPE_RA, (byte) 0, SADDR1, msg);

        assertNull(msg.option);



        msg = parseNduseroptMessage(toBuffer(MSG_PREF64));

        assertMatches((byte) 10, 16, IFINDEX2, (byte) 134, (byte) 0, SRCADDR2, msg);

        assertMatches(AF_INET6, 16, IFINDEX2, ICMP_TYPE_RA, (byte) 0, SADDR2, msg);

        assertPref64Option("2001:db8:3:4:5:6::/96", msg.option);

    }



    @Test

    public void testParseWithinNetlinkMessage() throws Exception {

        // A NduseroptMessage inside a netlink message. Ensure that it parses the same way both by

        // parsing the netlink message via NetlinkMessage.parse() and by parsing the option itself

        // with NduseroptMessage.parse().

        final String hexBytes =

                "44000000440000000000000000000000"             // len=68, RTM_NEWNDUSEROPT

                + "0A0010001E0000008600000000000000"           // IPv6, opt_bytes=16, ifindex=30, RA

                + "260202580064FF9B0000000000000000"           // pref64, prefix=64:ff9b::/96, 600

                + "14000100FE800000000000000250B6FFFEB7C499";  // srcaddr=fe80::250:b6ff:feb7:c499



        ByteBuffer buf = toBuffer(hexBytes);

        assertEquals(68, buf.limit());

        buf.order(ByteOrder.nativeOrder());



        NetlinkMessage nlMsg = NetlinkMessage.parse(buf);

        assertNotNull(nlMsg);

        assertTrue(nlMsg instanceof NduseroptMessage);



        NduseroptMessage msg = (NduseroptMessage) nlMsg;

        InetAddress srcaddr = InetAddress.getByName("fe80::250:b6ff:feb7:c499%30");

        assertMatches(AF_INET6, 16, 30, ICMP_TYPE_RA, (byte) 0, srcaddr, msg);

        assertPref64Option("64:ff9b::/96", msg.option);



        final String hexBytesWithoutHeader = hexBytes.substring(StructNlMsgHdr.STRUCT_SIZE * 2);

        ByteBuffer bufWithoutHeader = toBuffer(hexBytesWithoutHeader);

        assertEquals(52, bufWithoutHeader.limit());

        msg = parseNduseroptMessage(bufWithoutHeader);

        assertMatches(AF_INET6, 16, 30, ICMP_TYPE_RA, (byte) 0, srcaddr, msg);

        assertPref64Option("64:ff9b::/96", msg.option);

    }



    @Test

    public void testParseUnknownOptionWithinNetlinkMessage() throws Exception {

        final String hexBytes =

                "4C0000004400000000000000000000000"

                + "A0018001E0000008600000000000000"

                + "1903000000001770FD123456789000000000000000000001"  // RDNSS option

                + "14000100FE800000000000000250B6FFFEB7C499";



        ByteBuffer buf = toBuffer(hexBytes);

        assertEquals(76, buf.limit());

        buf.order(ByteOrder.nativeOrder());



        NetlinkMessage nlMsg = NetlinkMessage.parse(buf);

        assertNotNull(nlMsg);

        assertTrue(nlMsg instanceof NduseroptMessage);



        NduseroptMessage msg = (NduseroptMessage) nlMsg;

        InetAddress srcaddr = InetAddress.getByName("fe80::250:b6ff:feb7:c499%30");

        assertMatches(AF_INET6, 24, 30, ICMP_TYPE_RA, (byte) 0, srcaddr, msg);

        assertEquals(NdOption.UNKNOWN, msg.option);

    }



    @Test

    public void testUnknownOption() {

        ByteBuffer buf = toBuffer(MSG_PREF64);
@@ -85,7 +142,7 @@ public class NduseroptMessageTest { 
        buf.put(optionStart, (byte) 42);



        NduseroptMessage msg = parseNduseroptMessage(buf);

        assertMatches((byte) 10, 16, IFINDEX2, (byte) 134, (byte) 0, SRCADDR2, msg);

        assertMatches(AF_INET6, 16, IFINDEX2, ICMP_TYPE_RA, (byte) 0, SADDR2, msg);

        assertEquals(NdOption.UNKNOWN, msg.option);



        buf.flip();
@@ -93,7 +150,7 @@ public class NduseroptMessageTest { 
        buf.put(optionStart, (byte) 38);



        msg = parseNduseroptMessage(buf);

        assertMatches((byte) 10, 16, IFINDEX2, (byte) 134, (byte) 0, SRCADDR2, msg);

        assertMatches(AF_INET6, 16, IFINDEX2, ICMP_TYPE_RA, (byte) 0, SADDR2, msg);

        assertPref64Option("2001:db8:3:4:5:6::/96", msg.option);

    }
@@ -105,7 +162,7 @@ public class NduseroptMessageTest { 
        ByteBuffer buf = toBuffer(hexString);

        assertEquals(52, buf.limit());

        NduseroptMessage msg = parseNduseroptMessage(buf);

        assertMatches((byte) 10, 16, IFINDEX2, (byte) 134, (byte) 0, SRCADDR2, msg);

        assertMatches(AF_INET6, 16, IFINDEX2, ICMP_TYPE_RA, (byte) 0, SADDR2, msg);

        assertNull(msg.option);

    }
@@ -117,7 +174,7 @@ public class NduseroptMessageTest { 
        ByteBuffer buf = toBuffer(hexString);

        assertEquals(52, buf.limit());

        NduseroptMessage msg = parseNduseroptMessage(buf);

        assertMatches((byte) 10, 16, IFINDEX2, (byte) 134, (byte) 0, SRCADDR2, msg);

        assertMatches(AF_INET6, 16, IFINDEX2, ICMP_TYPE_RA, (byte) 0, SADDR2, msg);

        assertNull(msg.option);

    }
@@ -168,15 +225,15 @@ public class NduseroptMessageTest { 
        return ByteBuffer.wrap(HexEncoding.decode(hexString));

    }



    private void assertMatches(byte family, int optsLen, int ifindex, byte icmpType,

            byte icmpCode, String srcaddr, NduseroptMessage msg) {

    private void assertMatches(int family, int optsLen, int ifindex, byte icmpType,

            byte icmpCode, InetAddress srcaddr, NduseroptMessage msg) {

        assertNotNull(msg);

        assertEquals(family, msg.family);

        assertEquals(ifindex, msg.ifindex);

        assertEquals(optsLen, msg.opts_len);

        assertEquals(icmpType, msg.icmp_type);

        assertEquals(icmpCode, msg.icmp_code);

        assertEquals(srcaddr, msg.srcaddr.getHostAddress());

        assertEquals(srcaddr, msg.srcaddr);

    }



    private void assertPref64Option(String prefix, NdOption opt) {