[libvirt] [PATCH 0/3] Add proper validation of IP/MAC addresses to network.rng
I needed to add more elements to network.rng, and modify the possible values for some existing elements, but noticed that there was currently no validation of IP addresses or MAC addresses. As a first stpe in my modifications, I'm bringing network.rng's validation/format more in line with what's in, eg, interface.rng. I'll be adding the new elements later, in a complete patcheset to add IPv6 support, but since these patches stand on their own, I figured I'd get them out of the way first. The first patch puts the entire definition of network inside a <grammer>, which seemed like the right thing to do in order to be able to define refs for ipv4-addr/mac-addr outside the element def itself. I copied the way that it's done in other libvirt RNG files (not sure why this one wasn't like that already). Although this changed the nesting of almost the entire file, I left the indentation of existing lines as-is so the actual change wouldn't be obscured. The 2nd patch looks very long and confused, but it actually is just whitespace changes (as verified by diff -b). The 3rd patch adds the refs for ipv4-addr & mac-addr, and makes use of them. networkschematest continues to succeed after these changes, so it looks like it's all correct.
All the other RNG files in libvirt are enclosed within <grammer>. This commit makes the syntactical changes necessary to make network.rng fit that pattern. (This is the first step in adding some data type definitions to network.rng for more exact validation of IP and MAC addresses). Formatting changes (indentation) will be done in a subsequent commit, so that actual changes to the code won't be obscured by whitespace. --- docs/schemas/network.rng | 11 ++++++++++- 1 files changed, 10 insertions(+), 1 deletions(-) diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 33994bc..d898b77 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -1,6 +1,13 @@ <!-- A Relax NG schema for the libvirt network XML format --> -<element name="network" xmlns="http://relaxng.org/ns/structure/1.0" +<grammar xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> + <start> + <ref name="network"/> + </start> + +<define name="network"> + +<element name="network"> <interleave> <!-- The name of the network, used to refer to it through the API @@ -119,3 +126,5 @@ </optional> </interleave> </element> +</define> +</grammar> -- 1.7.2.3
On 11/11/2010 03:41 PM, Laine Stump wrote:
All the other RNG files in libvirt are enclosed within<grammer>. This commit makes the syntactical changes necessary to make network.rng fit that pattern. (This is the first step in adding some data type definitions to network.rng for more exact validation of IP and MAC addresses).
Formatting changes (indentation) will be done in a subsequent commit, so that actual changes to the code won't be obscured by whitespace. --- docs/schemas/network.rng | 11 ++++++++++- 1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 33994bc..d898b77 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -1,6 +1,13 @@ <!-- A Relax NG schema for the libvirt network XML format --> -<element name="network" xmlns="http://relaxng.org/ns/structure/1.0" +<grammar xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> +<start> +<ref name="network"/> +</start> + +<define name="network"> + +<element name="network"> <interleave>
<!-- The name of the network, used to refer to it through the API @@ -119,3 +126,5 @@ </optional> </interleave> </element> +</define> +</grammar> ACK.
Stefan
On 11/11/2010 03:56 PM, Stefan Berger wrote:
On 11/11/2010 03:41 PM, Laine Stump wrote:
All the other RNG files in libvirt are enclosed within<grammer>. This commit makes the syntactical changes necessary to make network.rng fit that pattern. (This is the first step in adding some data type definitions to network.rng for more exact validation of IP and MAC addresses).
Formatting changes (indentation) will be done in a subsequent commit, so that actual changes to the code won't be obscured by whitespace. --- docs/schemas/network.rng | 11 ++++++++++- 1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 33994bc..d898b77 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -1,6 +1,13 @@ <!-- A Relax NG schema for the libvirt network XML format --> -<element name="network" xmlns="http://relaxng.org/ns/structure/1.0" +<grammar xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> +<start> +<ref name="network"/> +</start> + +<define name="network"> + +<element name="network"> <interleave>
<!-- The name of the network, used to refer to it through the API @@ -119,3 +126,5 @@ </optional> </interleave> </element> +</define> +</grammar> ACK.
Stefan
Pushed, after fixing the typo in the commit message pointed out by Dave. Thanks!
On Thu, Nov 11, 2010 at 03:41:27PM -0500, Laine Stump wrote:
All the other RNG files in libvirt are enclosed within <grammer>. This commit makes the syntactical changes necessary to make network.rng fit that pattern. (This is the first step in adding some data type definitions to network.rng for more exact validation of IP and MAC addresses).
Formatting changes (indentation) will be done in a subsequent commit, so that actual changes to the code won't be obscured by whitespace. --- docs/schemas/network.rng | 11 ++++++++++- 1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 33994bc..d898b77 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -1,6 +1,13 @@ <!-- A Relax NG schema for the libvirt network XML format --> -<element name="network" xmlns="http://relaxng.org/ns/structure/1.0" +<grammar xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> + <start> + <ref name="network"/> + </start> + +<define name="network"> + +<element name="network"> <interleave>
<!-- The name of the network, used to refer to it through the API @@ -119,3 +126,5 @@ </optional> </interleave> </element> +</define> +</grammar> -- 1.7.2.3
--
The name of the tag <grammar> is correct in the patch, but to avoid confusion, you should fix it in the commit message. Dave
This commit is whitespace changes only, do avoid obscuring actual code changes. --- docs/schemas/network.rng | 206 +++++++++++++++++++++++----------------------- 1 files changed, 103 insertions(+), 103 deletions(-) diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index d898b77..ca100b7 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -5,126 +5,126 @@ <ref name="network"/> </start> -<define name="network"> + <define name="network"> -<element name="network"> - <interleave> + <element name="network"> + <interleave> - <!-- The name of the network, used to refer to it through the API - and in virsh --> - <element name="name"> - <text/> - </element> - - <!-- <uuid> element --> - <optional> - <element name="uuid"><text/></element> - </optional> - - <!-- <bridge> element --> - <optional> - <!-- The name of the network to be set up; this will back - the network on the host --> - <element name="bridge"> - <optional> - <attribute name="name"> + <!-- The name of the network, used to refer to it through the API + and in virsh --> + <element name="name"> <text/> - </attribute> - </optional> + </element> - <optional> - <attribute name="stp"> - <choice> - <value>on</value> - <value>off</value> - </choice> - </attribute> - </optional> + <!-- <uuid> element --> + <optional> + <element name="uuid"><text/></element> + </optional> - <optional> - <attribute name="delay"> - <data type="integer"/> - </attribute> - </optional> + <!-- <bridge> element --> + <optional> + <!-- The name of the network to be set up; this will back + the network on the host --> + <element name="bridge"> + <optional> + <attribute name="name"> + <text/> + </attribute> + </optional> - </element> - </optional> + <optional> + <attribute name="stp"> + <choice> + <value>on</value> + <value>off</value> + </choice> + </attribute> + </optional> - <!-- <forward> element --> - <optional> - <!-- The device through which the bridge is connected to the - rest of the network --> - <element name="forward"> - <optional> - <attribute name="dev"> - <text/> - </attribute> - </optional> + <optional> + <attribute name="delay"> + <data type="integer"/> + </attribute> + </optional> - <optional> - <attribute name="mode"> - <choice> - <value>nat</value> - <value>route</value> - </choice> - </attribute> - </optional> - </element> - </optional> + </element> + </optional> - <!-- <domain> element --> - <optional> - <element name="domain"> - <attribute name="name"><text/></attribute> - </element> - </optional> + <!-- <forward> element --> + <optional> + <!-- The device through which the bridge is connected to the + rest of the network --> + <element name="forward"> + <optional> + <attribute name="dev"> + <text/> + </attribute> + </optional> - <!-- <ip> element --> - <optional> - <!-- The IP element sets up NAT'ing and an optional DHCP server - local to the host. --> - <!-- FIXME: address, netmask and the start and end of the ranges - are IP addresses, and should be validated as such in the scheme --> - <element name="ip"> - <optional> - <attribute name="address"><text/></attribute> - </optional> - <optional> - <attribute name="netmask"><text/></attribute> - </optional> - <optional> - <element name="tftp"> - <attribute name="root"><text/></attribute> - </element> - </optional> - <!-- Define the range(s) of IP addresses that the DHCP - server should hand out --> - <element name="dhcp"> - <zeroOrMore> - <element name="range"> - <attribute name="start"><text/></attribute> - <attribute name="end"><text/></attribute> + <optional> + <attribute name="mode"> + <choice> + <value>nat</value> + <value>route</value> + </choice> + </attribute> + </optional> </element> - </zeroOrMore> - <zeroOrMore> - <element name="host"> - <attribute name="mac"><text/></attribute> + </optional> + + <!-- <domain> element --> + <optional> + <element name="domain"> <attribute name="name"><text/></attribute> - <attribute name="ip"><text/></attribute> </element> - </zeroOrMore> + </optional> + + <!-- <ip> element --> <optional> - <element name="bootp"> - <attribute name="file"><text/></attribute> + <!-- The IP element sets up NAT'ing and an optional DHCP server + local to the host. --> + <!-- FIXME: address, netmask and the start and end of the ranges + are IP addresses, and should be validated as such in the scheme --> + <element name="ip"> + <optional> + <attribute name="address"><text/></attribute> + </optional> + <optional> + <attribute name="netmask"><text/></attribute> + </optional> <optional> - <attribute name="server"><text/></attribute> + <element name="tftp"> + <attribute name="root"><text/></attribute> + </element> </optional> + <!-- Define the range(s) of IP addresses that the DHCP + server should hand out --> + <element name="dhcp"> + <zeroOrMore> + <element name="range"> + <attribute name="start"><text/></attribute> + <attribute name="end"><text/></attribute> + </element> + </zeroOrMore> + <zeroOrMore> + <element name="host"> + <attribute name="mac"><text/></attribute> + <attribute name="name"><text/></attribute> + <attribute name="ip"><text/></attribute> + </element> + </zeroOrMore> + <optional> + <element name="bootp"> + <attribute name="file"><text/></attribute> + <optional> + <attribute name="server"><text/></attribute> + </optional> + </element> + </optional> + </element> </element> </optional> - </element> + </interleave> </element> - </optional> - </interleave> -</element> -</define> + </define> </grammar> -- 1.7.2.3
On 11/11/2010 03:41 PM, Laine Stump wrote:
This commit is whitespace changes only, do avoid obscuring actual code changes. I trust you on this. ACK.
--- docs/schemas/network.rng | 206 +++++++++++++++++++++++----------------------- 1 files changed, 103 insertions(+), 103 deletions(-)
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index d898b77..ca100b7 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -5,126 +5,126 @@ <ref name="network"/> </start>
-<define name="network"> +<define name="network">
-<element name="network"> -<interleave> +<element name="network"> +<interleave>
-<!-- The name of the network, used to refer to it through the API - and in virsh --> -<element name="name"> -<text/> -</element> - -<!--<uuid> element --> -<optional> -<element name="uuid"><text/></element> -</optional> - -<!--<bridge> element --> -<optional> -<!-- The name of the network to be set up; this will back - the network on the host --> -<element name="bridge"> -<optional> -<attribute name="name"> +<!-- The name of the network, used to refer to it through the API + and in virsh --> +<element name="name"> <text/> -</attribute> -</optional> +</element>
-<optional> -<attribute name="stp"> -<choice> -<value>on</value> -<value>off</value> -</choice> -</attribute> -</optional> +<!--<uuid> element --> +<optional> +<element name="uuid"><text/></element> +</optional>
-<optional> -<attribute name="delay"> -<data type="integer"/> -</attribute> -</optional> +<!--<bridge> element --> +<optional> +<!-- The name of the network to be set up; this will back + the network on the host --> +<element name="bridge"> +<optional> +<attribute name="name"> +<text/> +</attribute> +</optional>
-</element> -</optional> +<optional> +<attribute name="stp"> +<choice> +<value>on</value> +<value>off</value> +</choice> +</attribute> +</optional>
-<!--<forward> element --> -<optional> -<!-- The device through which the bridge is connected to the - rest of the network --> -<element name="forward"> -<optional> -<attribute name="dev"> -<text/> -</attribute> -</optional> +<optional> +<attribute name="delay"> +<data type="integer"/> +</attribute> +</optional>
-<optional> -<attribute name="mode"> -<choice> -<value>nat</value> -<value>route</value> -</choice> -</attribute> -</optional> -</element> -</optional> +</element> +</optional>
-<!--<domain> element --> -<optional> -<element name="domain"> -<attribute name="name"><text/></attribute> -</element> -</optional> +<!--<forward> element --> +<optional> +<!-- The device through which the bridge is connected to the + rest of the network --> +<element name="forward"> +<optional> +<attribute name="dev"> +<text/> +</attribute> +</optional>
-<!--<ip> element --> -<optional> -<!-- The IP element sets up NAT'ing and an optional DHCP server - local to the host. --> -<!-- FIXME: address, netmask and the start and end of the ranges - are IP addresses, and should be validated as such in the scheme --> -<element name="ip"> -<optional> -<attribute name="address"><text/></attribute> -</optional> -<optional> -<attribute name="netmask"><text/></attribute> -</optional> -<optional> -<element name="tftp"> -<attribute name="root"><text/></attribute> -</element> -</optional> -<!-- Define the range(s) of IP addresses that the DHCP - server should hand out --> -<element name="dhcp"> -<zeroOrMore> -<element name="range"> -<attribute name="start"><text/></attribute> -<attribute name="end"><text/></attribute> +<optional> +<attribute name="mode"> +<choice> +<value>nat</value> +<value>route</value> +</choice> +</attribute> +</optional> </element> -</zeroOrMore> -<zeroOrMore> -<element name="host"> -<attribute name="mac"><text/></attribute> +</optional> + +<!--<domain> element --> +<optional> +<element name="domain"> <attribute name="name"><text/></attribute> -<attribute name="ip"><text/></attribute> </element> -</zeroOrMore> +</optional> + +<!--<ip> element --> <optional> -<element name="bootp"> -<attribute name="file"><text/></attribute> +<!-- The IP element sets up NAT'ing and an optional DHCP server + local to the host. --> +<!-- FIXME: address, netmask and the start and end of the ranges + are IP addresses, and should be validated as such in the scheme --> +<element name="ip"> +<optional> +<attribute name="address"><text/></attribute> +</optional> +<optional> +<attribute name="netmask"><text/></attribute> +</optional> <optional> -<attribute name="server"><text/></attribute> +<element name="tftp"> +<attribute name="root"><text/></attribute> +</element> </optional> +<!-- Define the range(s) of IP addresses that the DHCP + server should hand out --> +<element name="dhcp"> +<zeroOrMore> +<element name="range"> +<attribute name="start"><text/></attribute> +<attribute name="end"><text/></attribute> +</element> +</zeroOrMore> +<zeroOrMore> +<element name="host"> +<attribute name="mac"><text/></attribute> +<attribute name="name"><text/></attribute> +<attribute name="ip"><text/></attribute> +</element> +</zeroOrMore> +<optional> +<element name="bootp"> +<attribute name="file"><text/></attribute> +<optional> +<attribute name="server"><text/></attribute> +</optional> +</element> +</optional> +</element> </element> </optional> -</element> +</interleave> </element> -</optional> -</interleave> -</element> -</define> +</define> </grammar>
On 11/11/2010 03:53 PM, Stefan Berger wrote:
On 11/11/2010 03:41 PM, Laine Stump wrote:
This commit is whitespace changes only, do avoid obscuring actual code changes. I trust you on this. ACK.
Thanks for the vote of confidence :-) Pushed.
IP addresses and MAC addresses had been defined in the RNG simply as <text/> meaning that, according to the RNG, any string could go in there. Of course the C parsing code does a much better job of validating, but we may as well have this describing the contents accurately (even though it's currently only used during "make check"). --- docs/schemas/network.rng | 27 ++++++++++++++++++++------- 1 files changed, 20 insertions(+), 7 deletions(-) diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index ca100b7..aa98997 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -83,14 +83,12 @@ <optional> <!-- The IP element sets up NAT'ing and an optional DHCP server local to the host. --> - <!-- FIXME: address, netmask and the start and end of the ranges - are IP addresses, and should be validated as such in the scheme --> <element name="ip"> <optional> - <attribute name="address"><text/></attribute> + <attribute name="address"><ref name="ipv4-addr"/></attribute> </optional> <optional> - <attribute name="netmask"><text/></attribute> + <attribute name="netmask"><ref name="ipv4-addr"/></attribute> </optional> <optional> <element name="tftp"> @@ -102,13 +100,13 @@ <element name="dhcp"> <zeroOrMore> <element name="range"> - <attribute name="start"><text/></attribute> - <attribute name="end"><text/></attribute> + <attribute name="start"><ref name="ipv4-addr"/></attribute> + <attribute name="end"><ref name="ipv4-addr"/></attribute> </element> </zeroOrMore> <zeroOrMore> <element name="host"> - <attribute name="mac"><text/></attribute> + <attribute name="mac"><ref name="mac-addr"/></attribute> <attribute name="name"><text/></attribute> <attribute name="ip"><text/></attribute> </element> @@ -127,4 +125,19 @@ </interleave> </element> </define> + + <!-- An ipv4 "dotted quad" address --> + <define name='ipv4-addr'> + <data type='string'> + <param name="pattern">(((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))\.){3}((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))</param> + </data> + </define> + + <!-- a 6 byte MAC address in ASCII-hex format, eg "12:34:56:78:9A:BC" --> + <define name='mac-addr'> + <data type='string'> + <param name="pattern">([a-fA-F0-9]{2}:){5}[a-fA-F0-9]{2}</param> + </data> + </define> + </grammar> -- 1.7.2.3
On 11/11/2010 03:41 PM, Laine Stump wrote:
IP addresses and MAC addresses had been defined in the RNG simply as <text/> meaning that, according to the RNG, any string could go in there. Of course the C parsing code does a much better job of validating, but we may as well have this describing the contents accurately (even though it's currently only used during "make check"). --- docs/schemas/network.rng | 27 ++++++++++++++++++++------- 1 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index ca100b7..aa98997 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -83,14 +83,12 @@ <optional> <!-- The IP element sets up NAT'ing and an optional DHCP server local to the host. --> -<!-- FIXME: address, netmask and the start and end of the ranges - are IP addresses, and should be validated as such in the scheme --> <element name="ip"> <optional> -<attribute name="address"><text/></attribute> +<attribute name="address"><ref name="ipv4-addr"/></attribute> </optional> <optional> -<attribute name="netmask"><text/></attribute> +<attribute name="netmask"><ref name="ipv4-addr"/></attribute> </optional> <optional> <element name="tftp"> @@ -102,13 +100,13 @@ <element name="dhcp"> <zeroOrMore> <element name="range"> -<attribute name="start"><text/></attribute> -<attribute name="end"><text/></attribute> +<attribute name="start"><ref name="ipv4-addr"/></attribute> +<attribute name="end"><ref name="ipv4-addr"/></attribute> </element> </zeroOrMore> <zeroOrMore> <element name="host"> -<attribute name="mac"><text/></attribute> +<attribute name="mac"><ref name="mac-addr"/></attribute> <attribute name="name"><text/></attribute> <attribute name="ip"><text/></attribute> </element> @@ -127,4 +125,19 @@ </interleave> </element> </define> + +<!-- An ipv4 "dotted quad" address --> +<define name='ipv4-addr'> +<data type='string'> +<param name="pattern">(((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))\.){3}((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))</param> +</data> +</define> + +<!-- a 6 byte MAC address in ASCII-hex format, eg "12:34:56:78:9A:BC" --> +<define name='mac-addr'> +<data type='string'> +<param name="pattern">([a-fA-F0-9]{2}:){5}[a-fA-F0-9]{2}</param> +</data> +</define> + </grammar>
ACK. [would have put (2[0-4][0-9]) before (1[0-9]{2})] Stefan
On 11/11/2010 01:41 PM, Laine Stump wrote:
+ <!-- An ipv4 "dotted quad" address --> + <define name='ipv4-addr'> + <data type='string'> + <param name="pattern">(((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))\.){3}((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))</param>
This allows 01.1.1.1 (leading zero looks unusual in an IPv4 address). I would have done something like: (((25[0-5])|(2[0-4][0-9])|(1[0-9][0-9])|([1-9][0-9])|[0-9])\.){3}... -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 11/11/2010 04:05 PM, Eric Blake wrote:
On 11/11/2010 01:41 PM, Laine Stump wrote:
+<!-- An ipv4 "dotted quad" address --> +<define name='ipv4-addr'> +<data type='string'> +<param name="pattern">(((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))\.){3}((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))</param> This allows 01.1.1.1 (leading zero looks unusual in an IPv4 address). I would have done something like:
(((25[0-5])|(2[0-4][0-9])|(1[0-9][0-9])|([1-9][0-9])|[0-9])\.){3}...
I see what you mean. How about if I squash in the attached delta-diff? (I'll send a patch for the same regex in interface.rng, the source of this one, separately).
On 11/11/2010 02:57 PM, Laine Stump wrote:
name="pattern">(((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))\.){3}((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))</param>
This allows 01.1.1.1 (leading zero looks unusual in an IPv4 address). I would have done something like:
How about if I squash in the attached delta-diff?
Works for me. ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 11/11/2010 05:07 PM, Eric Blake wrote:
On 11/11/2010 02:57 PM, Laine Stump wrote:
name="pattern">(((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))\.){3}((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2}))</param>
This allows 01.1.1.1 (leading zero looks unusual in an IPv4 address). I would have done something like:
How about if I squash in the attached delta-diff? Works for me.
ACK.
Okay. I pushed the original patch with the delta squashed in (after verifying make check still passes). Thanks!
participants (4)
-
Dave Allan -
Eric Blake -
Laine Stump -
Stefan Berger