diff --git a/Notation.Plugin.AzureKeyVault.Tests/Command/GenerateSignatureTests.cs b/Notation.Plugin.AzureKeyVault.Tests/Command/GenerateSignatureTests.cs index 5c36a365..8cae7bbd 100644 --- a/Notation.Plugin.AzureKeyVault.Tests/Command/GenerateSignatureTests.cs +++ b/Notation.Plugin.AzureKeyVault.Tests/Command/GenerateSignatureTests.cs @@ -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"; @@ -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"; @@ -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"; diff --git a/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs b/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs index fbd0a784..35566ea8 100644 --- a/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs +++ b/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs @@ -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(() => 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(() => new KeyVaultClient(keyVaultUrl, name, version, Credentials.GetCredentials(defaultCredentialType))); + } + [Theory] [InlineData("")] public void TestConstructorWithEmptyKeyId(string invalidKeyId) { - Assert.Throws(() => new KeyVaultClient(invalidKeyId, Credentials.GetCredentials(defaultCredentialType))); + Assert.Throws(() => new KeyVaultClient(invalidKeyId, Credentials.GetCredentials(defaultCredentialType))); } private class TestableKeyVaultClient : KeyVaultClient @@ -72,7 +99,7 @@ public TestableKeyVaultClient(string keyVaultUrl, string name, string version, C this._cryptoClient = new Lazy(() => 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); @@ -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(new Uri("https://fake.vault.azure.net/certificates/fake-certificate/123"), new Mock().Object); + mockCertificateClient.Setup(c => c.GetCertificateAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(Response.FromValue(certWithPolicy, new Mock().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(new Uri("https://fake.vault.azure.net/secrets/fake-secret/123"), new Mock().Object); @@ -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); @@ -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(certificate); + Assert.Equal(testCertificate.RawData, certificate.RawData); + } + [Fact] public async Task GetCertificateAsyncThrowValidationException() { @@ -191,7 +238,7 @@ public async Task GetCertificateAsyncThrowValidationException() var keyVaultClient = CreateMockedKeyVaultClient(keyVaultCertificate); - await Assert.ThrowsAsync(async () => await keyVaultClient.GetCertificateAsync()); + await Assert.ThrowsAsync(keyVaultClient.GetCertificateAsync); } [Fact] diff --git a/Notation.Plugin.AzureKeyVault/Command/DescribeKey.cs b/Notation.Plugin.AzureKeyVault/Command/DescribeKey.cs index 4a4b1f49..eb0bf774 100644 --- a/Notation.Plugin.AzureKeyVault/Command/DescribeKey.cs +++ b/Notation.Plugin.AzureKeyVault/Command/DescribeKey.cs @@ -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://.vault.azure.net///\"}'"; /// /// Constructor to create DescribeKey object from JSON string. @@ -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 Notation maintainers to resolve the issue."); } this._request = request; this._keyVaultClient = new KeyVaultClient( diff --git a/Notation.Plugin.AzureKeyVault/Command/GenerateSignature.cs b/Notation.Plugin.AzureKeyVault/Command/GenerateSignature.cs index 67f06ea4..bbb0a8fb 100644 --- a/Notation.Plugin.AzureKeyVault/Command/GenerateSignature.cs +++ b/Notation.Plugin.AzureKeyVault/Command/GenerateSignature.cs @@ -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 Notation maintainers to resolve the issue."); } this._request = request; this._keyVaultClient = new KeyVaultClient( diff --git a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs index 6fade0cf..37a68c5a 100644 --- a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs +++ b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs @@ -34,7 +34,7 @@ public class KeyVaultClient : IKeyVaultClient /// /// A helper record to store KeyVault metadata. /// - 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 @@ -43,50 +43,52 @@ private record KeyVaultMetadata(string KeyVaultUrl, string Name, string Version) protected Lazy _cryptoClient; // Secret client (lazy initialization) protected Lazy _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://.vault.azure.net///\"}'"; // Key name or certificate name private string _name; // Key version or certificate version - private string _version; + private string? _version; // Key identifier (e.g. https://.vault.azure.net/keys//) private string _keyId; // Internal getters for unit test internal string Name => _name; - internal string Version => _version; + internal string? Version => _version; internal string KeyId => _keyId; /// /// Constructor to create AzureKeyVault object from keyVaultUrl, name /// and version. /// - 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) { - 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(() => new CertificateClient(new Uri(keyVaultUrl), credential)); - this._cryptoClient = new Lazy(() => new CryptographyClient(new Uri(_keyId), credential)); - this._secretClient = new Lazy(() => new SecretClient(new Uri(keyVaultUrl), credential)); + _certificateClient = new Lazy(() => new CertificateClient(new Uri(keyVaultUrl), credential)); + _cryptoClient = new Lazy(() => new CryptographyClient(new Uri(_keyId), credential)); + _secretClient = new Lazy(() => new SecretClient(new Uri(keyVaultUrl), credential)); } /// @@ -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); + var uri = new Uri(id.TrimEnd('/')); // 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.azure.net/certificates//[certificate-version]\""); } - 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 ); } @@ -148,9 +156,10 @@ private static KeyVaultMetadata ParseId(string id) public async Task 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 key identifier. User required {_keyId} does not match {signResult.KeyId} in response. Please ensure the provided key identifier is correct."); } if (signResult.Algorithm != algorithm) @@ -166,17 +175,25 @@ public async Task SignAsync(SignatureAlgorithm algorithm, byte[] payload /// public async Task 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); } /// diff --git a/docs/ca-signed-workflow.md b/docs/ca-signed-workflow.md index a736d332..65bff33e 100644 --- a/docs/ca-signed-workflow.md +++ b/docs/ca-signed-workflow.md @@ -31,9 +31,9 @@ --secret-permissions get \ --object-id "$userId" ``` - > [!NOTE] - > The script assigns the permission to the current user, and you can also assign the permission to your [managed identity](https://learn.microsoft.com/azure/active-directory/managed-identities-azure-resources/overview) or [service principal](https://learn.microsoft.com/azure/active-directory/develop/app-objects-and-service-principals?tabs=browser). - > To know more about permission management, please visit [Azure Key Vualt access policy](https://learn.microsoft.com/azure/key-vault/general/assign-access-policy?tabs=azure-portal). +> [!NOTE] +> The script assigns the permission to the current user, and you can also assign the permission to your [managed identity](https://learn.microsoft.com/azure/active-directory/managed-identities-azure-resources/overview) or [service principal](https://learn.microsoft.com/azure/active-directory/develop/app-objects-and-service-principals?tabs=browser). +> To know more about permission management, please visit [Azure Key Vualt access policy](https://learn.microsoft.com/azure/key-vault/general/assign-access-policy?tabs=azure-portal). 4. Create a Certificate Signing Request (CSR): Generate certificate policy: @@ -100,8 +100,8 @@ certChainPath="${certName}-chain.crt" cat "$signedCertPath" ca.crt > "$certChainPath" ``` - > [!NOTE] - > If you have merged your certificate to Azure Key Vault without certificate chain or you don't want the plugin access your certificate chain with the `Secrets Get` permission, please use [ca_certs](./plugin-config.md#ca_certs) plugin configuration argument instead. +> [!NOTE] +> If you have merged your certificate to Azure Key Vault without certificate chain or you don't want the plugin access your certificate chain with the `Secrets Get` permission, please use [ca_certs](./plugin-config.md#ca_certs) plugin configuration argument instead. 6. After you get the leaf certificate, you can merge the certificate chain (file at `$certChainPath`) to your Azure Key Vault: ```sh @@ -111,6 +111,8 @@ ```sh keyID=$(az keyvault certificate show -n $certName --vault-name $keyVault --query 'kid' -o tsv) ``` +> [!TIP] +> Versionless KeyID is also supported. For example: https://{vault-name}.vault.azure.net/certificates/{certificate-name} 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 `.azurecr.io`, but you must substitute your own login server value. 8. Log in to container registry and push an image for signing: ```sh diff --git a/docs/self-signed-workflow.md b/docs/self-signed-workflow.md index 2c887395..98c1152e 100644 --- a/docs/self-signed-workflow.md +++ b/docs/self-signed-workflow.md @@ -33,9 +33,9 @@ --certificates-permissions get \ --upn "$userId" ``` - > [!NOTE] - > The script assigns the permission to the current user, and you can also assign the permission to your [managed identity](https://learn.microsoft.com/azure/active-directory/managed-identities-azure-resources/overview) or [service principal](https://learn.microsoft.com/azure/active-directory/develop/app-objects-and-service-principals?tabs=browser). - > To know more about permission management, please visit [Azure Key Vualt access policy](https://learn.microsoft.com/azure/key-vault/general/assign-access-policy?tabs=azure-portal). +> [!NOTE] +> The script assigns the permission to the current user, and you can also assign the permission to your [managed identity](https://learn.microsoft.com/azure/active-directory/managed-identities-azure-resources/overview) or [service principal](https://learn.microsoft.com/azure/active-directory/develop/app-objects-and-service-principals?tabs=browser). +> To know more about permission management, please visit [Azure Key Vualt access policy](https://learn.microsoft.com/azure/key-vault/general/assign-access-policy?tabs=azure-portal). 4. create a self-signed certificate: ```sh # generate certificate policy @@ -74,6 +74,8 @@ # get the key identifier keyID=$(az keyvault certificate show -n $certName --vault-name $keyVault --query 'kid' -o tsv) ``` +> [!TIP] +> Versionless KeyID is also supported. For example: https://{vault-name}.vault.azure.net/certificates/{certificate-name} 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 `.azurecr.io`, but you must substitute your own login server value. 6. Log in to container registry and push an image for signing: ```sh diff --git a/test/e2e/action.yml b/test/e2e/action.yml index 479e71a0..838d3cc5 100644 --- a/test/e2e/action.yml +++ b/test/e2e/action.yml @@ -35,6 +35,26 @@ runs: target_artifact_reference: localhost:5000/hello-world:v1 signature_format: cose plugin_config: 'self_signed=true' + - name: self-signed versionless pem certificate + uses: notaryproject/notation-action/sign@v1 + with: + plugin_name: azure-kv + plugin_url: ${{ inputs.pluginDownloadURL }} + plugin_checksum: ${{ inputs.pluginChecksum }} + key_id: https://acrci-test-kv.vault.azure.net/keys/self-signed-pem + target_artifact_reference: localhost:5000/hello-world:v1 + signature_format: cose + plugin_config: 'self_signed=true' + - name: self-signed versionless pem certificate id ends with slash + uses: notaryproject/notation-action/sign@v1 + with: + plugin_name: azure-kv + plugin_url: ${{ inputs.pluginDownloadURL }} + plugin_checksum: ${{ inputs.pluginChecksum }} + key_id: https://acrci-test-kv.vault.azure.net/keys/self-signed-pem/ + target_artifact_reference: localhost:5000/hello-world:v1 + signature_format: cose + plugin_config: 'self_signed=true' - name: imported ca-issued pem uses: notaryproject/notation-action/sign@v1 with: @@ -109,6 +129,26 @@ runs: target_artifact_reference: localhost:5000/hello-world:v1 signature_format: cose plugin_config: 'ca_certs=./test/e2e/certs/cert-bundle.pem' + + # failed test cases + - name: invalid certificate version + continue-on-error: true + id: invalid-certificate-version + uses: notaryproject/notation-action/sign@v1 + with: + plugin_name: azure-kv + plugin_url: ${{ inputs.pluginDownloadURL }} + plugin_checksum: ${{ inputs.pluginChecksum }} + key_id: https://acrci-test-kv.vault.azure.net/keys/self-signed-pem/invalid + target_artifact_reference: localhost:5000/hello-world:v1 + signature_format: cose + plugin_config: 'self_signed=true' + - name: 'Should Fail: invalid certificate version' + if: steps.invalid-certificate-version.outcome != 'failure' + run: | + echo "invalid certificate version should failed, but succeeded." + exit 1 + shell: bash - name: partial pem cert chain with local invalid local cert bundle continue-on-error: true id: partial-pem-cert-chain-invalid-local-cert-bundle @@ -255,4 +295,4 @@ runs: signature_format: cose plugin_config: | credential_type=azurecli - self_signed=true \ No newline at end of file + self_signed=true