ASN.1 Deserialize fails with SequenceOf strings

[SteveSyfuhs]

Let me preface this with the statement that I fully understand this is an internal-only API (for now at least). Nevertheless, this is a fairly important test case.

Suppose an ASN.1 of

kdc-req ::= SEQUENCE {
...
    sname [3] PrincipalName OPTIONAL,
...
}

PrincipalName ::= SEQUENCE {
    name-type   [0] Int32,
    name-string [1] SEQUENCE OF KerberosString
}

KerberosString  ::= GeneralString (IA5String)

There are two issues here.

1. GeneralString is not actually supported. You cannot futz your way through by decorating the struct field to think it's an IA5String. I had to add a new attribute and modified the deserializer to treat it the same as IA5String.
2. SequenceOf GeneralString/IA5String cannot parse correctly when there are multiple values in the sequence.

It decodes the array as a struct and throws because there's extra data. There's no way it can move on to the next element in the sequence. Conversely, you can't coerce the deserializer to operate against string[] because it can't be a SequenceOf and GeneralizedString/IA5String at the same time.

Sample

This will throw when it finishes reading the first string of sname. It'll complete the pass over the struct fields, but there's more data in the sequence.

I think it would be helpful if you could decorate a SequenceOf as a specialized type, e.g. GeneralizedString.

var asReq = "3081d3a103020105a20302010aa31530133011a10402020080a20904073005a0030101ffa481af30" 
+ "81aca00703050040810010a1153013a003020101a10c300a1b087374737966756873a2091b07524" 
+ "5444d4f4e44a31c301aa003020102a11330111b066b72627467741b075245444d4f4e44a511180f3" 
+ "2303337303931333032343830355aa611180f32303337303931333032343830355aa70502036875" 
+ "0da81530130201120201110201170201180202ff79020103a91d301b3019a003020114a11204104" 
+ "d494e494e542d364a49454842472020";

var req = AsnSerializer.Deserialize<KdcReq>(StringToByteArray(asReq), AsnEncodingRules.DER);

[StructLayout(LayoutKind.Sequential)]
public struct KdcReq
{
    /*
    AS-REQ          ::= [APPLICATION 10] KDC-REQ

    TGS-REQ         ::= [APPLICATION 12] KDC-REQ

    KDC-REQ         ::= SEQUENCE {
        -- NOTE: first tag is [1], not [0]
        pvno            [1] INTEGER (5) ,
        msg-type        [2] INTEGER (10 -- AS -- | 12 -- TGS --),
        padata          [3] SEQUENCE OF PA-DATA OPTIONAL
                            -- NOTE: not empty --,
        req-body        [4] KDC-REQ-BODY
    }*/

    [ExpectedTag(1, ExplicitTag = true)]
    public int pvno;

    [ExpectedTag(2, ExplicitTag = true)]
    public int msgType;

    [ExpectedTag(3, ExplicitTag = true)]
    public PaData[] PaData;

    [ExpectedTag(4, ExplicitTag = true)]
    public KdcReqBody reqBody;
}

[StructLayout(LayoutKind.Sequential)]
public struct PaData
{
    // padata-type[1] Int32,
    // padata-value[2] OCTET STRING -- might be encoded AP-REQ

    [ExpectedTag(1, ExplicitTag = true)]
    public int paDataType;

    [ExpectedTag(2, ExplicitTag = true), OctetString]
    public ReadOnlyMemory<byte> paDataValue;
}

[StructLayout(LayoutKind.Sequential)]
public struct KdcReqBody
{
    /*
    kdc-options             [0] KDCOptions,
    cname                   [1] PrincipalName OPTIONAL
                                -- Used only in AS-REQ --,
    realm                   [2] Realm
                                -- Server's realm
                                -- Also client's in AS-REQ --,
    sname                   [3] PrincipalName OPTIONAL,
    from                    [4] KerberosTime OPTIONAL,
    till                    [5] KerberosTime,
    rtime                   [6] KerberosTime OPTIONAL,
    nonce                   [7] UInt32,
    etype                   [8] SEQUENCE OF Int32 -- EncryptionType
                                -- in preference order --,
    addresses               [9] HostAddresses OPTIONAL,
    enc-authorization-data  [10] EncryptedData OPTIONAL
                                -- AuthorizationData --,
    additional-tickets      [11] SEQUENCE OF Ticket OPTIONAL
                                    -- NOTE: not empty         
    */
    [ExpectedTag(0, ExplicitTag = true), BitString]
    public ReadOnlyMemory<byte> kdcOptions;

