Mercurial > dovecot > original-hg > dovecot-1.2
annotate doc/securecoding.txt @ 9308:1072d2b53f72 HEAD
login-proxy: If proxy destination is known to be down, fail immediately.
We'll use a simple rule: If connection failed or timed out more recently
than it succeeded AND there are currently no clients trying to connect to
it, fail it. Since the connect isn't failed unless there is at least one
client already trying to connect to it, the proxy notices immediately when
the server comes back up and then starts serving it again.
| author | Timo Sirainen <tss@iki.fi> |
|---|---|
| date | Wed, 12 Aug 2009 14:51:35 -0400 |
| parents | 51406aaccb46 |
| children |
| rev | line source |
|---|---|
| 1787 | 1 Simplicity provides security. The more you have to remember to maintain |
| 2 security the easier it is to forget something. | |
| 3 | |
| 4 | |
| 5 Use Multiple Layers of Security | |
| 6 ------------------------------- | |
| 7 | |
| 8 Input validation is useful to prevent clients from taking too much server | |
| 9 resources. Add the restrictions only where it's useful. For example a | |
| 10 simple "maximum line length" will limit the length of pretty much all | |
| 11 possible client input. | |
| 12 | |
| 13 Don't rely on input validation. Maybe you missed something. Maybe someone | |
| 14 calls your function somewhere else where you didn't originally intend it. | |
| 15 Maybe someone makes the input validation less restrictive for some reason. | |
| 16 Point is, it's not an excuse to cause a security hole just because input | |
| 17 wasn't what you expected it to be. | |
| 18 | |
| 19 Don't trust memory. If code somewhere overflowed a buffer, don't make it | |
| 20 easier to exploit it. For example if you have code: | |
| 21 | |
| 22 static char staticbuf[100]; | |
| 23 .. | |
| 24 char stackbuf[100]; | |
| 25 strcpy(stackbuf, staticbuf); | |
| 26 | |
| 27 Just because staticbuf was declared as [100], it doesn't mean it couldn't | |
| 28 contain more data. Overflowing static buffers can't be directly exploited, | |
| 29 but the strcpy() overflowing stackbuf makes it possible. Always copy data | |
| 30 with bounds checking. | |
| 31 | |
| 32 | |
| 33 Prevent Buffer Overflows | |
| 34 ------------------------ | |
| 35 | |
| 36 Avoid writing to buffers directly. Write everything through buffer API | |
| 37 (lib/buffer.h) which guarantees protection against buffer overflows. | |
| 38 There are various safe string APIs as well (lib/str.h, lib/strfuncs.h). | |
|
3872
b054cd10ceaa
Small updates: Added note about array API and updates to data stack.
Timo Sirainen <tss@iki.fi>
parents:
1787
diff
changeset
|
39 Dovecot also provides a type safe array API (lib/array.h). |
| 1787 | 40 |
| 41 If you do write to buffers directly, mark the code with /* @UNSAFE */ | |
| 42 unless it's _obviously_ safe. Only obviously safe code is calling a | |
| 43 function with (buffer, sizeof(buffer)) parameters. If you do _any_ | |
| 44 calculations with buffer size, mark it unsafe. | |
| 45 | |
| 46 Use const with buffers whenever you can. It guarantees that you can't | |
| 47 accidentally modify it. | |
| 48 | |
| 49 Use "char *" only for NUL-terminated strings. Use "unsigned char *" | |
| 50 if it's not guaranteed to be NUL-terminated. | |
| 51 | |
| 52 | |
| 53 Avoid free() | |
| 54 ------------ | |
| 55 | |
| 56 Accessing freed memory is the most difficult problem to solve with C code. | |
| 57 Only real solution is to use garbage collector, but it's not possible to | |
| 58 write a portable GC without radical changes in how you write code. | |
| 59 | |
| 60 I've added support for Boehm GC, but it doesn't seem to be working very | |
| 61 well currently. In any case I'd rather not make it required. | |
| 62 | |
| 63 There are a few ways to avoid most free() calls however: data stack and | |
| 64 memory pools. | |
| 65 | |
| 66 Data stack works in somewhat similiar way to C's control stack. alloca() is | |
| 67 quite near to what it does, but there's one major difference: Stack frames | |
|
3872
b054cd10ceaa
Small updates: Added note about array API and updates to data stack.
Timo Sirainen <tss@iki.fi>
parents:
1787
diff
changeset
|
68 are explicitly defined, so functions can return values allocated from data |
|
b054cd10ceaa
Small updates: Added note about array API and updates to data stack.
Timo Sirainen <tss@iki.fi>
parents:
1787
diff
changeset
|
69 stack. t_strdup_printf() call is an excellent example of why this is |
|
b054cd10ceaa
Small updates: Added note about array API and updates to data stack.
Timo Sirainen <tss@iki.fi>
parents:
1787
diff
changeset
|
70 useful. Rather than creating some arbitrary sized buffer and using |
|
b054cd10ceaa
Small updates: Added note about array API and updates to data stack.
Timo Sirainen <tss@iki.fi>
parents:
1787
diff
changeset
|
71 snprintf() which may truncate the value, you can just use t_strdup_printf() |
|
b054cd10ceaa
Small updates: Added note about array API and updates to data stack.
Timo Sirainen <tss@iki.fi>
parents:
1787
diff
changeset
|
72 without worrying about buffer sizes being large enough. |
|
b054cd10ceaa
Small updates: Added note about array API and updates to data stack.
Timo Sirainen <tss@iki.fi>
parents:
1787
diff
changeset
|
73 |
|
b054cd10ceaa
Small updates: Added note about array API and updates to data stack.
Timo Sirainen <tss@iki.fi>
parents:
1787
diff
changeset
|
74 Try to keep the allocations from data stack small, since the data stack's |
|
b054cd10ceaa
Small updates: Added note about array API and updates to data stack.
Timo Sirainen <tss@iki.fi>
parents:
1787
diff
changeset
|
75 highest memory usage size is kept for the rest of the process's lifetime. |
|
b054cd10ceaa
Small updates: Added note about array API and updates to data stack.
Timo Sirainen <tss@iki.fi>
parents:
1787
diff
changeset
|
76 The initial data stack size is 32kB and it should be enough in normal use. |
|
b054cd10ceaa
Small updates: Added note about array API and updates to data stack.
Timo Sirainen <tss@iki.fi>
parents:
1787
diff
changeset
|
77 See lib/data-stack.h. |
| 1787 | 78 |
| 79 Memory pools are useful when you have to construct an object from multiple | |
| 80 pieces and you can free it all at once. Actually Dovecot's Memory Pool API | |
| 81 is just an abstract class for allocating memory. There's system_pool for | |
| 82 allocating memory with calloc(), realloc() and free() and you can create a | |
| 83 pool to allocate memory from data stack. If your function needs to allocate | |
| 84 memory for multiple objects, you may want to take struct pool as parameter | |
| 85 to allow caller to specify where the memory is allocated from. | |
| 86 See lib/mempool.h | |
| 87 | |
| 88 | |
|
3935
51406aaccb46
Added something about setting freed pointers to NULL.
Timo Sirainen <tss@iki.fi>
parents:
3872
diff
changeset
|
89 Deinitialize safely |
|
51406aaccb46
Added something about setting freed pointers to NULL.
Timo Sirainen <tss@iki.fi>
parents:
3872
diff
changeset
|
90 ------------------- |
|
51406aaccb46
Added something about setting freed pointers to NULL.
Timo Sirainen <tss@iki.fi>
parents:
3872
diff
changeset
|
91 |
|
51406aaccb46
Added something about setting freed pointers to NULL.
Timo Sirainen <tss@iki.fi>
parents:
3872
diff
changeset
|
92 Whenever you free a pointer, set it to NULL. That way if you accidentally |
|
51406aaccb46
Added something about setting freed pointers to NULL.
Timo Sirainen <tss@iki.fi>
parents:
3872
diff
changeset
|
93 try to free it again, it's less likely to cause a security hole. Dovecot |
|
51406aaccb46
Added something about setting freed pointers to NULL.
Timo Sirainen <tss@iki.fi>
parents:
3872
diff
changeset
|
94 does this automatically with most of its free() calls, but you should also |
|
51406aaccb46
Added something about setting freed pointers to NULL.
Timo Sirainen <tss@iki.fi>
parents:
3872
diff
changeset
|
95 make it a habit of making all your _destroy() functions take a |
|
51406aaccb46
Added something about setting freed pointers to NULL.
Timo Sirainen <tss@iki.fi>
parents:
3872
diff
changeset
|
96 pointer-to-pointer parameter which you set to NULL. |
|
51406aaccb46
Added something about setting freed pointers to NULL.
Timo Sirainen <tss@iki.fi>
parents:
3872
diff
changeset
|
97 |
| 1787 | 98 Don't Keep Secrets |
| 99 ------------------ | |
| 100 | |
| 101 We don't do anything special to protect ourself against read access buffer | |
| 102 overflows, so don't store anything sensitive in memory. We use multiple | |
| 103 processes to protect sensitive information between users. | |
| 104 | |
| 105 When dealing with passwords and such, erase them from memory after you | |
| 106 don't need it anymore. Note that such memset() may be optimized away by | |
| 107 compiler, use safe_memset(). | |
| 108 | |
| 109 | |
| 110 Use GCC Extensions | |
| 111 ------------------ | |
| 112 | |
| 113 GCC makes it easy to catch some potential errors: | |
| 114 | |
| 115 Format string vulnerabilities can be prevented by marking all functions | |
| 116 using format strings with __attr_format__() and __attr_format_arg__() | |
| 117 macros and using -Wformat=2 GCC option. | |
| 118 | |
| 119 -W option checks that you don't compare signed and unsigned variables. | |
| 120 | |
| 121 I hope GCC will later emit a warning whenever there's potential integer | |
| 122 truncation. -Wconversion kind of does that, but it's not really meant for | |
| 123 it and it gives too many other useless warnings. | |
| 124 | |
| 125 | |
| 126 Use union Safely | |
| 127 ---------------- | |
| 128 | |
| 129 Suppose there was code: | |
| 130 | |
| 131 union { | |
| 132 unsigned int number; | |
| 133 char *str; | |
| 134 } u; | |
| 135 | |
| 136 If it was possible for user to set number arbitrarily, but access the union | |
| 137 as string it'd be possible to read or write arbitrary memory locations. | |
| 138 | |
| 139 There's two ways to handle this. First would be to avoid union entirely and | |
| 140 use a struct instead. You don't really need the extra few bytes of memory | |
| 141 that union saves. | |
| 142 | |
| 143 Another way is to access the union only through macro that verifies that | |
| 144 you're accessing it correctly. See IMAP_ARG_*() macros in | |
| 145 lib-imap/imap-parser.h. |