Skip to content
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.

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

feat: add versionless key identifier support #181

Merged
merged 16 commits into from
May 29, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ public void Constructor_Invalid()
}

[Fact]
public async void RunAsync_NoSecertsGetPermission()
public async Task RunAsync_NoSecertsGetPermission()
{
// Arrange
var keyId = "https://testvault.vault.azure.net/keys/testkey/123";
Expand All @@ -196,7 +196,7 @@ public async void RunAsync_NoSecertsGetPermission()
}

[Fact]
public async void RunAsync_OtherRequestFailedException()
public async Task RunAsync_OtherRequestFailedException()
{
// Arrange
var keyId = "https://testvault.vault.azure.net/keys/testkey/123";
Expand All @@ -223,7 +223,7 @@ public async void RunAsync_OtherRequestFailedException()
}

[Fact]
public async void RunAsync_SelfSignedWithCaCerts()
public async Task RunAsync_SelfSignedWithCaCerts()
{
// Arrange
var keyId = "https://testvault.vault.azure.net/keys/testkey/123";
Expand Down
67 changes: 57 additions & 10 deletions Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,21 +47,48 @@ public void TestConstructorWithKeyVaultUrlNameVersion()
Assert.Equal($"{keyVaultUrl}/keys/{name}/{version}", keyVaultClient.KeyId);
}

[Fact]
public void TestConstructorWithVersionlessKey()
{
string keyVaultUrl = "https://myvault.vault.azure.net";
string name = "my-key";

KeyVaultClient keyVaultClient = new KeyVaultClient(keyVaultUrl, name, null, Credentials.GetCredentials(defaultCredentialType));
Assert.Equal(name, keyVaultClient.Name);
Assert.Null(keyVaultClient.Version);
Assert.Equal($"{keyVaultUrl}/keys/{name}", keyVaultClient.KeyId);

keyVaultClient = new KeyVaultClient($"{keyVaultUrl}/keys/{name}", Credentials.GetCredentials(defaultCredentialType));
Assert.Equal(name, keyVaultClient.Name);
Assert.Null(keyVaultClient.Version);
Assert.Equal($"{keyVaultUrl}/keys/{name}", keyVaultClient.KeyId);
}

[Theory]
[InlineData("")]
[InlineData("https://myvault.vault.azure.net/invalid/my-key/123")]
[InlineData("https://myvault.vault.azure.net/keys/my-key")]
[InlineData("https://myvault.vault.azure.net/keys/my-key/")]
[InlineData("http://myvault.vault.azure.net/keys/my-key/123")]
[InlineData("https://myvault.vault.azure.net/keys")]
[InlineData("https://myvault.vault.azure.net/invalid/my-key/123/1234")]
public void TestConstructorWithInvalidKeyId(string invalidKeyId)
{
Assert.Throws<ValidationException>(() => new KeyVaultClient(invalidKeyId, Credentials.GetCredentials(defaultCredentialType)));
}

[Theory]
[InlineData("", "", "")]
[InlineData("https://myvault.vault.azure.net", "", "")]
[InlineData("https://myvault.vault.azure.net", "my-key", "")]
public void TestConstructorWithInvalidArguments(string keyVaultUrl, string name, string? version)
{
Assert.Throws<ValidationException>(() => new KeyVaultClient(keyVaultUrl, name, version, Credentials.GetCredentials(defaultCredentialType)));
}

[Theory]
[InlineData("")]
public void TestConstructorWithEmptyKeyId(string invalidKeyId)
{
Assert.Throws<ArgumentNullException>(() => new KeyVaultClient(invalidKeyId, Credentials.GetCredentials(defaultCredentialType)));
Assert.Throws<ValidationException>(() => new KeyVaultClient(invalidKeyId, Credentials.GetCredentials(defaultCredentialType)));
}

private class TestableKeyVaultClient : KeyVaultClient
Expand All @@ -72,7 +99,7 @@ public TestableKeyVaultClient(string keyVaultUrl, string name, string version, C
this._cryptoClient = new Lazy<CryptographyClient>(() => cryptoClient);
}