    [ExpectedTag(1, ExplicitTag = true), OptionalValue]
    public PrincipalName? cname;

    [ExpectedTag(2)]
    public KerberosString realm;

    [ExpectedTag(3, ExplicitTag = true), OptionalValue]
    public PrincipalName? sname;

    [ExpectedTag(4, ExplicitTag = true), GeneralizedTime, OptionalValue]
    public DateTimeOffset? from;

    [ExpectedTag(5, ExplicitTag = true), GeneralizedTime]
    public DateTimeOffset till;

    [ExpectedTag(6, ExplicitTag = true), GeneralizedTime, OptionalValue]
    public DateTimeOffset? rtime;

    [ExpectedTag(7, ExplicitTag = true)]
    public uint nonce;

    [ExpectedTag(8, ExplicitTag = true)]
    public int[] etype;

    [ExpectedTag(9, ExplicitTag = true)]
    public HostAddress[] addresses;

    [ExpectedTag(10, ExplicitTag = true), OptionalValue]
    public EncryptedData? encAuthorizationData;

    [ExpectedTag(11, ExplicitTag = true), OptionalValue]
    public Ticket[] additionalTickets;
}

[StructLayout(LayoutKind.Sequential)]
public struct PrincipalName
{
    // name-type[0] Int32,
    // name-string[1] SEQUENCE OF KerberosString

    [ExpectedTag(0, ExplicitTag = true)]
    public int nameType;

    [ExpectedTag(1)]
    public KerberosString[] nameString;

    // this does not work either
    // [ExpectedTag(1), GeneralizedString]
    // public string[] nameString;
}

[StructLayout(LayoutKind.Sequential)]
public struct KerberosString
{
    [GeneralizedString]
    public string name;
}

[StructLayout(LayoutKind.Sequential)]
public struct HostAddress
{
    // addr-type[0] Int32,
    // address[1] OCTET STRING

    [ExpectedTag(0, ExplicitTag = true)]
    public int addrType;

    [ExpectedTag(1, ExplicitTag = true), OctetString]
    public ReadOnlyMemory<byte> address;
}

[StructLayout(LayoutKind.Sequential)]
public struct EncryptedData
{
    // EncryptedData::= SEQUENCE {
    //     etype[0] Int32 -- EncryptionType --,
    //     kvno[1] UInt32 OPTIONAL,
    //     cipher[2] OCTET STRING -- ciphertext
    // }

    [ExpectedTag(0, ExplicitTag = true)]
    public int etype;

    [ExpectedTag(1, ExplicitTag = true), OptionalValue]
    public int kvno;

    [ExpectedTag(2, ExplicitTag = true), OctetString]
    public ReadOnlyMemory<byte> cipher;
}

[StructLayout(LayoutKind.Sequential)]
public struct AuthorizationData
{
    [ExpectedTag(0, ExplicitTag = true)]
    public int adType;

    [ExpectedTag(1, ExplicitTag = true), OctetString]
    public ReadOnlyMemory<byte> adData;
}

[StructLayout(LayoutKind.Sequential)]
public struct Ticket { }

[bartonjs]

The problem with GeneralString is that it (theoretically) supports changing out the codepage as a portion of the payload. At least, that's how I've interpreted it, because I haven't been able to really follow the series of jumps to understand what the bytes actually look like for that. So adding it means either adding it properly, or having a surprise exception when you get a payload that actually used GeneralString for the properties it has over IA5String (codepage switching).

When the deserializer gets in the way the "easiest" thing to do is to toss in an [AnyValue] Memory<byte> _whatever and then read it later with the AsnReader.

I can't think of a reason why string[] would deserialize incorrectly. That's certainly easier to look at than making GeneralString actually work.

[SteveSyfuhs]

So adding it means either adding it properly, or having a surprise exception when you get a payload that actually used GeneralString

Obviously adding it properly is preferred, but since that's a non-trivial undertaking, would it be possible to add a hook where callers can just do the dirty work? Or alternately, just a way to tell it to automatically map types from one to another? Providing an option to shoot yourself in the foot sucks, but that's at least something I'd be willing to accept. :)

I tried fiddling with the string[] bit. It looks like you can't tell it to have two AsnTypeAttribute attributes because of the length check in GetFieldInfo. Removing the check doesn't help because it thinks string is ambiguous.

[bartonjs]

