Skip to content

Imdsv2: Generate CSR and execute CSR request #5427

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 21 commits into
base: rginsburg/msiv2_feature_branch
Choose a base branch
from
Open

Imdsv2: Generate CSR and execute CSR request #5427

wants to merge 21 commits into from

Conversation

Robbie-Microsoft
Copy link
Contributor

@Robbie-Microsoft Robbie-Microsoft commented Aug 6, 2025 •
edited
Loading

.NET native CSR generation via RSA and ASN.1 Encoding. Tested with net462, net472, net8.0, netstandard2.0

gladjohn
gladjohn reviewed Aug 6, 2025
View reviewed changes
gladjohn
gladjohn reviewed Aug 6, 2025
View reviewed changes
@Robbie-Microsoft Robbie-Microsoft marked this pull request as ready for review August 6, 2025 22:07
@Robbie-Microsoft Robbie-Microsoft requested a review from a team as a code owner August 6, 2025 22:07
gladjohn
gladjohn reviewed Aug 7, 2025
View reviewed changes
bgavrilMS
bgavrilMS reviewed Aug 11, 2025
View reviewed changes
bgavrilMS
bgavrilMS reviewed Aug 11, 2025
View reviewed changes
bartonjs
bartonjs reviewed Aug 12, 2025
View reviewed changes
src/client/Microsoft.Identity.Client/ManagedIdentity/Csr.cs Outdated
Comment on lines 255 to 413
}

/// <summary>
/// Encodes an ASN.1 NULL.
/// </summary>
private static byte[] EncodeAsn1Null()
{
return new byte[] { 0x05, 0x00 };
}

/// <summary>
/// Encodes an ASN.1 OBJECT IDENTIFIER.
/// </summary>
private static byte[] EncodeAsn1ObjectIdentifier(int[] oid)
{
if (oid == null || oid.Length < 2)
throw new ArgumentException("OID must have at least 2 components");

var bytes = new System.Collections.Generic.List<byte>();

// First two components are encoded as (first * 40 + second)
bytes.AddRange(EncodeOidComponent(oid[0] * 40 + oid[1]));

// Remaining components
for (int i = 2; i < oid.Length; i++)
{
bytes.AddRange(EncodeOidComponent(oid[i]));
}

return EncodeAsn1Tag(0x06, bytes.ToArray());
}

/// <summary>
/// Encodes an ASN.1 context-specific tag.
/// </summary>
private static byte[] EncodeAsn1ContextSpecific(int tagNumber, byte[] content)
{
byte tag = (byte)(0xA0 | tagNumber); // Context-specific, constructed
return EncodeAsn1Tag(tag, content);
}

/// <summary>
/// Encodes an ASN.1 tag with length and content.
/// </summary>
private static byte[] EncodeAsn1Tag(byte tag, byte[] content)
{
byte[] lengthBytes = EncodeAsn1Length(content.Length);
byte[] result = new byte[1 + lengthBytes.Length + content.Length];
result[0] = tag;
Array.Copy(lengthBytes, 0, result, 1, lengthBytes.Length);
Array.Copy(content, 0, result, 1 + lengthBytes.Length, content.Length);
return result;
}

/// <summary>
/// Encodes ASN.1 length field.
/// </summary>
private static byte[] EncodeAsn1Length(int length)
{
if (length < 0x80)
{
return new byte[] { (byte)length };
}

var lengthBytes = new System.Collections.Generic.List<byte>();
int temp = length;
while (temp > 0)
{
lengthBytes.Insert(0, (byte)(temp & 0xFF));
temp >>= 8;
}

byte[] result = new byte[lengthBytes.Count + 1];
result[0] = (byte)(0x80 | lengthBytes.Count);
lengthBytes.CopyTo(result, 1);
return result;
}

/// <summary>
/// Encodes a single OID component using variable-length encoding.
/// </summary>
private static byte[] EncodeOidComponent(int value)
{
if (value == 0)
return new byte[] { 0x00 };

var bytes = new System.Collections.Generic.List<byte>();
int temp = value;

bytes.Insert(0, (byte)(temp & 0x7F));
temp >>= 7;

while (temp > 0)
{
bytes.Insert(0, (byte)((temp & 0x7F) | 0x80));
temp >>= 7;
}

return bytes.ToArray();
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these seem like they can/should use System.Formats.Asn1.

Copy link
Contributor Author

@Robbie-Microsoft Robbie-Microsoft Aug 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this is not available in .NET Framework 462/472 and .NET Standard 2.0. Do you think it's worth it to conditionally use System.Formats.Asn1 for only 8.0? Or would it be clearer to use the same code for all 4 versions (how it is now).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's available, you just need to reference the System.Formats.Asn1 NuGet package.

bgavrilMS
bgavrilMS reviewed Aug 18, 2025
View reviewed changes
src/client/Microsoft.Identity.Client/ManagedIdentity/CsrMetadata.cs
@@ -57,13 +57,12 @@ public CsrMetadata() { }
/// Validates a JSON decoded CsrMetadata instance.
/// </summary>
/// <param name="csrMetadata">The CsrMetadata object.</param>
/// <returns>false if any field is null.</returns>
/// <returns>false if any required field is null. Note: Vmid is required, Vmssid is optional.</returns>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you keep the naming consistent? Is this a CsrResponse?

bgavrilMS
bgavrilMS reviewed Aug 18, 2025
View reviewed changes
tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/CsrValidator.cs
/// <summary>
/// Test helper to expose CsrValidator methods for testing malformed PEM.
/// </summary>
internal static class TestCsrValidator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't read any of these tests. At least add a commen on what this does.

bgavrilMS
bgavrilMS reviewed Aug 18, 2025
View reviewed changes
tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/ImdsV2Tests.cs
Assert.IsFalse(string.IsNullOrWhiteSpace(csrRequest.Pem));

// Validate the CSR contents - this should handle null/empty VMSSID gracefully
CsrValidator.ValidateCsrContent(csrRequest.Pem, TestConstants.ClientId, TestConstants.TenantId, cuid);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not clear what exactly you are testing here. Ideally we would test that the CSR is valid and that it contains all the necessary data (e.g. oid=clientId etc.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(or maybe you are doing this, but I am not able to tell)

bgavrilMS
bgavrilMS reviewed Aug 18, 2025
View reviewed changes
tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/ImdsV2Tests.cs
Assert.IsFalse(string.IsNullOrWhiteSpace(csrRequest.Pem));

// Validate the CSR contents - this should handle null/empty VMSSID gracefully
CsrValidator.ValidateCsrContent(csrRequest.Pem, TestConstants.ClientId, TestConstants.TenantId, cuid);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a test idea, you can have a test that runs on net8 only and which uses the rutnime to create the CSR and then compare it with yours. Just an idea, not sure how useful it would be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bgavrilMS bgavrilMS bgavrilMS left review comments

@neha-bhargava neha-bhargava Awaiting requested review from neha-bhargava

@bartonjs bartonjs Awaiting requested review from bartonjs

@gladjohn gladjohn Awaiting requested review from gladjohn

Assignees
No one assigned
Labels
Feature Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Robbie-Microsoft @bartonjs @bgavrilMS @neha-bhargava @gladjohn