public TestableKeyVaultClient(string keyVaultUrl, string name, string version, CertificateClient certificateClient, TokenCredential credenital)
public TestableKeyVaultClient(string keyVaultUrl, string name, string? version, CertificateClient certificateClient, TokenCredential credenital)
: base(keyVaultUrl, name, version, credenital)
{
this._certificateClient = new Lazy<CertificateClient>(() => certificateClient);
Expand Down Expand Up @@ -103,6 +130,15 @@ private TestableKeyVaultClient CreateMockedKeyVaultClient(KeyVaultCertificate ce
return new TestableKeyVaultClient("https://fake.vault.azure.net", "fake-certificate", "123", mockCertificateClient.Object, Credentials.GetCredentials(defaultCredentialType));
}

private TestableKeyVaultClient CreateMockedKeyVaultClient(KeyVaultCertificateWithPolicy certWithPolicy)
{
var mockCertificateClient = new Mock<CertificateClient>(new Uri("https://fake.vault.azure.net/certificates/fake-certificate/123"), new Mock<TokenCredential>().Object);
mockCertificateClient.Setup(c => c.GetCertificateAsync(It.IsAny<string>(), It.IsAny<CancellationToken>()))
.ReturnsAsync(Response.FromValue(certWithPolicy, new Mock<Response>().Object));

return new TestableKeyVaultClient("https://fake.vault.azure.net", "fake-certificate", null, mockCertificateClient.Object, Credentials.GetCredentials(defaultCredentialType));
}

private TestableKeyVaultClient CreateMockedKeyVaultClient(KeyVaultSecret secret)
{
var mockSecretClient = new Mock<SecretClient>(new Uri("https://fake.vault.azure.net/secrets/fake-secret/123"), new Mock<TokenCredential>().Object);
Expand Down Expand Up @@ -158,11 +194,6 @@ public async Task TestSignAsyncThrowsExceptionOnInvalidAlgorithm()
public async Task GetCertificateAsync_ReturnsCertificate()
{
var testCertificate = new X509Certificate2(Path.Combine(Directory.GetCurrentDirectory(), "TestData", "rsa_2048.crt"));
var signResult = CryptographyModelFactory.SignResult(
keyId: "https://fake.vault.azure.net/keys/fake-key/123",
signature: new byte[] { 1, 2, 3 },
algorithm: SignatureAlgorithm.RS384);

var keyVaultCertificate = CertificateModelFactory.KeyVaultCertificate(
properties: CertificateModelFactory.CertificateProperties(version: "123"),
cer: testCertificate.RawData);
Expand All @@ -176,6 +207,22 @@ public async Task GetCertificateAsync_ReturnsCertificate()
Assert.Equal(testCertificate.RawData, certificate.RawData);
}

[Fact]
public async Task GetVersionlessCertificateAsync_ReturnCertificate()
{
var testCertificate = new X509Certificate2(Path.Combine(Directory.GetCurrentDirectory(), "TestData", "rsa_2048.crt"));
var keyVaultCertificateWithPolicy = CertificateModelFactory.KeyVaultCertificateWithPolicy(
properties: CertificateModelFactory.CertificateProperties(version: "123"),
cer: testCertificate.RawData);

var keyVaultClient = CreateMockedKeyVaultClient(keyVaultCertificateWithPolicy);
var certificate = await keyVaultClient.GetCertificateAsync();

Assert.NotNull(certificate);
Assert.IsType<X509Certificate2>(certificate);
Assert.Equal(testCertificate.RawData, certificate.RawData);
}

[Fact]
public async Task GetCertificateAsyncThrowValidationException()
{
Expand All @@ -191,7 +238,7 @@ public async Task GetCertificateAsyncThrowValidationException()

var keyVaultClient = CreateMockedKeyVaultClient(keyVaultCertificate);

await Assert.ThrowsAsync<ValidationException>(async () => await keyVaultClient.GetCertificateAsync());
await Assert.ThrowsAsync<PluginException>(keyVaultClient.GetCertificateAsync);
}

[Fact]
Expand Down
3 changes: 1 addition & 2 deletions Notation.Plugin.AzureKeyVault/Command/DescribeKey.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ public class DescribeKey : IPluginCommand
{
private DescribeKeyRequest _request;
private IKeyVaultClient _keyVaultClient;
private const string invalidInputError = "Invalid input. The valid input format is '{\"contractVersion\":\"1.0\",\"keyId\":\"https://<vaultname>.vault.azure.net/<keys|certificate>/<name>/<version>\"}'";

/// <summary>
/// Constructor to create DescribeKey object from JSON string.
Expand All @@ -23,7 +22,7 @@ public DescribeKey(string inputJson)
var request = JsonSerializer.Deserialize(inputJson, DescribeKeyRequestContext.Default.DescribeKeyRequest);
if (request == null)
{
throw new ValidationException(invalidInputError);
throw new ValidationException("Failed to parse the request in JSON format. Please contact the Notation developer to resolve the issue.");
JeyJeyGao marked this conversation as resolved.
Show resolved Hide resolved
}
this._request = request;
this._keyVaultClient = new KeyVaultClient(
Expand Down
2 changes: 1 addition & 1 deletion Notation.Plugin.AzureKeyVault/Command/GenerateSignature.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public GenerateSignature(string inputJson)
var request = JsonSerializer.Deserialize(inputJson, GenerateSignatureRequestContext.Default.GenerateSignatureRequest);
if (request == null)
{
throw new ValidationException("Invalid input");
throw new ValidationException("Failed to parse the request in JSON format. Please contact the Notation developer to resolve the issue.");
JeyJeyGao marked this conversation as resolved.
Show resolved Hide resolved
}
this._request = request;
this._keyVaultClient = new KeyVaultClient(
Expand Down
85 changes: 51 additions & 34 deletions Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public class KeyVaultClient : IKeyVaultClient
/// <summary>
/// A helper record to store KeyVault metadata.
/// </summary>
private record KeyVaultMetadata(string KeyVaultUrl, string Name, string Version);
private record KeyVaultMetadata(string KeyVaultUrl, string Name, string? Version);

// Certificate client (lazy initialization)
// Protected for unit test
Expand All @@ -43,50 +43,52 @@ private record KeyVaultMetadata(string KeyVaultUrl, string Name, string Version)
protected Lazy<CryptographyClient> _cryptoClient;
// Secret client (lazy initialization)
protected Lazy<SecretClient> _secretClient;
// Error message for invalid input
private const string INVALID_INPUT_ERROR_MSG = "Invalid input. The valid input format is '{\"contractVersion\":\"1.0\",\"keyId\":\"https://<vaultname>.vault.azure.net/<keys|certificate>/<name>/<version>\"}'";

// Key name or certificate name
private string _name;
// Key version or certificate version
private string _version;
private string? _version;
// Key identifier (e.g. https://<vaultname>.vault.azure.net/keys/<name>/<version>)
private string _keyId;

// Internal getters for unit test
internal string Name => _name;
internal string Version => _version;
internal string? Version => _version;
internal string KeyId => _keyId;

/// <summary>
/// Constructor to create AzureKeyVault object from keyVaultUrl, name
/// and version.
/// </summary>
public KeyVaultClient(string keyVaultUrl, string name, string version, TokenCredential credential)
public KeyVaultClient(string keyVaultUrl, string name, string? version, TokenCredential credential)
{
if (string.IsNullOrEmpty(keyVaultUrl))
{
throw new ArgumentNullException(nameof(keyVaultUrl), "KeyVaultUrl must not be null or empty");
throw new ValidationException("Key vault URL must not be null or empty");
}

if (string.IsNullOrEmpty(name))
{
throw new ArgumentNullException(nameof(name), "KeyName must not be null or empty");
throw new ValidationException("Key name must not be null or empty");
}

if (string.IsNullOrEmpty(version))
if (version != null && version == string.Empty)
Two-Hearts marked this conversation as resolved.
Show resolved Hide resolved
{
throw new ArgumentNullException(nameof(version), "KeyVersion must not be null or empty");
throw new ValidationException("Key version must not be empty");
}

this._name = name;
this._version = version;
this._keyId = $"{keyVaultUrl}/keys/{name}/{version}";
_name = name;
_version = version;
_keyId = $"{keyVaultUrl}/keys/{name}";
if (version != null)
{
_keyId = $"{_keyId}/{version}";
}

// initialize credential and lazy clients
this._certificateClient = new Lazy<CertificateClient>(() => new CertificateClient(new Uri(keyVaultUrl), credential));
this._cryptoClient = new Lazy<CryptographyClient>(() => new CryptographyClient(new Uri(_keyId), credential));
this._secretClient = new Lazy<SecretClient>(() => new SecretClient(new Uri(keyVaultUrl), credential));
_certificateClient = new Lazy<CertificateClient>(() => new CertificateClient(new Uri(keyVaultUrl), credential));
_cryptoClient = new Lazy<CryptographyClient>(() => new CryptographyClient(new Uri(_keyId), credential));
_secretClient = new Lazy<SecretClient>(() => new SecretClient(new Uri(keyVaultUrl), credential));
}

/// <summary>
Expand Down Expand Up @@ -115,30 +117,36 @@ private static KeyVaultMetadata ParseId(string id)
{
if (string.IsNullOrEmpty(id))
{
throw new ArgumentNullException(nameof(id), "Id must not be null or empty");
throw new ValidationException("Input passed to \"--id\" must not be empty");
}

var uri = new Uri(id);
// Validate uri
if (uri.Segments.Length != 4)
if (uri.Segments.Length < 3 || uri.Segments.Length > 4)
{
throw new ValidationException(INVALID_INPUT_ERROR_MSG);
throw new ValidationException("Invalid input passed to \"--id\". Please follow this format to input the ID \"https://{vault-name}.vault.azure.net/certificates/{certificate-name}\" or \"https://{vault-name}.vault.azure.net/certificates/{certificate-name}/{certificate-version}\"");
JeyJeyGao marked this conversation as resolved.
Show resolved Hide resolved
}

if (uri.Segments[1] != "keys/" && uri.Segments[1] != "certificates/")
var type = uri.Segments[1].TrimEnd('/');
if (type != "keys" && type != "certificates")
{
throw new ValidationException(INVALID_INPUT_ERROR_MSG);
throw new ValidationException($"Unsupported key vualt object type {type}.");
}

if (uri.Scheme != "https")
{
throw new ValidationException(INVALID_INPUT_ERROR_MSG);
throw new ValidationException($"Unsupported scheme {uri.Scheme}. The scheme must be https.");
}

string? version = null;
if (uri.Segments.Length == 4)
{
version = uri.Segments[3].TrimEnd('/');
}
return new KeyVaultMetadata(
KeyVaultUrl: $"{uri.Scheme}://{uri.Host}",
Name: uri.Segments[2].TrimEnd('/'),
Version: uri.Segments[3].TrimEnd('/')
Version: version
);
}

Expand All @@ -148,9 +156,10 @@ private static KeyVaultMetadata ParseId(string id)
public async Task<byte[]> SignAsync(SignatureAlgorithm algorithm, byte[] payload)
{
var signResult = await _cryptoClient.Value.SignDataAsync(algorithm, payload);
if (signResult.KeyId != _keyId)

if (!string.IsNullOrEmpty(_version) && signResult.KeyId != _keyId)
{
throw new PluginException($"Invalid keys identifier. The user provides {_keyId} but the response contains {signResult.KeyId} as the keys");
throw new PluginException($"Invalid keys identifier. The user provides {_keyId} but the response contains {signResult.KeyId} as the keys. Please ensure the keys identifier is correct.");
JeyJeyGao marked this conversation as resolved.
Show resolved Hide resolved
}

if (signResult.Algorithm != algorithm)
Expand All @@ -166,17 +175,25 @@ public async Task<byte[]> SignAsync(SignatureAlgorithm algorithm, byte[] payload
/// </summary>
public async Task<X509Certificate2> GetCertificateAsync()
{
var cert = await _certificateClient.Value.GetCertificateVersionAsync(_name, _version);

// If the version is invalid, the cert will be fallback to
// the latest. So if the version is not the same as the
// requested version, it means the version is invalid.
if (cert.Value.Properties.Version != _version)
KeyVaultCertificate cert;
if (string.IsNullOrEmpty(_version))
{
throw new ValidationException($"Invalid certificate version. The user provides {_version} but the response contains {cert.Value.Properties.Version} as the version");
// If the version is not specified, get the latest version
cert = (await _certificateClient.Value.GetCertificateAsync(_name)).Value;
}

return new X509Certificate2(cert.Value.Cer);
else
{
cert = (await _certificateClient.Value.GetCertificateVersionAsync(_name, _version)).Value;

// If the version is invalid, the cert will be fallback to
// the latest. So if the version is not the same as the
// requested version, it means the version is invalid.
if (cert.Properties.Version != _version)
{
throw new PluginException($"The version specified in the request is {_version} but the version retrieved from Azure Key Vault is {cert.Properties.Version}. Please ensure the version is correct.");
}
}
return new X509Certificate2(cert.Cer);
}

/// <summary>
Expand Down
2 changes: 2 additions & 0 deletions docs/ca-signed-workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@
```sh
keyID=$(az keyvault certificate show -n $certName --vault-name $keyVault --query 'kid' -o tsv)
```
>[!NOTE]
Versionless KeyID is also supported. For example: https://{vault-name}.vault.azure.net/certificates/{certificate-name}
JeyJeyGao marked this conversation as resolved.
Show resolved Hide resolved
7. [Create an Azure Container Registry](https://learn.microsoft.com/azure/container-registry/container-registry-get-started-portal?tabs=azure-cli). The remaining steps use the example login server `<registry-name>.azurecr.io`, but you must substitute your own login server value.
8. Log in to container registry and push an image for signing:
```sh
Expand Down
2 changes: 2 additions & 0 deletions docs/self-signed-workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@
# get the key identifier
keyID=$(az keyvault certificate show -n $certName --vault-name $keyVault --query 'kid' -o tsv)
```
>[!NOTE]
Versionless KeyID is also supported. For example: https://{vault-name}.vault.azure.net/certificates/{certificate-name}
JeyJeyGao marked this conversation as resolved.
Show resolved Hide resolved
5. [Create an Azure Container Registry](https://learn.microsoft.com/azure/container-registry/container-registry-get-started-portal?tabs=azure-cli). The remaining steps use the example login server `<registry-name>.azurecr.io`, but you must substitute your own login server value.
6. Log in to container registry and push an image for signing:
```sh
Expand Down
Loading
Loading