Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 9182571

Browse files
committed
Fix behavior and test for X509Store.Add when store is readonly.
Despite what was concluded from the earlier testing, Windows does actually report an access denied error when calling X509Store.Add on a store instance that was opened as read-only. This fixes the test, and the Unix implementation.
1 parent 51f757c commit 9182571

File tree

2 files changed

+8
-10
lines changed

2 files changed

+8
-10
lines changed

src/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/DirectoryBasedStoreProvider.cs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,12 @@ private List<X509Certificate2> ReadDirectory()
190190

191191
public void Add(ICertificatePal certPal)
192192
{
193+
if (_readOnly)
194+
{
195+
// Windows compatibility: Remove only throws when it needs to do work, add throws always.
196+
throw new CryptographicException(SR.Cryptography_X509_StoreReadOnly);
197+
}
198+
193199
// Save the collection to a local so it's consistent for the whole method
194200
List<X509Certificate2> certificates = _certificates;
195201
OpenSslX509CertificateReader cert = (OpenSslX509CertificateReader)certPal;
@@ -267,13 +273,6 @@ public void Add(ICertificatePal certPal)
267273
}
268274
}
269275

270-
if (_readOnly)
271-
{
272-
// Windows compatibility: Don't throw until it has been established that
273-
// the certificate is not present, and we need to modify the filesystem.
274-
throw new CryptographicException(SR.Cryptography_X509_StoreReadOnly);
275-
}
276-
277276
string destinationFilename;
278277
FileMode mode = FileMode.CreateNew;
279278

src/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public static void AddReadOnlyThrows()
6060

6161
[Fact]
6262
[ActiveIssue(2820, PlatformID.AnyUnix)]
63-
public static void AddReadOnlyDoesntThrowWhenCertificateExists()
63+
public static void AddReadOnlyThrowsWhenCertificateExists()
6464
{
6565
using (X509Store store = new X509Store(StoreName.My, StoreLocation.CurrentUser))
6666
{
@@ -82,8 +82,7 @@ public static void AddReadOnlyDoesntThrowWhenCertificateExists()
8282

8383
if (toAdd != null)
8484
{
85-
// This shouldn't throw, the cert is already present.
86-
store.Add(toAdd);
85+
Assert.Throws<CryptographicException>(() => store.Add(toAdd));
8786
}
8887
}
8988
}

0 commit comments

Comments
 (0)