From 81d539de4be21e9952d7f8fc30870d73cfe50c45 Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Mon, 27 May 2024 13:23:36 +0800 Subject: [PATCH 01/16] feat: add versionless key identifier support Signed-off-by: Junjie Gao --- .../Command/GenerateSignatureTests.cs | 6 +- .../KeyVault/KeyVaultClientTests.cs | 39 ++++++------ .../Command/DescribeKey.cs | 3 +- .../Command/GenerateSignature.cs | 2 +- .../KeyVault/KeyVaultClient.cs | 63 +++++++++---------- test/e2e/action.yml | 10 +++ 6 files changed, 62 insertions(+), 61 deletions(-) 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..18670c37 100644 --- a/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs +++ b/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs @@ -47,10 +47,25 @@ 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("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")] public void TestConstructorWithInvalidKeyId(string invalidKeyId) { @@ -61,7 +76,7 @@ public void TestConstructorWithInvalidKeyId(string invalidKeyId) [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 @@ -176,24 +191,6 @@ public async Task GetCertificateAsync_ReturnsCertificate() Assert.Equal(testCertificate.RawData, certificate.RawData); } - [Fact] - public async Task GetCertificateAsyncThrowValidationException() - { - 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: "1234"), - cer: testCertificate.RawData); - - var keyVaultClient = CreateMockedKeyVaultClient(keyVaultCertificate); - - await Assert.ThrowsAsync(async () => await keyVaultClient.GetCertificateAsync()); - } - [Fact] public async Task GetCertificateChainAsync_PKCS12() { diff --git a/Notation.Plugin.AzureKeyVault/Command/DescribeKey.cs b/Notation.Plugin.AzureKeyVault/Command/DescribeKey.cs index 4a4b1f49..ba6334d6 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 for the plugin. Please contact the Notation developer 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..17e68a1f 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 for the plugin. Please contact the Notation developer 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..853db0b2 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,47 @@ 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)) + _name = name; + _version = version; + _keyId = $"{keyVaultUrl}/keys/{name}"; + if (!string.IsNullOrEmpty(version)) { - throw new ArgumentNullException(nameof(version), "KeyVersion must not be null or empty"); + _keyId = $"{_keyId}/{version}"; } - this._name = name; - this._version = version; - this._keyId = $"{keyVaultUrl}/keys/{name}/{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 +112,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("identifier must not be null or empty"); } var uri = new Uri(id); // Validate uri - if (uri.Segments.Length != 4) + if (uri.Segments.Length < 3) { - throw new ValidationException(INVALID_INPUT_ERROR_MSG); + throw new ValidationException("Invalid key identifier or certificate identifier. Please visit the link for more information: https://learn.microsoft.com/azure/key-vault/general/about-keys-secrets-certificates#object-identifiers"); } - 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 ); } @@ -166,15 +169,7 @@ 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) - { - throw new ValidationException($"Invalid certificate version. The user provides {_version} but the response contains {cert.Value.Properties.Version} as the version"); - } + var cert = await _certificateClient.Value.GetCertificateVersionAsync(_name, _version ?? ""); return new X509Certificate2(cert.Value.Cer); } @@ -184,7 +179,7 @@ public async Task GetCertificateAsync() /// public async Task GetCertificateChainAsync() { - var secret = await _secretClient.Value.GetSecretAsync(_name, _version); + var secret = await _secretClient.Value.GetSecretAsync(_name, _version ?? ""); var chain = new X509Certificate2Collection(); var contentType = secret.Value.Properties.ContentType; diff --git a/test/e2e/action.yml b/test/e2e/action.yml index 479e71a0..50e490bd 100644 --- a/test/e2e/action.yml +++ b/test/e2e/action.yml @@ -35,6 +35,16 @@ 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: imported ca-issued pem uses: notaryproject/notation-action/sign@v1 with: From baf35de9eb8798eca0dcacfdd56ea9259dc5b841 Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Mon, 27 May 2024 13:38:19 +0800 Subject: [PATCH 02/16] fix: update Signed-off-by: Junjie Gao --- .../KeyVault/KeyVaultClient.cs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs index 853db0b2..9be4b71d 100644 --- a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs +++ b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs @@ -169,9 +169,16 @@ public async Task SignAsync(SignatureAlgorithm algorithm, byte[] payload /// public async Task GetCertificateAsync() { - var cert = await _certificateClient.Value.GetCertificateVersionAsync(_name, _version ?? ""); - - return new X509Certificate2(cert.Value.Cer); + KeyVaultCertificate cert; + if (string.IsNullOrEmpty(_version)) + { + cert = (await _certificateClient.Value.GetCertificateAsync(_name)).Value; + } + else + { + cert = (await _certificateClient.Value.GetCertificateVersionAsync(_name, _version)).Value; + } + return new X509Certificate2(cert.Cer); } /// @@ -179,7 +186,7 @@ public async Task GetCertificateAsync() /// public async Task GetCertificateChainAsync() { - var secret = await _secretClient.Value.GetSecretAsync(_name, _version ?? ""); + var secret = await _secretClient.Value.GetSecretAsync(_name, _version); var chain = new X509Certificate2Collection(); var contentType = secret.Value.Properties.ContentType; From e40f48fda6b52b6cf33f56dbff718c0ee1453f2b Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Mon, 27 May 2024 13:42:55 +0800 Subject: [PATCH 03/16] fix: update Signed-off-by: Junjie Gao --- Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs index 9be4b71d..911604f4 100644 --- a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs +++ b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs @@ -151,10 +151,6 @@ 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) - { - throw new PluginException($"Invalid keys identifier. The user provides {_keyId} but the response contains {signResult.KeyId} as the keys"); - } if (signResult.Algorithm != algorithm) { From 7e5223e47beeace41cb580ff242c1a2c92c0afc1 Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Mon, 27 May 2024 13:50:34 +0800 Subject: [PATCH 04/16] test: update Signed-off-by: Junjie Gao --- .../KeyVault/KeyVaultClientTests.cs | 17 +++-------------- .../KeyVault/KeyVaultClient.cs | 2 +- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs b/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs index 18670c37..620ae13e 100644 --- a/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs +++ b/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs @@ -65,8 +65,11 @@ public void TestConstructorWithVersionlessKey() } [Theory] + [InlineData("")] [InlineData("https://myvault.vault.azure.net/invalid/my-key/123")] [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))); @@ -142,20 +145,6 @@ public async Task TestSignAsyncReturnsExpectedSignature() Assert.Equal(signResult.Signature, signature); } - [Fact] - public async Task TestSignAsyncThrowsExceptionOnInvalidKeyId() - { - var signResult = CryptographyModelFactory.SignResult( - keyId: "https://fake.vault.azure.net/keys/invalid-key/123", - signature: new byte[] { 1, 2, 3 }, - algorithm: SignatureAlgorithm.RS256); - - TestableKeyVaultClient keyVaultClient = CreateMockedKeyVaultClient(signResult); - byte[] payload = new byte[] { 4, 5, 6 }; - - await Assert.ThrowsAsync(async () => await keyVaultClient.SignAsync(SignatureAlgorithm.RS256, payload)); - } - [Fact] public async Task TestSignAsyncThrowsExceptionOnInvalidAlgorithm() { diff --git a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs index 911604f4..39cbe5e2 100644 --- a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs +++ b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs @@ -117,7 +117,7 @@ private static KeyVaultMetadata ParseId(string id) var uri = new Uri(id); // Validate uri - if (uri.Segments.Length < 3) + if (uri.Segments.Length < 3 || uri.Segments.Length > 4) { throw new ValidationException("Invalid key identifier or certificate identifier. Please visit the link for more information: https://learn.microsoft.com/azure/key-vault/general/about-keys-secrets-certificates#object-identifiers"); } From 8a98dbba125cda0283a734c4d9aa54d67206e695 Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Mon, 27 May 2024 14:22:20 +0800 Subject: [PATCH 05/16] test: add test cases Signed-off-by: Junjie Gao --- .../KeyVault/KeyVaultClientTests.cs | 58 +++++++++++++++++-- .../KeyVault/KeyVaultClient.cs | 9 +++ test/e2e/action.yml | 22 ++++++- 3 files changed, 82 insertions(+), 7 deletions(-) diff --git a/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs b/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs index 620ae13e..4f18d869 100644 --- a/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs +++ b/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs @@ -75,6 +75,14 @@ public void TestConstructorWithInvalidKeyId(string invalidKeyId) Assert.Throws(() => new KeyVaultClient(invalidKeyId, Credentials.GetCredentials(defaultCredentialType))); } + [Theory] + [InlineData("", "", "")] + [InlineData("https://myvault.vault.azure.net", "", "")] + 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) @@ -90,7 +98,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); @@ -121,6 +129,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); @@ -162,11 +179,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); @@ -180,6 +192,40 @@ 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() + { + 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: "1234"), + cer: testCertificate.RawData); + + var keyVaultClient = CreateMockedKeyVaultClient(keyVaultCertificate); + + await Assert.ThrowsAsync(keyVaultClient.GetCertificateAsync); + } + [Fact] public async Task GetCertificateChainAsync_PKCS12() { diff --git a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs index 39cbe5e2..f6cc4d40 100644 --- a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs +++ b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs @@ -168,11 +168,20 @@ public async Task GetCertificateAsync() KeyVaultCertificate cert; if (string.IsNullOrEmpty(_version)) { + // If the version is not specified, get the latest version cert = (await _certificateClient.Value.GetCertificateAsync(_name)).Value; } 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 of the certificate retrieved from Azure Key Vault is different from the version specified in the request. The version specified in the request is {_version} but the version retrieved from Azure Key Vault is {cert.Properties.Version}"); + } } return new X509Certificate2(cert.Cer); } diff --git a/test/e2e/action.yml b/test/e2e/action.yml index 50e490bd..227a19c7 100644 --- a/test/e2e/action.yml +++ b/test/e2e/action.yml @@ -119,6 +119,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 @@ -265,4 +285,4 @@ runs: signature_format: cose plugin_config: | credential_type=azurecli - self_signed=true \ No newline at end of file + self_signed=true From f9479613624267ffb00f1408de38a8e9f116b7b4 Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Mon, 27 May 2024 14:47:57 +0800 Subject: [PATCH 06/16] fix: update error message Signed-off-by: Junjie Gao --- Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs index f6cc4d40..557994dd 100644 --- a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs +++ b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs @@ -112,14 +112,14 @@ private static KeyVaultMetadata ParseId(string id) { if (string.IsNullOrEmpty(id)) { - throw new ValidationException("identifier 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 < 3 || uri.Segments.Length > 4) { - throw new ValidationException("Invalid key identifier or certificate identifier. Please visit the link for more information: https://learn.microsoft.com/azure/key-vault/general/about-keys-secrets-certificates#object-identifiers"); + 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}\""); } var type = uri.Segments[1].TrimEnd('/'); From 071ae73544a470665d74ea007e9b5f626a32e260 Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Mon, 27 May 2024 16:19:35 +0800 Subject: [PATCH 07/16] fix: resolve comments Signed-off-by: Junjie Gao --- .../KeyVault/KeyVaultClientTests.cs | 1 + .../Command/DescribeKey.cs | 2 +- .../Command/GenerateSignature.cs | 2 +- .../KeyVault/KeyVaultClient.cs | 16 +++++++++++++--- docs/ca-signed-workflow.md | 2 ++ docs/self-signed-workflow.md | 2 ++ 6 files changed, 20 insertions(+), 5 deletions(-) diff --git a/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs b/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs index 4f18d869..80502d97 100644 --- a/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs +++ b/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs @@ -78,6 +78,7 @@ public void TestConstructorWithInvalidKeyId(string invalidKeyId) [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))); diff --git a/Notation.Plugin.AzureKeyVault/Command/DescribeKey.cs b/Notation.Plugin.AzureKeyVault/Command/DescribeKey.cs index ba6334d6..50c24651 100644 --- a/Notation.Plugin.AzureKeyVault/Command/DescribeKey.cs +++ b/Notation.Plugin.AzureKeyVault/Command/DescribeKey.cs @@ -22,7 +22,7 @@ public DescribeKey(string inputJson) var request = JsonSerializer.Deserialize(inputJson, DescribeKeyRequestContext.Default.DescribeKeyRequest); if (request == null) { - throw new ValidationException("Failed to parse the request for the plugin. Please contact the Notation developer to resolve the issue."); + throw new ValidationException("Failed to parse the request in JSON format. Please contact the Notation developer 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 17e68a1f..cd3581c4 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("Failed to parse the request for the plugin. Please contact the Notation developer to resolve the issue."); + throw new ValidationException("Failed to parse the request in JSON format. Please contact the Notation developer 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 557994dd..fe98db9f 100644 --- a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs +++ b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs @@ -72,10 +72,15 @@ public KeyVaultClient(string keyVaultUrl, string name, string? version, TokenCre throw new ValidationException("Key name must not be null or empty"); } + if (version != null && version == string.Empty) + { + throw new ValidationException("Key version must not be empty"); + } + _name = name; _version = version; _keyId = $"{keyVaultUrl}/keys/{name}"; - if (!string.IsNullOrEmpty(version)) + if (version != null) { _keyId = $"{_keyId}/{version}"; } @@ -119,7 +124,7 @@ private static KeyVaultMetadata ParseId(string id) // Validate uri if (uri.Segments.Length < 3 || uri.Segments.Length > 4) { - 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}\""); + 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}\""); } var type = uri.Segments[1].TrimEnd('/'); @@ -152,6 +157,11 @@ public async Task SignAsync(SignatureAlgorithm algorithm, byte[] payload { var signResult = await _cryptoClient.Value.SignDataAsync(algorithm, payload); + 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. Please ensure the keys identifier is correct."); + } + if (signResult.Algorithm != algorithm) { throw new PluginException($"Invalid signature algorithm. The user provides {algorithm} but the response contains {signResult.Algorithm} as the algorithm"); @@ -180,7 +190,7 @@ public async Task GetCertificateAsync() // requested version, it means the version is invalid. if (cert.Properties.Version != _version) { - throw new PluginException($"The version of the certificate retrieved from Azure Key Vault is different from the version specified in the request. The version specified in the request is {_version} but the version retrieved from Azure Key Vault is {cert.Properties.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..54579163 100644 --- a/docs/ca-signed-workflow.md +++ b/docs/ca-signed-workflow.md @@ -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} 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..c4785384 100644 --- a/docs/self-signed-workflow.md +++ b/docs/self-signed-workflow.md @@ -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} 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 From 25ab95559a09f3c57ad12d2e24260cbdc8049d7c Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Mon, 27 May 2024 16:41:45 +0800 Subject: [PATCH 08/16] test: restore test Signed-off-by: Junjie Gao --- .../KeyVault/KeyVaultClientTests.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs b/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs index 80502d97..35566ea8 100644 --- a/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs +++ b/Notation.Plugin.AzureKeyVault.Tests/KeyVault/KeyVaultClientTests.cs @@ -163,6 +163,20 @@ public async Task TestSignAsyncReturnsExpectedSignature() Assert.Equal(signResult.Signature, signature); } + [Fact] + public async Task TestSignAsyncThrowsExceptionOnInvalidKeyId() + { + var signResult = CryptographyModelFactory.SignResult( + keyId: "https://fake.vault.azure.net/keys/invalid-key/123", + signature: new byte[] { 1, 2, 3 }, + algorithm: SignatureAlgorithm.RS256); + + TestableKeyVaultClient keyVaultClient = CreateMockedKeyVaultClient(signResult); + byte[] payload = new byte[] { 4, 5, 6 }; + + await Assert.ThrowsAsync(async () => await keyVaultClient.SignAsync(SignatureAlgorithm.RS256, payload)); + } + [Fact] public async Task TestSignAsyncThrowsExceptionOnInvalidAlgorithm() { From 8410e3e2ffdb4d2c582c6598be9ca0d9000c7f8e Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Tue, 28 May 2024 10:46:25 +0800 Subject: [PATCH 09/16] fix: resolve comments Signed-off-by: Junjie Gao --- Notation.Plugin.AzureKeyVault/Command/DescribeKey.cs | 2 +- Notation.Plugin.AzureKeyVault/Command/GenerateSignature.cs | 2 +- Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs | 2 +- docs/ca-signed-workflow.md | 2 +- docs/self-signed-workflow.md | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Notation.Plugin.AzureKeyVault/Command/DescribeKey.cs b/Notation.Plugin.AzureKeyVault/Command/DescribeKey.cs index 50c24651..eb0bf774 100644 --- a/Notation.Plugin.AzureKeyVault/Command/DescribeKey.cs +++ b/Notation.Plugin.AzureKeyVault/Command/DescribeKey.cs @@ -22,7 +22,7 @@ public DescribeKey(string inputJson) var request = JsonSerializer.Deserialize(inputJson, DescribeKeyRequestContext.Default.DescribeKeyRequest); if (request == null) { - throw new ValidationException("Failed to parse the request in JSON format. Please contact the Notation developer to resolve the issue."); + 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 cd3581c4..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("Failed to parse the request in JSON format. Please contact the Notation developer to resolve the issue."); + 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 fe98db9f..f26f40db 100644 --- a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs +++ b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs @@ -159,7 +159,7 @@ public async Task SignAsync(SignatureAlgorithm algorithm, byte[] payload 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. Please ensure the keys identifier is correct."); + 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) diff --git a/docs/ca-signed-workflow.md b/docs/ca-signed-workflow.md index 54579163..0ab6bdde 100644 --- a/docs/ca-signed-workflow.md +++ b/docs/ca-signed-workflow.md @@ -111,7 +111,7 @@ ```sh keyID=$(az keyvault certificate show -n $certName --vault-name $keyVault --query 'kid' -o tsv) ``` - >[!NOTE] + > [!NOTE] 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: diff --git a/docs/self-signed-workflow.md b/docs/self-signed-workflow.md index c4785384..46d48091 100644 --- a/docs/self-signed-workflow.md +++ b/docs/self-signed-workflow.md @@ -74,7 +74,7 @@ # get the key identifier keyID=$(az keyvault certificate show -n $certName --vault-name $keyVault --query 'kid' -o tsv) ``` - >[!NOTE] + > [!NOTE] 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: From 7b7c168813cfcf920d1cdc25e9f9b588493a8ccc Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Tue, 28 May 2024 10:48:20 +0800 Subject: [PATCH 10/16] doc: update note Signed-off-by: Junjie Gao --- docs/ca-signed-workflow.md | 2 +- docs/self-signed-workflow.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/ca-signed-workflow.md b/docs/ca-signed-workflow.md index 0ab6bdde..d8f56437 100644 --- a/docs/ca-signed-workflow.md +++ b/docs/ca-signed-workflow.md @@ -112,7 +112,7 @@ 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} + > 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 46d48091..310e1ede 100644 --- a/docs/self-signed-workflow.md +++ b/docs/self-signed-workflow.md @@ -75,7 +75,7 @@ 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} + > 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 From 21b75d94649c4d0b2626d6acfb63d3d5f54f1090 Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Tue, 28 May 2024 10:52:22 +0800 Subject: [PATCH 11/16] doc: update Signed-off-by: Junjie Gao --- docs/ca-signed-workflow.md | 1 + docs/self-signed-workflow.md | 1 + 2 files changed, 2 insertions(+) diff --git a/docs/ca-signed-workflow.md b/docs/ca-signed-workflow.md index d8f56437..e120c51c 100644 --- a/docs/ca-signed-workflow.md +++ b/docs/ca-signed-workflow.md @@ -111,6 +111,7 @@ ```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} 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. diff --git a/docs/self-signed-workflow.md b/docs/self-signed-workflow.md index 310e1ede..b1eb2994 100644 --- a/docs/self-signed-workflow.md +++ b/docs/self-signed-workflow.md @@ -74,6 +74,7 @@ # 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} 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. From 9cbabc1f552453aaedb22d0b1acdba761d09eb4c Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Tue, 28 May 2024 10:55:55 +0800 Subject: [PATCH 12/16] doc: update Signed-off-by: Junjie Gao --- docs/ca-signed-workflow.md | 1 + docs/self-signed-workflow.md | 1 + 2 files changed, 2 insertions(+) diff --git a/docs/ca-signed-workflow.md b/docs/ca-signed-workflow.md index e120c51c..6df6272d 100644 --- a/docs/ca-signed-workflow.md +++ b/docs/ca-signed-workflow.md @@ -114,6 +114,7 @@ > [!NOTE] > 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 b1eb2994..dbc32a11 100644 --- a/docs/self-signed-workflow.md +++ b/docs/self-signed-workflow.md @@ -77,6 +77,7 @@ > [!NOTE] > 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 From ae77682160d8ac3099516107ee7995a488ef7a1d Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Tue, 28 May 2024 11:01:09 +0800 Subject: [PATCH 13/16] doc: update Signed-off-by: Junjie Gao --- docs/ca-signed-workflow.md | 6 ++---- docs/self-signed-workflow.md | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/docs/ca-signed-workflow.md b/docs/ca-signed-workflow.md index 6df6272d..941429ca 100644 --- a/docs/ca-signed-workflow.md +++ b/docs/ca-signed-workflow.md @@ -111,10 +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} - +> [!NOTE] +> 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 dbc32a11..0b88bdfb 100644 --- a/docs/self-signed-workflow.md +++ b/docs/self-signed-workflow.md @@ -74,10 +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} - +> [!NOTE] +> 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 From e2d3a6b2d61866a041af44c3b33a41debadaf2ac Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Tue, 28 May 2024 11:03:46 +0800 Subject: [PATCH 14/16] doc: update Signed-off-by: Junjie Gao --- docs/ca-signed-workflow.md | 12 ++++++------ docs/self-signed-workflow.md | 8 ++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/ca-signed-workflow.md b/docs/ca-signed-workflow.md index 941429ca..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,7 +111,7 @@ ```sh keyID=$(az keyvault certificate show -n $certName --vault-name $keyVault --query 'kid' -o tsv) ``` -> [!NOTE] +> [!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: diff --git a/docs/self-signed-workflow.md b/docs/self-signed-workflow.md index 0b88bdfb..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,7 +74,7 @@ # get the key identifier keyID=$(az keyvault certificate show -n $certName --vault-name $keyVault --query 'kid' -o tsv) ``` -> [!NOTE] +> [!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: From 7667e270342afd179420fc525ce5a3afe253e693 Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Tue, 28 May 2024 11:28:46 +0800 Subject: [PATCH 15/16] fix: resolve comments Signed-off-by: Junjie Gao --- .../KeyVault/KeyVaultClient.cs | 4 ++-- test/e2e/action.yml | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs index f26f40db..a86eb2f4 100644 --- a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs +++ b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs @@ -120,11 +120,11 @@ private static KeyVaultMetadata ParseId(string id) 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 < 3 || uri.Segments.Length > 4) { - 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}\""); + 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}/[certificate-version]\""); } var type = uri.Segments[1].TrimEnd('/'); diff --git a/test/e2e/action.yml b/test/e2e/action.yml index 227a19c7..838d3cc5 100644 --- a/test/e2e/action.yml +++ b/test/e2e/action.yml @@ -45,6 +45,16 @@ runs: 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: From deb88914d62ffdc8f7ed798b983de18ca614e331 Mon Sep 17 00:00:00 2001 From: Junjie Gao Date: Tue, 28 May 2024 12:37:44 +0800 Subject: [PATCH 16/16] fix: update error message Signed-off-by: Junjie Gao --- Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs index a86eb2f4..37a68c5a 100644 --- a/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs +++ b/Notation.Plugin.AzureKeyVault/KeyVault/KeyVaultClient.cs @@ -124,7 +124,7 @@ private static KeyVaultMetadata ParseId(string id) // Validate uri if (uri.Segments.Length < 3 || uri.Segments.Length > 4) { - 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}/[certificate-version]\""); + throw new ValidationException("Invalid input passed to \"--id\". Please follow this format to input the ID \"https://.vault.azure.net/certificates//[certificate-version]\""); } var type = uri.Segments[1].TrimEnd('/');