Amos Jeffries [Tue, 27 Nov 2012 21:19:46 +0000 (10:19 +1300)]
Audit Review updates
* guarantee that note values output by the HelperReply parser are ""
nil-terminated string and not undefined String.
* rename HelperReply::responseKeys to 'notes'
* rename Notes::findByName() to find()
* add Note::firstValue() to locate first value provided for a given key
and present it as a char* terminated string
* various documentation updates
Amos Jeffries [Tue, 27 Nov 2012 10:57:12 +0000 (23:57 +1300)]
Treat no-cache and must-revalidate in Authentication
Wrapped as a violation because this operation is off-spec.
CC:no-cache was omitted from the HTTP spec apparently on grounds that
changing its caching effects on authentication would come as a surprise.
The actual operation is safe enough to use when parameterless no-cache
is treated strictly as an alias for must-revalidate (as done by Squid now).
Alex Rousskov [Mon, 26 Nov 2012 18:54:43 +0000 (11:54 -0700)]
Make it possible to match empty header field values using req_header and rep_header.
Warning: Some req_header and rep_header ACLs that were [accidentally] not
matching empty headers (e.g., "^$" or ".*") will now start matching them.
A new HttpHeader::getByNameIfPresent() method is added to be able to detect
presence of empty header fields while ACLHTTPHeaderData::match() is adjusted
to convert undefined String values into empty c-strings ("") for
ACLRegexData::match() to work.
Prior to these changes, when trying to match an empty header value with a
regex like "^$", ACLHTTPHeaderData::match() would return false because:
* HttpHeader::getStrOrList() and getByName() return an undefined String.
* String::termedBuf() returns NULL for undefined Strings; and
* ACLRegexData::match() always fails on NULL c-strings.
Amos Jeffries [Sat, 24 Nov 2012 14:30:02 +0000 (03:30 +1300)]
Fix helper reply length detection
Stateless helper reply handler was incrementing 't' position before
passing the details of response length to HelperReply parser.
Stateful helper reply handler was pointing at position of \n insteaf of
position of \r when \r\n received.
Both of these cause the response length parameter sent to the reply
parser to be longer than the actual response characters length. Breaking
parse on responses with just result code followed by \n since the string
comparison EOL is now tested against input length 2/3/4.
Amos Jeffries [Sun, 18 Nov 2012 10:37:19 +0000 (03:37 -0700)]
Fix several buffer termination bugs
* strcpy() replaced in several places with strncpy() to ensure destination
buffers are not overflowed.
* strncpy() does not nul-terminate the destination when the string being
copied in exactly fills the buffer. Ensure we have terminated strings
where it may matter.
Detected by Coverity Scan. Issues 740309, 740310, 740311, 740481, 740483
Amos Jeffries [Sun, 18 Nov 2012 10:02:38 +0000 (03:02 -0700)]
Remove MemPoolChunked::memPID
This member variable appears to have been missed when MemPool was split
into generic framework and specific Chunked implementation.
(rev:10513.1.1 aka trunnk rev:10517)
The memPID and its maintenance code was moved into MemImplementingAllocator
but this definitio left here un-initialized and shadowing the framework
member.
Amos Jeffries [Sun, 18 Nov 2012 07:06:26 +0000 (00:06 -0700)]
negotiate_kerberos_auth: better bounds checking
* sysconf() may return -N values on some platforms or values larger than
the hard-coded 1024 buffer size for hostname. Use sizeof() instead
since the buffer is hardcoded anyway.
* also, use return instead of exit() on the test binary to reduce
warnings from static analysis compilers.
Amos Jeffries [Sat, 17 Nov 2012 12:58:59 +0000 (05:58 -0700)]
Various memory leaks in configuration parsing
This lot are all small issues derived from allocating new memory and
assigning to a pointer already pointing at previous allocation, or
passing xstrdup() output to a caller which does not directly hold the
passed memory.
Both cases will disappear once we clean up teh string handlign in Squid
but for now these still need fixing to avoid leaking memory.
Detected by Coverity Scan. Issues 740430, 740432, 740439.
Amos Jeffries [Fri, 16 Nov 2012 06:45:56 +0000 (23:45 -0700)]
Fix various assertion with side effects
When compiled with high optimization and assert disabled these operations
would have disappeared. The side effects being:
* Disk I/O failure protection disabled. Allowing loops in diskd write.
* squidpurge error handling on command line parse gone. Causing segfault.
* squidpurge 'I am Alive' ticker feature cease working.
Detected by Coverity Scan. Issues 740299, 740300, 740301, 740302, 740303
Amos Jeffries [Thu, 15 Nov 2012 09:00:29 +0000 (02:00 -0700)]
ntlm_fake_auth: Fix nesting error
Broken macro wrapping leads to the fake authenticator sending bad
responses to Squid. This can ead to users being rejected by the fake
helper whose purpose is to accept everything.
Amos Jeffries [Wed, 14 Nov 2012 01:33:49 +0000 (18:33 -0700)]
digest_edirectory_auth: improved error handling
Malicious response from LDAP server can cause squid helper to crash.
Missing realm value returned from LDAP without error/missing value being
indicated in the response can lead to strcmp() using a NULL pointer.
Extremely unlikely to happen in practice, but worth fixing.
Changed HERE macro to be a true nop, clarified documentation, and marked is as deprecated.
Fixed syntax bug in comm.cc unearthed by the macro-to-function change.
- Try to update the index file in all cases the database modified
- The find operator in database should not modify the database. Currently
if an entry is expired, ssl_crtd removes the cert file but does not
update the index file.
rows. Currently we are using the new operator.
- Fix a small memory leak when remove entries from database: A row object
removed from TXT_DB indexes but never released. This patch:
* Use OPENSSL_malloc and OPENSSL_free to allocate/release memory for
TXT_DB rows. OpenSSL SDK assumes that always allocated using these
functions.
* Add code in Ssl::CertificateDb::Row destructor to correctly release
a TXT_DB row.
* Add the sq_TXT_DB_delete and sq_TXT_DB_delete_row functions which
removes a row from TXT_DB indexes.
Amos Jeffries [Fri, 9 Nov 2012 04:23:08 +0000 (17:23 +1300)]
Upgrade Digest authentication to use key=value parameters
* Update the Digest auth code in Squid to process the HelperReply key-pair
notes list for data
+ output upgrade WARNING messages when old format responses are identified
+ handle TT result code (as an error with critical level ERROR message)
+ handle BH result code (as an error)
* Adds a ha1="" key to Digest auth response protocol to pass the HA1.
* Upgrade the Digest helpers to use OK/ERR/BH result codes and key=value
parameters.
With the patch r12422 which added support for OK/ERR/BH helper response codes,
the ssl_crtd interface become broken. The ConnStateData::sslCrtdHandleReply
ssl_crtd reply handler, expects a response code included at the beggining of
the returned message. This is not true after the changes.
This patch add a parameter in Ssl::CrtdMessage constructor to define a message
as "ssl_crtd reply" or "ssl_crtd request" message. For ssl_crtd reply messages
the message parser does not expect helper response code.
Amos Jeffries [Wed, 7 Nov 2012 13:13:47 +0000 (02:13 +1300)]
Use Notes objects for key=pair handling in HelperReply
* NTLM/Negotiate auth are expeted to return OK/ERR/BH/TT codes and key=pair.
The old result lines are still received, but converted to the new format
silently.
* new key accepted:
'token=' for passing NTLM and Negotiate auth tokens
* some undocumented old tags accepted by external_acl_type are now ignored:
'error=' replaced by 'message='
'passwd=' replaced by 'password='
'login=' replaced by 'user='
These were replaced some time ago and never formally documented.
Amos Jeffries [Tue, 6 Nov 2012 22:32:56 +0000 (11:32 +1300)]
Update Notes API to support key=pair additions and lookup
* Adds a Notes::const_iterator definition to match Notes::iterator
* Adds a Notes::findByName(note-name) method to locate notes given their
textual key / name string.
* Adds a Notes::add(key, value) method to insert key and value as a
single operation. No more need for callers to be using Note::Pointer
or be aware of the Note:: API just to insert the value.
TODO:
* add "key=value" syntax parser to Notes. The current parse() method
onlyl accepts MiME header syntax (line- and whitespace- oriented).
* add ostream operator to dump the notes list content to a stream.
Amos Jeffries [Tue, 30 Oct 2012 09:16:33 +0000 (22:16 +1300)]
Fix HelperReply(const HelperReply&) is private
GCC 4.1 and possibly others use copy-constructor when an instance is
created in a function parameter list using copy by reference.
We must create such objects as loal variables and pass to the function
for best efficiency.
Amos Jeffries [Mon, 29 Oct 2012 23:12:04 +0000 (12:12 +1300)]
Support OK/ERR/BH response codes from any helper
Updates the helper reponse callback API from using char* buffer to a
HelperReply object storing teh response code, a blob buffer, and
pointer to the responding helper 'server' (if stateful).
* the helper I/O read handler is updated to parse the result code off
the start of the helper response as is currently done for channel-ID.
The callback handlers are altered to use the HelperReply::status instead
of parsing it off themselves individually.
* the remaining I/O read buffer is stored in a MemBuf and callbacks are
updated to use it via the method other().
* the responding helper-server is stored into the HelperReply object and
stateful helper callbacks are combined into the same API as stateless.
The callback handlers are updated to use HelperReply::lastserver instead
of function parameter.
After this patch the helper response format is:
[channel-ID] SP [result] [ [SP] blob] <terminator>
'SP' being one octet \0x20 character.
The behavour changes expected from this is that all helpers are now able
to send OK/ERR/BH states. Although the handlers for some helpers will
deal with the new states as unknown response. None of the bundled
helpers have yet been altered to make use of this changed potential.
TODO:
* implement key=value parser for the blob area of the format, and update
handlers to use the HelperReply API to retrieve them.
* upgrade helpers to make use of new response format
Amos Jeffries [Mon, 29 Oct 2012 04:59:58 +0000 (22:59 -0600)]
Polish: update the RefCount API a bt and split Lock out
* Shuffle RefCount.h and its unit-tests into src/base/
* Reworks struct Refcountable_ into class LockableObject in its own header
+ changing the reference counter accessors to a lock()/unlock() names
+ some minor symbol updates of code directly utilizing the RefCountable_
members
With this we can begin the process of replacing our multiple different
implementations of the reference-counting pattern using LockableObject.
No code changes have been made. Just symbol polishing.
TODO: update the unit-tests for refcounting to use CPPUnit
This patch adds note option to squid.conf. It is similar to
adaptation_meta but is applied after all adaptation and before logging.
Values of key/value pair notes can be logged using %{key}note macros.
note key value acl ...
logformat myFormat ... %{key}note ...
This option may be initially used to log custom information about the master
transaction. For example, an admin may configure Squid to log which "user group"
the transaction belongs to, where "user group" will be determined based on a
set of ACLs and not [just] authentication information.
From user point of view, adaptation_header sets/implies meta (i.e.,
setting adaptation_meta is sufficient to be able to log it using
%note) but the note option itself (if any) is evaluated later,
so it has no effect on ICAP headers.
Amos Jeffries [Fri, 26 Oct 2012 11:36:45 +0000 (05:36 -0600)]
Polish: Http::MethodType upgrade
Convert the METHOD_* enum into a class with accessors to determine
method string/image, idempotency, safety (as per RFC 2616 terminology).
* creates the initial http/ directory and http/libsquid-http.la
library as per SourceLayout needs
- libsquid-http is chosen to prevent symbols mistakes with libhttp
which may exist on some systems under the libhttp or http/libhttp name.
enum changes:
* migrates _method_t to Http::MethodType and makes it a enum with
auto-generated strings array provided by the new library.
* the enum list is extended to contain all methods in the HTTPbis WG
new method registry
- documentation now references that registry and HTTPbis draft
document listing methods
HttpRequest class changes:
* cacheable() changed to maybeCacheable() accessor
- to reflect that it determines NON-cacheability with certainty, but
when returns TRUE it can still be overridden by Http Reply details
and/or local environment details.
- updated to cache the complex calculation result.
HttpRequestMethod class changes:
* The new isIdempotent() and isHttpSafe() accessors are altered
slightly from the original code to match the RFCs. The existing code
produces incorrect true/false values on some methods.
* new accessor shouldInvalidate() which produces true if the request
method alone is enough to determine an RFC SHOULD invalidate condition.
- purgesOthers() accessor is still present with the old behaviour, but
for better caching we should convert the code to using
shouldInvalidate()
* new accessors respMaybeCacheable() which produces true if the
request method alone is enough to determine an RFC MAY cache condition.
- replaces isCacheable() accessor which did not produce any certainty
as implied by the name, and was producing incorrect assessments anyway
for some methods.
- it is more of a test whether *Squid* is known to be able to cache
the response to this method, than whether the RFC permit it.
- There are several cases where it should produce true according to
the RFCs but isCacheable() was doing false due to bugs or missing
feature support. These cases are now documented with reasons and things
to test or fix before they can be made to return true.
* the remainders of obsolete extension_methods directive code is dropped
Global httpCachable(method) function dropped - replaced by
HttpRequest::maybeCacheable() accessor which does better cacheability
checks.
... and there are a lot of METHOD_FOO -> Http::METHOD_FOO symbol-only
changes fluffing out the patch size.
Amos Jeffries [Tue, 23 Oct 2012 06:39:34 +0000 (19:39 +1300)]
Remove Bungled warning on missing component directives
* Make Squid display a ERROR notice and ignore squid.conf directives
which belong to components disabled at build time instead of throwing
up 'FATAL: Bungled' message and aborting.
* Polish the 'requires X' message text for better display of cases like
'requires MS Windows' and 'requires Linux'
* Add 'requires X' message for --enable-auth, --enable-eui, and
--enable-ssl-crtd which were missing.
projects / thirdparty / squid.git / log
