Repository : http://git.fedorahosted.org/git/?p=secure-coding.git On branch : master >--------------------------------------------------------------- commit 8341df782d8f94f60539c0a2d3682ab843bb22f4 Merge: 7ada5ed 13faeec Author: Eric Christensen <echriste@xxxxxxxxxx> Date: Wed May 28 11:00:16 2014 -0400 Merge branch 'master' of git+ssh://git.fedorahosted.org/git/secure-coding >--------------------------------------------------------------- defensive-coding/Makefile | 2 +- defensive-coding/en-US/C-Allocators.xml | 7 +- defensive-coding/en-US/Defensive_Coding.xml | 3 + defensive-coding/en-US/Features-TLS.xml | 11 +++ defensive-coding/en-US/Go.xml | 90 ++++++++++++++++++ defensive-coding/en-US/Tasks-Packaging.xml | 131 +++++++++++++++++++++++++++ defensive-coding/en-US/Vala.xml | 53 +++++++++++ defensive-coding/src/.gitignore | 1 + defensive-coding/src/Go-Error_Handling.go | 48 ++++++++++ defensive-coding/src/src.mk | 6 ++ 10 files changed, 348 insertions(+), 4 deletions(-) diff --git a/defensive-coding/Makefile b/defensive-coding/Makefile index 6220afc..2090dad 100644 --- a/defensive-coding/Makefile +++ b/defensive-coding/Makefile @@ -9,7 +9,7 @@ build: build-src build-manual build-snippets: mkdir -p en-US/snippets python scripts/split-snippets.py . \ - src/*.c src/*.cpp src/*.java src/*.py + src/*.c src/*.cpp src/*.java src/*.py src/*.go build-manual: build-snippets publican build --formats=html,epub,pdf --langs=en-US diff --git a/defensive-coding/en-US/C-Allocators.xml b/defensive-coding/en-US/C-Allocators.xml index 1bff610..87d2682 100644 --- a/defensive-coding/en-US/C-Allocators.xml +++ b/defensive-coding/en-US/C-Allocators.xml @@ -29,7 +29,7 @@ realloc(ptr, size);</literal> is wrong because the memory pointed to by <literal>ptr</literal> leaks in case of an error. </para> - <section> + <section id="sect-Defensive_Coding-C-Use-After-Free"> <title>Use-after-free errors</title> <para> After <function>free</function>, the pointer is invalid. @@ -140,7 +140,7 @@ </para> </section> - <section> + <section id="sect-Defensive_Coding-C-Allocators-Custom"> <title>Custom memory allocators</title> <para> Custom memory allocates come in two forms: replacements for @@ -176,7 +176,8 @@ allocators. In micro-benchmarks, pool allocators can show huge wins, and size-specific pools can reduce internal fragmentation. But often, utilization of individual pools - is poor, and + is poor, and external fragmentation increases the overall + memory usage. </para> </listitem> </itemizedlist> diff --git a/defensive-coding/en-US/Defensive_Coding.xml b/defensive-coding/en-US/Defensive_Coding.xml index b8ca3de..ee96c8d 100644 --- a/defensive-coding/en-US/Defensive_Coding.xml +++ b/defensive-coding/en-US/Defensive_Coding.xml @@ -8,6 +8,8 @@ <xi:include href="CXX.xml" xmlns:xi="http://www.w3.org/2001/XInclude"; /> <xi:include href="Java.xml" xmlns:xi="http://www.w3.org/2001/XInclude"; /> <xi:include href="Python.xml" xmlns:xi="http://www.w3.org/2001/XInclude"; /> + <xi:include href="Go.xml" xmlns:xi="http://www.w3.org/2001/XInclude"; /> + <xi:include href="Vala.xml" xmlns:xi="http://www.w3.org/2001/XInclude"; /> </part> <part> <title>Specific Programming Tasks</title> @@ -18,6 +20,7 @@ <xi:include href="Tasks-Processes.xml" xmlns:xi="http://www.w3.org/2001/XInclude"; /> <xi:include href="Tasks-Serialization.xml" xmlns:xi="http://www.w3.org/2001/XInclude"; /> <xi:include href="Tasks-Cryptography.xml" xmlns:xi="http://www.w3.org/2001/XInclude"; /> + <xi:include href="Tasks-Packaging.xml" xmlns:xi="http://www.w3.org/2001/XInclude"; /> </part> <part> <title>Implementing Security Features</title> diff --git a/defensive-coding/en-US/Features-TLS.xml b/defensive-coding/en-US/Features-TLS.xml index 936910d..5d9e39d 100644 --- a/defensive-coding/en-US/Features-TLS.xml +++ b/defensive-coding/en-US/Features-TLS.xml @@ -186,6 +186,17 @@ verify</command> result in an exit status of zero. </para> <para> + OpenSSL command-line commands, such as <command>openssl + genrsa</command>, do not ensure that physical entropy is used + for key generationâ??they obtain entropy from + <filename>/dev/urandom</filename> and other sources, but not + from <filename>/dev/random</filename>. This can result in + weak keys if the system lacks a proper entropy source (e.g., a + virtual machine with solid state storage). Depending on local + policies, keys generated by these OpenSSL tools should not be + used in high-value, critical functions. + </para> + <para> The OpenSSL server and client applications (<command>openssl s_client</command> and <command>openssl s_server</command>) are debugging tools and should <emphasis>never</emphasis> be diff --git a/defensive-coding/en-US/Go.xml b/defensive-coding/en-US/Go.xml new file mode 100644 index 0000000..0e44d5e --- /dev/null +++ b/defensive-coding/en-US/Go.xml @@ -0,0 +1,90 @@ +<?xml version='1.0' encoding='utf-8' ?> +<!DOCTYPE chapter PUBLIC "-//OASIS//DTD DocBook XML V4.5//EN" "http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd"; [ +]> +<chapter id="chap-Defensive_Coding-Go"> +<title>The Go Programming Language</title> +<para> + This chapter contains language-specific recommendations for Go. +</para> +<section id="chap-Defensive_Coding-Go-Memory_Safety"> + <title>Memory safety</title> + <para> + Go provides memory safety, but only if the program is not executed + in parallel (that is, <envar>GOMAXPROCS</envar> is not larger than + <literal>1</literal>). The reason is that interface values and + slices consist of multiple words are not updated atomically. + Another thread of execution can observe an inconsistent pairing + between type information and stored value (for interfaces) or + pointer and length (for slices), and such inconsistency can lead + to a memory safety violation. + </para> + <para> + Code which does not run in parallel and does not use the + <literal>unsafe</literal> package (or other packages which expose + unsafe constructs) is memory-safe. For example, invalid casts and + out-of-range subscripting cause panics and run time. + </para> + <para> + Keep in mind that finalization can introduce parallelism because + finalizers are executed concurrently, potentially interleaved with + the rest of the program. + </para> +</section> +<section id="chap-Defensive_Coding-Go-Error_Handling"> + <title>Error handling</title> + <para> + Only a few common operations (such as pointer dereference, integer + division, array subscripting) trigger exceptions in Go, called + <emphasis>panics</emphasis>. Most interfaces in the standard + library use a separate return value of type + <literal>error</literal> to signal error. + </para> + <para> + Not checking error return values can lead to incorrect operation + and data loss (especially in the case of writes, using interfaces + such as <literal>io.Writer</literal>). + </para> + <para> + The correct way to check error return values depends on the + function or method being called. In the majority of cases, the + first step after calling a function should be an error check + against the <literal>nil</literal> value, handling any encountered + error. See <xref + linkend="ex-Defensive_Coding-Go-Error_Handling-Regular"/> for + details. + </para> + <example id="ex-Defensive_Coding-Go-Error_Handling-Regular"> + <title>Regular error handling in Go</title> + <xi:include href="snippets/Go-Error_Handling-Regular.xml" + xmlns:xi="http://www.w3.org/2001/XInclude"; /> + </example> + <para> + However, with <literal>io.Reader</literal>, + <literal>io.ReaderAt</literal> and related interfaces, it is + necessary to check for a non-zero number of read bytes first, as + shown in <xref + linkend="ex-Defensive_Coding-Go-Error_Handling-IO"/>. If this + pattern is not followed, data loss may occur. This is due to the + fact that the <literal>io.Reader</literal> interface permits + returning both data and an error at the same time. + </para> + <example id="ex-Defensive_Coding-Go-Error_Handling-IO"> + <title>Read error handling in Go</title> + <xi:include href="snippets/Go-Error_Handling-IO.xml" + xmlns:xi="http://www.w3.org/2001/XInclude"; /> + </example> +</section> +<section id="chap-Defensive_Coding-Go-Garbage_Collector"> + <title>Garbage Collector</title> + <para> + Older Go releases (before Go 1.3) use a conservative garbage + collector without blacklisting. This means that data blobs can + cause retention of unrelated data structures because the data is + conservatively interpreted as pointers. This phenomenon can be + triggered accidentally on 32-bit architectures and is more likely + to occur if the heap grows larger. On 64-bit architectures, it + may be possible to trigger it deliberatelyâ??it is unlikely to occur + spontaneously. + </para> +</section> +</chapter> diff --git a/defensive-coding/en-US/Tasks-Packaging.xml b/defensive-coding/en-US/Tasks-Packaging.xml new file mode 100644 index 0000000..95bfbc6 --- /dev/null +++ b/defensive-coding/en-US/Tasks-Packaging.xml @@ -0,0 +1,131 @@ +<?xml version='1.0' encoding='utf-8' ?> +<!DOCTYPE chapter PUBLIC "-//OASIS//DTD DocBook XML V4.5//EN" "http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd"; [ +]> +<chapter id="chap-Defensive_Coding-Tasks-Packaging"> + <title>RPM packaging</title> + <para> + This chapter deals with security-related concerns around RPM + packaging. It has to be read in conjunction with + distribution-specific packaging guidelines. + </para> + <section id="sect-Defensive_Coding-Tasks-Packaging-Certificates"> + <title>Generating X.509 self-signed certificates during + installation</title> + <para> + Some applications need X.509 certificates for authentication + purposes. For example, a single private/public key pair could + be used to define cluster membership, enabling authentication + and encryption of all intra-cluster communication. (Lack of + certification from a CA matters less in such a context.) For + such use, generating the key pair at package installation time + when preparing system images for use in the cluster is + reasonable. For other use cases, it is necessary to generate + the key pair before the service is started for the first time. + </para> + <important> + <para> + The way the key is generated may not be suitable for key + material of critical value. (<command>openssl + genrsa</command> uses, but does not require, entropy from a + physical source of randomness, among other things.) Such keys + should be stored in a hardware security module if possible, + and generated from random bits reserved for this purpose + derived from a non-deterministic physical source. + </para> + </important> + <para> + In the spec file, we define two RPM variables which contain the + names of the files used to store the private and public key, and + the user name for the service: + </para> + <informalexample> + <programlisting language="RPM Spec"> +# Name of the user owning the file with the private key +%define tlsuser %{name} +# Name of the directory which contains the key and certificate files +%define tlsdir %{_sysconfdir}/%{name} +%define tlskey %{tlsdir}/%{name}.key +%define tlscert %{tlsdir}/%{name}.crt + </programlisting> + </informalexample> + <para> + These variables likely need adjustment based on the needs of the + package. + </para> + <para> + Typically, the file with the private key needs to be owned by + the system user which needs to read it, + <literal>%{tlsuser}</literal> (not <literal>root</literal>). In + order to avoid races, if the <emphasis>directory</emphasis> + <literal>%{tlsdir}</literal> is <emphasis>owned by the services + user</emphasis>, you should use the code in <xref + linkend="ex-Defensive_Coding-Packaging-Certificates-Owned"/>. + The invocation of <application>su</application> with the + <option>-s /bin/bash</option> argument is necessary in case the + login shell for the user has been disabled. + </para> + <example id="ex-Defensive_Coding-Packaging-Certificates-Owned"> + <title>Creating a key pair in a user-owned directory</title> + <programlisting language="Bash"> +%post +if [ $1 -eq 1 ] ; then + if ! test -e %{tlskey} ; then + su -s /bin/bash \ + -c "umask 077 && openssl genrsa -out %{tlskey} 2048 2>/dev/null" \ + %{tlsuser} + fi + if ! test -e %{tlscert} ; then + cn="Automatically generated certificate for the %{tlsuser} service" + req_args="-key %{tlskey} -out %{tlscert} -days 7305 -subj \"/CN=$cn/\"" + su -s /bin/bash \ + -c "openssl req -new -x509 -extensions usr_cert $req_args" \ + %{tlsuser} + fi +fi + +%files +%dir %attr(0755,%{tlsuser},%{tlsuser]) %{tlsdir} +%ghost %attr(0600,%{tlsuser},%{tlsuser}) %{tlskey} +%ghost %attr(0644,%{tlsuser},%{tlsuser}) %{tlscert} + </programlisting> + </example> + <para> + If the <emphasis>directory</emphasis> + <literal>%{tlsdir}</literal> <emphasis>is owned by</emphasis> + <literal>root</literal>, use the code in <xref + linkend="ex-Defensive_Coding-Packaging-Certificates-Unowned"/>. + </para> + <example id="ex-Defensive_Coding-Packaging-Certificates-Unowned"> + <title>Creating a key pair in a <literal>root</literal>-owned directory</title> + <programlisting language="Bash"> +%post +if [ $1 -eq 1 ] ; then + if ! test -e %{tlskey} ; then + (umask 077 && openssl genrsa -out %{tlskey} 2048 2>/dev/null) + chown %{tlsuser} %{tlskey} + fi + if ! test -e %{tlscert} ; then + cn="Automatically generated certificate for the %{tlsuser} service" + openssl req -new -x509 -extensions usr_cert \ + -key %{tlskey} -out %{tlscert} -days 7305 -subj "/CN=$cn/" + fi +fi + +%files +%dir %attr(0755,%{root},%{root}]) %{tlsdir} +%ghost %attr(0600,%{tlsuser},%{tlsuser}) %{tlskey} +%ghost %attr(0644,%{root},%{root}) %{tlscert} + </programlisting> + </example> + <para> + In order for this to work, the package which generates the keys + must require the <application>openssl</application> package. If + the user which owns the key file is generated by a different + package, the package generating the certificate must specify a + <literal>Requires(pre):</literal> on the package which creates + the user. This ensures that the user account will exist when it + is needed for the <application>su</application> or + <application>chmod</application> invocation. + </para> + </section> +</chapter> diff --git a/defensive-coding/en-US/Vala.xml b/defensive-coding/en-US/Vala.xml new file mode 100644 index 0000000..3dea943 --- /dev/null +++ b/defensive-coding/en-US/Vala.xml @@ -0,0 +1,53 @@ +<?xml version='1.0' encoding='utf-8' ?> +<!DOCTYPE section PUBLIC "-//OASIS//DTD DocBook XML V4.5//EN" "http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd"; [ +]> +<chapter id="chap-Defensive_Coding-Vala"> +<title>The Vala Programming Language</title> +<para> + Vala is a programming language mainly targeted at GNOME developers. +</para> +<para> + Its syntax is inspired by C# (and thus, indirectly, by Java). But + unlike C# and Java, Vala does not attempt to provide memory safety: + Vala is compiled to C, and the C code is compiled with GCC using + typical compiler flags. Basic operations like integer arithmetic + are directly mapped to C constructs. As a results, the + recommendations in <xref linkend="chap-Defensive_Coding-C"/> apply. +</para> +<para> + In particular, the following Vala language constructs can result in + undefined behavior at run time: +</para> +<itemizedlist> + <listitem> + <para> + Integer arithmetic, as described in <xref + linkend="sect-Defensive_Coding-C-Arithmetic"/>. + </para> + </listitem> + <listitem> + <para> + Pointer arithmetic, string subscripting and the + <literal>substring</literal> method on strings (the + <literal>string</literal> class in the + <literal>glib-2.0</literal> package) are not range-checked. It + is the responsibility of the calling code to ensure that the + arguments being passed are valid. This applies even to cases + (like <literal>substring</literal>) where the implementation + would have range information to check the validity of indexes. + See <xref linkend="sect-Defensive_Coding-C-Pointers"/>. + </para> + </listitem> + <listitem> + <para> + Similarly, Vala only performs garbage collection (through + reference counting) for <literal>GObject</literal> values. For + plain C pointers (such as strings), the programmer has to ensure + that storage is deallocated once it is no longer needed (to + avoid memory leaks), and that storage is not being deallocated + while it is still being used (see <xref + linkend="sect-Defensive_Coding-C-Use-After-Free"/>). + </para> + </listitem> +</itemizedlist> +</chapter> diff --git a/defensive-coding/src/.gitignore b/defensive-coding/src/.gitignore index 4adbfe5..335a122 100644 --- a/defensive-coding/src/.gitignore +++ b/defensive-coding/src/.gitignore @@ -4,5 +4,6 @@ /TLS-Client-OpenSSL /XML-Parser-Expat /XML-Parser-Qt +/Go-Error_Handling *.class *.o diff --git a/defensive-coding/src/Go-Error_Handling.go b/defensive-coding/src/Go-Error_Handling.go new file mode 100644 index 0000000..d546018 --- /dev/null +++ b/defensive-coding/src/Go-Error_Handling.go @@ -0,0 +1,48 @@ +package main + +import "io" + +//+ Go Error_Handling-Regular +type Processor interface { + Process(buf []byte) (message string, err error) +} + +type ErrorHandler interface { + Handle(err error) +} + +func RegularError(buf []byte, processor Processor, + handler ErrorHandler) (message string, err error) { + message, err = processor.Process(buf) + if err != nil { + handler.Handle(err) + return "", err + } + return +} +//- + +//+ Go Error_Handling-IO +func IOError(r io.Reader, buf []byte, processor Processor, + handler ErrorHandler) (message string, err error) { + n, err := r.Read(buf) + // First check for available data. + if n > 0 { + message, err = processor.Process(buf[0:n]) + // Regular error handling. + if err != nil { + handler.Handle(err) + return "", err + } + } + // Then handle any error. + if err != nil { + handler.Handle(err) + return "", err + } + return +} +//- + +func main() { +} diff --git a/defensive-coding/src/src.mk b/defensive-coding/src/src.mk index d47fc09..18bd592 100644 --- a/defensive-coding/src/src.mk +++ b/defensive-coding/src/src.mk @@ -2,10 +2,12 @@ CC = gcc CXX = g++ +GCCGO = gccgo CWARNFLAGS = -Wall -W -Wno-unused-parameter -Werror=implicit-function-declaration CXXWARNFLAGS = -Wall -W CFLAGS = -std=gnu99 -O2 $(CWARNFLAGS) -g CXXFLAGS = -std=c++03 -O2 $(CXXWARNFLAGS) -g +GOFLAGS = -O2 -Wall -W LDFLAGS = -g # List files which should only be compiled for syntax checking. @@ -41,6 +43,7 @@ compile_and_link += XML-Parser-Expat LIBS_XML-Parser-Expat = -lexpat compile_and_link += XML-Parser-Qt LIBS_XML-Parser-Qt = -lQtCore -lQtXml +compile_and_link += Go-Error_Handling # Define preprocessor symbols if certain functions exist. CHECK_FUNCTION = crypto/X509_check_host/-DHAVE_X509_CHECK_HOST \ @@ -68,6 +71,9 @@ src/%.class: src/%.java src/%: src/%.o $(CXX) $(LDFLAGS) $^ -o $@ $(LIBS_$(notdir $@)) +src/%: src/%.go + $(GCCGO) $(GOFLAGS) $(LDFLAGS) -o $@ $^ + src/TLS-Client-GNUTLS: src/tcp_connect.o src/TLS-Client-OpenSSL: src/tcp_connect.o src/x509_check_host.o src/TLS-Client-NSS: src/tcp_connect.o
-- security mailing list security@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/security