Without figuring out how to float the disambiguator through a sequence-of/set-of, you can work around it with an implicit choice.

byte[] inputData = (
    "3080" +
      "3080" +
        "0C0455544638" +
        "0C0455544639" +
        "0C045554463A" +
        "0C045554463B" +
      "0000" +
      "3080" +
        "1603494135" +
        "1603494136" +
      "0000" +
    "0000").HexToByteArray();

StringArrays arrays = AsnSerializer.Deserialize<StringArrays>(inputData, AsnEncodingRules.BER);
Assert.Equal(4, arrays.Utf8Strings.Length);
Assert.Equal(2, arrays.IA5Strings.Length);

Console.WriteLine(string.Join(", ", arrays.Utf8Strings.Select(u8S => u8S.Utf8String)));
Console.WriteLine(string.Join(", ", arrays.IA5Strings.Select(ia5S => ia5S.IA5String)));

which is powered by

    [StructLayout(LayoutKind.Sequential)]
    public struct StringArrays
    {
        [SequenceOf]
        public Utf8Choice[] Utf8Strings;

        [SequenceOf]
        public IA5Choice[] IA5Strings;
    }

    [Choice]
    public struct Utf8Choice
    {
        [UTF8String]
        public string Utf8String;
    }

    [Choice]
    public struct IA5Choice
    {
        [IA5String]
        public string IA5String;
    }

That still doesn't help you with GeneralString. The best I can do for that without supporting it is

    [Choice]
    public struct GSChoice
    {
        [AnyValue]
        [ExpectedTag(TagClass.Universal, (int)UniversalTagNumber.GeneralString)]
        public ReadOnlyMemory<byte>? GeneralString;
    }

        {
            ...
            Console.WriteLine(string.Join(", ", arrays.GeneralStrings.Select(genS => ReadGeneralString(genS))));
        }

        private static string ReadGeneralString(GSChoice generalStringChoice)
        {
            ReadOnlyMemory<byte> generalStringValue = generalStringChoice.GeneralString.Value;
            AsnReader reader = new AsnReader(generalStringValue, AsnEncodingRules.BER);

            if (reader.TryGetPrimitiveCharacterStringBytes(UniversalTagNumber.GeneralString,
                out ReadOnlyMemory<byte> payload))
            {
                // I don't have netcoreapp21 in scope, so I can't use the new overloads here.
                return Text.Encoding.ASCII.GetString(payload.ToArray());
            }

            byte[] buf = ArrayPool<byte>.Shared.Rent(generalStringValue.Length);
            if (!reader.TryCopyCharacterStringBytes(UniversalTagNumber.GeneralString, buf, out int bytesWritten))
            {
                throw new InvalidOperationException();
            }

            Span<byte> payloadSpan = buf.AsSpan(0, bytesWritten);
            // Again, don't have the new overloads in scope
            string str = Text.Encoding.ASCII.GetString(payloadSpan.ToArray());
            payloadSpan.Clear();
            ArrayPool<byte>.Shared.Return(buf);
            return str;
        }

It's ugly, but it works.

[SteveSyfuhs]

I can confirm this works. I tried by adding back in the cheat attribute, and the GSChoice => ReadGeneralizedString method. I can also confirm serialize works with this, but WriteGeneralizedString is equally ugly.

[bartonjs]

FWIW, we've moved off of using AsnSerializer and use a code generator now. The generator does a better job with things like SEQUENCE OF IA5String.

[SteveSyfuhs]

Oh interesting! Can you point me to a sample? I'd love to try this out.

[bartonjs]

https://github.com/dotnet/corefx/blob/6ffca612323448f1488a6c63b86b69611b4286d9/src/Common/src/System/Security/Cryptography/Asn1/AsnXml.targets#L1 (and the asn.xsl and asn.xsd files in the same directory)

https://github.com/dotnet/corefx/pull/31885 was the PR that started the changeover (a little bit has changed in the generator since).

A snippet from my giant test

<?xml version="1.0" encoding="utf-8" ?>
<asn:Sequence
  xmlns:asn="http://schemas.dot.net/asnxml/201808/"
  name="AllTheThingsAsn"
  namespace="System.Security.Cryptography.Asn1">
...
  <asn:SequenceOf name="SequenceOfUTF8StringImplicitDefault" implicitTag="2" defaultDerInit="0xA2, 0x04, 0x0C, 0x02, 0x34, 0x32">
    <asn:UTF8String/>
  </asn:SequenceOf>
...
</asn:Sequence>