[Mono-dev] Question about Mono.Security.X509
Sebastien Pouliot
sebastien.pouliot at gmail.com
Mon Jun 16 09:11:05 EDT 2008
Hello David,
General note: Please review the Mono coding guidelines.
http://www.mono-project.com/Coding_Guidelines
E.g. use tabs not spaces, add a space before '('
> diff -urpN mono-1.9.1-old/mcs/class/Mono.Security/Mono.Security_test.dll.sources mono-1.9.1/mcs/class/Mono.Security/Mono.Security_test.dll.sources
> --- mono-1.9.1-old/mcs/class/Mono.Security/Mono.Security_test.dll.sources 2007-11-08 17:34:20.000000000 -0500
> +++ mono-1.9.1/mcs/class/Mono.Security/Mono.Security_test.dll.sources 2008-06-13 01:58:07.000000000 -0400
> @@ -40,3 +40,4 @@ Mono.Security.X509/X520AttributesTest.cs
> Mono.Security.X509.Extensions/KeyUsageExtensionTest.cs
> Mono.Security.X509.Extensions/ExtendedKeyUsageExtensionTest.cs
> Mono.Security.X509.Extensions/BasicConstraintsExtensionTest.cs
> +Mono.Security.X509.Extensions/SubjectAltNameTest.cs
> diff -urpN mono-1.9.1-old/mcs/class/Mono.Security/Mono.Security.X509/ChangeLog mono-1.9.1/mcs/class/Mono.Security/Mono.Security.X509/ChangeLog
> --- mono-1.9.1-old/mcs/class/Mono.Security/Mono.Security.X509/ChangeLog 2007-11-08 17:34:18.000000000 -0500
> +++ mono-1.9.1/mcs/class/Mono.Security/Mono.Security.X509/ChangeLog 2008-06-13 02:02:37.000000000 -0400
> @@ -1,3 +1,8 @@
> +2008-06-13 David Wolinsky <davidiw at ufl.edu>
> +
> + * X509CertificateBuilder.cs: Extensions is now writable so that it
> + can be added unto during run time.
> +
> 2007-05-09 Sebastien Pouliot <sebastien at ximian.com>
>
> * PKCS12.cs: Adds SecretBag support. Patch by Jay Miller.
> diff -urpN mono-1.9.1-old/mcs/class/Mono.Security/Mono.Security.X509.Extensions/ChangeLog mono-1.9.1/mcs/class/Mono.Security/Mono.Security.X509.Extensions/ChangeLog
> --- mono-1.9.1-old/mcs/class/Mono.Security/Mono.Security.X509.Extensions/ChangeLog 2008-01-29 17:05:09.000000000 -0500
> +++ mono-1.9.1/mcs/class/Mono.Security/Mono.Security.X509.Extensions/ChangeLog 2008-06-13 02:01:27.000000000 -0400
> @@ -1,3 +1,9 @@
> +2008-06-13 David Wolinsky <davidiw at ufl.edu>
> + * SubjectAltName.cs: IP Addresses are handled and now this class
> + can be generated via the constructor from arrays.
> + * GeneralNames.cs: Added support to generate GeneralNames from
> + an arrays of strings.
> +
> 2007-12-14 Sebastien Pouliot <sebastien at ximian.com>
>
> * AuthorityKeyIdentifierExtension.cs: Don't throw on what we don't
> diff -urpN mono-1.9.1-old/mcs/class/Mono.Security/Mono.Security.X509.Extensions/GeneralNames.cs mono-1.9.1/mcs/class/Mono.Security/Mono.Security.X509.Extensions/GeneralNames.cs
> --- mono-1.9.1-old/mcs/class/Mono.Security/Mono.Security.X509.Extensions/GeneralNames.cs 2007-11-08 17:34:17.000000000 -0500
> +++ mono-1.9.1/mcs/class/Mono.Security/Mono.Security.X509.Extensions/GeneralNames.cs 2008-06-13 01:58:52.000000000 -0400
> @@ -72,11 +72,52 @@ namespace Mono.Security.X509.Extensions
> private ArrayList directoryNames;
> private ArrayList uris;
> private ArrayList ipAddr;
> + private ASN1 asn;
> +
>
> public GeneralNames ()
> {
> }
>
> + public GeneralNames(ArrayList rfc822s, ArrayList dnsNames, ArrayList ipAddresses, ArrayList uris)
let's change the ArrayList into string[]...
> + {
> + // This is an extension
> + asn = new ASN1 ( 0x30);
> +
> + if(rfc822s != null) {
> + foreach(string rfc822 in rfc822s) {
> + asn.Add( new ASN1 (0x81, Encoding.ASCII.GetBytes (rfc822)));
> + }
> + rfc822Name = rfc822s;
... since it fix the problem here, i.e. this is a reference, not a copy
(that the caller could change after the call making the internal state
inconsistent).
> + }
> +
> + if(dnsNames != null) {
the easy fix is to:
dnsName = new ArrayList ();
> + foreach(string dnsname in dnsNames) {
> + asn.Add( new ASN1 (0x82, Encoding.ASCII.GetBytes (dnsname)));
dnsName.Add (dnsname);
> + }
> + dnsName = dnsNames;
(and here)
> + }
> +
> + if(ipAddresses != null) {
> + foreach(string ipaddress in ipAddresses) {
> + string[] parts = ipaddress.Split('.', ':');
> + byte[] bytes = new byte[parts.Length];
> + for(int i = 0; i < parts.Length; i++) {
> + bytes[i] = Byte.Parse(parts[i]);
> + }
> + asn.Add( new ASN1 (0x87, bytes));
> + }
> + ipAddr = ipAddresses;
(and here)
> + }
> +
> + if(uris != null) {
> + foreach(string uri in uris) {
> + asn.Add( new ASN1 (0x86, Encoding.ASCII.GetBytes (uri)));
> + }
> + this.uris = uris;
(and here)
> + }
> + }
> +
> public GeneralNames (ASN1 sequence)
> {
> for (int i = 0; i < sequence.Count; i++) {
> @@ -103,9 +144,16 @@ namespace Mono.Security.X509.Extensions
> uris.Add (Encoding.ASCII.GetString (sequence[i].Value));
> break;
> case 0x87: // iPAddress [7] OCTET STRING
> + // Todo this is not correct!
??? please provide some additional details
> if (ipAddr == null)
> ipAddr = new ArrayList ();
> - // TODO - Must find sample certificates
> + byte[] bytes = sequence[i].Value;
> + string space = (bytes.Length == 4) ? "." : ":";
> + string res = string.Empty;
> + for (int j = 0; j < bytes.Length; j++) {
> + res += bytes[j] + ((j < bytes.Length - 1) ? space : string.Empty);
> + }
> + ipAddr.Add ( res);
> break;
> default:
> break;
> @@ -145,7 +193,6 @@ namespace Mono.Security.X509.Extensions
> }
> }
>
> - // Incomplete support
> public string[] IPAddresses {
> get {
> if (ipAddr == null)
> @@ -154,6 +201,11 @@ namespace Mono.Security.X509.Extensions
> }
> }
>
> + public byte[] GetBytes()
> + {
> + return asn.GetBytes();
> + }
> +
> public override string ToString ()
> {
> StringBuilder sb = new StringBuilder ();
> diff -urpN mono-1.9.1-old/mcs/class/Mono.Security/Mono.Security.X509.Extensions/SubjectAltNameExtension.cs mono-1.9.1/mcs/class/Mono.Security/Mono.Security.X509.Extensions/SubjectAltNameExtension.cs
> --- mono-1.9.1-old/mcs/class/Mono.Security/Mono.Security.X509.Extensions/SubjectAltNameExtension.cs 2007-11-08 17:34:17.000000000 -0500
> +++ mono-1.9.1/mcs/class/Mono.Security/Mono.Security.X509.Extensions/SubjectAltNameExtension.cs 2008-06-13 01:58:52.000000000 -0400
> @@ -69,7 +69,6 @@ namespace Mono.Security.X509.Extensions
>
> // TODO - incomplete (only rfc822Name, dNSName are supported)
> public class SubjectAltNameExtension : X509Extension {
> -
don't add or remove empty lines from the source, keep the patch as small
as possible ;-)
> private GeneralNames _names;
>
> public SubjectAltNameExtension ()
> @@ -88,6 +87,16 @@ namespace Mono.Security.X509.Extensions
> {
> }
>
> + public SubjectAltNameExtension (ArrayList rfc822, ArrayList dnsNames,
> + ArrayList ipAddresses, ArrayList uris)
> + {
> + _names = new GeneralNames(rfc822, dnsNames, ipAddresses, uris);
> + // 0x04 for string decoding and then the General Names!
> + extnValue = new ASN1( 0x04, _names.GetBytes());
> + extnOid = "2.5.29.17";
> + // extnCritical = true;
> + }
> +
> protected override void Decode ()
> {
> ASN1 sequence = new ASN1 (extnValue.Value);
> @@ -113,6 +122,10 @@ namespace Mono.Security.X509.Extensions
> get { return _names.IPAddresses; }
> }
>
> + public string[] UniformResourceIdentifiers {
> + get { return _names.UniformResourceIdentifiers; }
> + }
> +
> public override string ToString ()
> {
> return _names.ToString ();
> diff -urpN mono-1.9.1-old/mcs/class/Mono.Security/Test/Mono.Security.X509.Extensions/SubjectAltNameTest.cs mono-1.9.1/mcs/class/Mono.Security/Test/Mono.Security.X509.Extensions/SubjectAltNameTest.cs
> --- mono-1.9.1-old/mcs/class/Mono.Security/Test/Mono.Security.X509.Extensions/SubjectAltNameTest.cs 1969-12-31 19:00:00.000000000 -0500
> +++ mono-1.9.1/mcs/class/Mono.Security/Test/Mono.Security.X509.Extensions/SubjectAltNameTest.cs 2008-06-13 01:58:31.000000000 -0400
> @@ -0,0 +1,66 @@
> +//
> +// SubjectAltNameTest.cs - NUnit Test Cases for
> +// Mono.Security.X509.Extensions.SubjectAltName
> +//
> +// Authors:
> +// David Wolinsky <davidiw at ufl.edu>
> +//
> +// Copyright (C) 2008
> +//
Please add the license to new files
> +
> +using System;
> +using System.Collections;
> +using System.Security.Cryptography;
> +using System.IO;
> +
> +using Mono.Security;
> +using Mono.Security.X509;
> +using Mono.Security.X509.Extensions;
> +
> +using NUnit.Framework;
> +
> +namespace MonoTests.Mono.Security.X509.Extensions {
> +
> + [TestFixture]
> + public class SubjectAltNameTest {
> +
> + [Test]
> + public void SubjectAltNameGenerator ()
> + {
> + RSACryptoServiceProvider rsa = new RSACryptoServiceProvider();
> + X509CertificateBuilder x509 = new X509CertificateBuilder();
> + x509.IssuerName = "C=ZA, ST=Western Cape, L=Cape Town, O=Thawte Consulting cc, OU=Certification Services Division, CN=Thawte Server";
> + x509.NotAfter = DateTime.MaxValue;
> + x509.NotBefore = DateTime.MinValue;
> + x509.SubjectName = "C=US, ST=Maryland, L=Pasadena, O=Brent Baccala, OU=FreeSoft, CN=www.freesoft.org";
> + x509.SerialNumber = new byte[]{12, 34, 56, 78, 90};
> + x509.Version = 3;
> + x509.SubjectPublicKey = rsa;
> +
> + ArrayList dns = new ArrayList();
> + dns.Add("one");
> + dns.Add("two");
> + ArrayList uris = new ArrayList();
> + uris.Add("one:two://three");
> + uris.Add("Here:I:AM://12345");
> + uris.Add("last:one");
> + SubjectAltNameExtension sane = new SubjectAltNameExtension(new ArrayList(0), dns, new ArrayList(0), uris);
> + x509.Extensions = new X509ExtensionCollection();
> + x509.Extensions.Add(sane);
> + byte[] data = x509.Sign(rsa);
> +
> + X509Certificate x509_loaded = new X509Certificate(data);
> + SubjectAltNameExtension sane_test = null;
> + try {
> + sane_test = new SubjectAltNameExtension(x509_loaded.Extensions[0]);
> + } catch { }
??? why the catch, if it fails it will be reported by nunit
> + Assert.IsTrue(sane_test != null, "Unable to get SubjectAltNameExtension from Extensions!");
not needed without the catch
> + Assert.AreEqual(sane_test.RFC822.Length, 0, "RFC822 count");
> + Assert.AreEqual(sane_test.DNSNames.Length, 2, "DNSName count");
> + Assert.AreEqual(sane_test.IPAddresses.Length, 0, "IPAddresses count");
> + Assert.AreEqual(sane_test.UniformResourceIdentifiers.Length, 3, "URI Count");
> + Assert.AreEqual(sane_test.DNSNames[1], "two", "DNSName test");
> + Assert.AreEqual(sane_test.UniformResourceIdentifiers[2], "last:one", "URI Test");
> + }
> + }
> +}
The test does a roundtrip which is great but incomplete. If the
certificate generation is bad, and the parsing code adjusted to it, then
the test won't fail. To avoid this you should embed a, 3rd party (e.g.
openssl), certificate that use the extension and parse it.
Thanks,
Sebastien
More information about the Mono-devel-list
mailing list