Re: [PATCH] Ignore and warn about attempts to reconfigure static Rock store options.

[Dmitry Kurochkin]

Hi Alex, Amos.

On Mon, 19 Sep 2011 23:07:43 -0600, Alex Rousskov 
 wrote:
> On 09/19/2011 08:10 PM, Amos Jeffries wrote:
> > On Tue, 20 Sep 2011 02:40:13 +0400, Dmitry Kurochkin wrote:
> >> Attached patch does some polishing for Rock store cache_dir
> >> reconfiguration.
> >>
> >>
> >> Some Rock store options cannot be changed dynamically: path, size, and
> >> max-size.  Before the change, there were no checks during reconfigure
> >> to prevent changing these options.  This may lead to Rock cache
> >> corruption and other bugs.  The patch adds necessary checks to Rock
> >> store code.  If user tries to change an option that cannot be updated
> >> dynamically, a warning is reported and the value is left unchanged.
> 
> 
> 
> > Of itself the patch looks okay.
> > 
> > BUT... IMO _path_ should never be re-configurable.
> 
> Good point! I wonder whether admins expect cache_dir lines to be tied to
> paths (e.g., changing cache_dir order changes nothing) or to position
> (i.e., changing path can be supported). We should document what we
> actually support (but that documentation change, if needed, is outside
> this patch scope).
> 
> 
> > If rock store can't handle multiple cache_dir of type rock with unique
> > paths there is something deeper wrong.
> 
> It can. Nothing special here.
> 
> 
> > AFAIK path should be the unique key to identify different cache_dir.
> > Changing the path means a completely different dir is now being
> > referenced. If not already loaded the dir needs to be loaded as if on
> > startup. If any existing are no longer referenced the old cache_dir
> > details needs discarding.
> 
> I tend to agree: Path-matching is probably better than position-matching.
> 
> Dmitry, could you please check whether current code maps cache_dir lines
> to SwapDir objects using cache_dir position in squid.conf or using
> cache_dir paths? If it is the latter, you can remove the path-related
> warning from the patch, right?
> 

Cache dir mapping uses paths (though it ignores case).  So it works as
you would expect.  I knew that when I was working on the patch and I
still included the path check.  Because it feels to me like we should
check the path if it is passed to SwapDir::reconfigure().  Otherwise we
should not pass path to SwapDir::reconfigure() at all.  (There is also
index parameter which should be checked or removed as well following the
same arguments.)  What do you think about:

* remove path check from this patch

* prepare another patch that removes index and path parameters from
  SwapDir::reconfigure()

Regards,
  Dmitry

> 
> Thank you,
> 
> Alex.

Re: [PATCH] Fix cache_dir type check during reconfiguration.

[Dmitry Kurochkin]

On Tue, 20 Sep 2011 13:48:39 +1200, Amos Jeffries  wrote:
>  On Tue, 20 Sep 2011 13:44:24 +1200, Amos Jeffries wrote:
> > On Tue, 20 Sep 2011 02:38:08 +0400, Dmitry Kurochkin wrote:
> >> Hello.
> >>
> >> This is a small and simple fix for cache_dir reconfiguration.
> >>
> >>
> >> SwapDir::type() returns C strings which should be compared with
> >> strcmp(3) instead of checking pointers for equality.
> >>
> >> Regards,
> >>   Dmitry
> >
> >
> > eek!
> >
> > +1 please commit.
> >
> > Amos
> 
> 
>  Actually. Please check if this fixes:
>   http://bugs.squid-cache.org/show_bug.cgi?id=3054
> 
>  and reference the bug the usual way if it does.
> 

The bug is about adding new cache_dir, not reconfiguring existing ones.
So it is not related AFAICT.  Also, there is support for adding new
cache dirs during reconfigure, looks like it should work.  (Though I
think it would not work properly for shared Rock store.)  Unfortunately,
the bug has no logs...

Regards,
  Dmitry

>  Amos

Re: SSL Bump Certificate Blacklist

[Fabian Hugelshofer]

On 09/16/2011 04:35 PM, Alex Rousskov wrote:

On 09/16/2011 12:55 AM, Fabian Hugelshofer wrote:


manually issuing CRLs
for a certain CA is not possible because the CRL has to be singed by the
CA that issued the certificates.


Wow, that is a major SSL limitation indeed! If this is true, we either
must implement a custom black list functionality in Squid or customize
OpenSSL to allow any trusted CA to revoke any certificate. Since there
are too many OpenSSL versions, the former is probably more practical.


The requirements are not as strict as I initially thought. According to 
RFC5280 section 5.1.1.3, the key used to issue certificates does not 
have to be identical ot the one that is used to sign the CRL. Still, an 
arbitrary trusted CA does not seem to be allowed to revoke certificates 
of another one. At least OpenSSL does not allow this.


check_crl_chain() in crypto/x509/x509_vfy.c has the following comment:
"/* RFC3280 says nothing about the relationship between CRL path
 * and certificate path, which could lead to situations where a
 * certificate could be revoked or validated by a CA not authorised
 * to do so. RFC5280 is more strict and states that the two paths must
 * end in the same trust anchor, though some discussions remain...
 * until this is resolved we use the RFC5280 version
 */"

Like you say, there is need for a custom certificate blacklist. 
Optimally, this blacklist is generic and can be configured through an ACL.




What would you use to identify the certificates?
- Serial number?
- Serial number and common name?
- Serial number and issuer?
- Fingerprint (might not be available in each case)?



I hope somebody already answered these important questions in general
CRL context!


CRLs use the serial number in combination with the issuer.


The issuer name and the serial are probably not sufficient as there can 
be multiple CAs with the same name (certificates with different 
validity, different trust chains, ...).




If we implement a custom black list, we can be flexible and support all
of the above. To start with, I guess we should block all certificates
with DigiNotar in the chain.



What are the next steps, would you like me to open a Squid Bug?

Regards,

Fabian

Re: SSL Bump Certificate Blacklist

[Alex Rousskov]

On 09/20/2011 05:46 AM, Fabian Hugelshofer wrote:
> On 09/16/2011 04:35 PM, Alex Rousskov wrote:
>> On 09/16/2011 12:55 AM, Fabian Hugelshofer wrote:
> What would you use to identify the certificates?
> - Serial number?
> - Serial number and common name?
> - Serial number and issuer?
> - Fingerprint (might not be available in each case)?


 I hope somebody already answered these important questions in general
 CRL context!
>>>
>>> CRLs use the serial number in combination with the issuer.
> 
> The issuer name and the serial are probably not sufficient as there can
> be multiple CAs with the same name (certificates with different
> validity, different trust chains, ...).
> 
> 
>> If we implement a custom black list, we can be flexible and support all
>> of the above. To start with, I guess we should block all certificates
>> with DigiNotar in the chain.
> 
> 
> What are the next steps, would you like me to open a Squid Bug?

I believe the rule is that new features should be documented on wiki,
but opening a "feature request" bug report would not hurt, I guess.

Can you summarize the minimum number of new ACL types that you think we
should support to make this work? I know we want issuer name and serial
number. Do we want notBefore and notAfter matching? Anything else?

Do we need to be able to test a match against a certificate at Nth
position in the chain? Or can we always test all certificates in the chain?


It would be nice to hear from others regarding this issue to make sure
we are not missing something important.


Thank you,

Alex.

Re: [PATCH] Ignore and warn about attempts to reconfigure static Rock store options.

[Alex Rousskov]

On 09/20/2011 04:30 AM, Dmitry Kurochkin wrote:
> Hi Alex, Amos.
> 
> On Mon, 19 Sep 2011 23:07:43 -0600, Alex Rousskov 
>  wrote:
>> On 09/19/2011 08:10 PM, Amos Jeffries wrote:
>>> On Tue, 20 Sep 2011 02:40:13 +0400, Dmitry Kurochkin wrote:
 Attached patch does some polishing for Rock store cache_dir
 reconfiguration.


 Some Rock store options cannot be changed dynamically: path, size, and
 max-size.  Before the change, there were no checks during reconfigure
 to prevent changing these options.  This may lead to Rock cache
 corruption and other bugs.  The patch adds necessary checks to Rock
 store code.  If user tries to change an option that cannot be updated
 dynamically, a warning is reported and the value is left unchanged.
>>
>>
>>
>>> Of itself the patch looks okay.
>>>
>>> BUT... IMO _path_ should never be re-configurable.
>>
>> Good point! I wonder whether admins expect cache_dir lines to be tied to
>> paths (e.g., changing cache_dir order changes nothing) or to position
>> (i.e., changing path can be supported). We should document what we
>> actually support (but that documentation change, if needed, is outside
>> this patch scope).
>>
>>
>>> If rock store can't handle multiple cache_dir of type rock with unique
>>> paths there is something deeper wrong.
>>
>> It can. Nothing special here.
>>
>>
>>> AFAIK path should be the unique key to identify different cache_dir.
>>> Changing the path means a completely different dir is now being
>>> referenced. If not already loaded the dir needs to be loaded as if on
>>> startup. If any existing are no longer referenced the old cache_dir
>>> details needs discarding.
>>
>> I tend to agree: Path-matching is probably better than position-matching.
>>
>> Dmitry, could you please check whether current code maps cache_dir lines
>> to SwapDir objects using cache_dir position in squid.conf or using
>> cache_dir paths? If it is the latter, you can remove the path-related
>> warning from the patch, right?
>>
> 
> Cache dir mapping uses paths (though it ignores case).  So it works as
> you would expect.  I knew that when I was working on the patch and I
> still included the path check.  Because it feels to me like we should
> check the path if it is passed to SwapDir::reconfigure().  Otherwise we
> should not pass path to SwapDir::reconfigure() at all.  (There is also
> index parameter which should be checked or removed as well following the
> same arguments.)  What do you think about:
> 
> * remove path check from this patch
> 
> * prepare another patch that removes index and path parameters from
>   SwapDir::reconfigure()


Sounds like a good plan to me.


Thank you,

Alex.

Re: [PATCH] Ignore and warn about attempts to reconfigure static Rock store options.

[Dmitry Kurochkin]

On Tue, 20 Sep 2011 09:08:33 -0600, Alex Rousskov 
 wrote:
> On 09/20/2011 04:30 AM, Dmitry Kurochkin wrote:
> > Hi Alex, Amos.
> > 
> > On Mon, 19 Sep 2011 23:07:43 -0600, Alex Rousskov 
> >  wrote:
> >> On 09/19/2011 08:10 PM, Amos Jeffries wrote:
> >>> On Tue, 20 Sep 2011 02:40:13 +0400, Dmitry Kurochkin wrote:
>  Attached patch does some polishing for Rock store cache_dir
>  reconfiguration.
> 
> 
>  Some Rock store options cannot be changed dynamically: path, size, and
>  max-size.  Before the change, there were no checks during reconfigure
>  to prevent changing these options.  This may lead to Rock cache
>  corruption and other bugs.  The patch adds necessary checks to Rock
>  store code.  If user tries to change an option that cannot be updated
>  dynamically, a warning is reported and the value is left unchanged.
> >>
> >>
> >>
> >>> Of itself the patch looks okay.
> >>>
> >>> BUT... IMO _path_ should never be re-configurable.
> >>
> >> Good point! I wonder whether admins expect cache_dir lines to be tied to
> >> paths (e.g., changing cache_dir order changes nothing) or to position
> >> (i.e., changing path can be supported). We should document what we
> >> actually support (but that documentation change, if needed, is outside
> >> this patch scope).
> >>
> >>
> >>> If rock store can't handle multiple cache_dir of type rock with unique
> >>> paths there is something deeper wrong.
> >>
> >> It can. Nothing special here.
> >>
> >>
> >>> AFAIK path should be the unique key to identify different cache_dir.
> >>> Changing the path means a completely different dir is now being
> >>> referenced. If not already loaded the dir needs to be loaded as if on
> >>> startup. If any existing are no longer referenced the old cache_dir
> >>> details needs discarding.
> >>
> >> I tend to agree: Path-matching is probably better than position-matching.
> >>
> >> Dmitry, could you please check whether current code maps cache_dir lines
> >> to SwapDir objects using cache_dir position in squid.conf or using
> >> cache_dir paths? If it is the latter, you can remove the path-related
> >> warning from the patch, right?
> >>
> > 
> > Cache dir mapping uses paths (though it ignores case).  So it works as
> > you would expect.  I knew that when I was working on the patch and I
> > still included the path check.  Because it feels to me like we should
> > check the path if it is passed to SwapDir::reconfigure().  Otherwise we
> > should not pass path to SwapDir::reconfigure() at all.  (There is also
> > index parameter which should be checked or removed as well following the
> > same arguments.)  What do you think about:
> > 
> > * remove path check from this patch
> > 
> > * prepare another patch that removes index and path parameters from
> >   SwapDir::reconfigure()
> 
> 
> Sounds like a good plan to me.
> 

Attached patch without the path check.

Regards,
  Dmitry

> 
> Thank you,
> 
> Alex.
Ignore and warn about attempts to reconfigure static Rock store options.

Some Rock store options cannot be changed dynamically: path, size, and
max-size.  Before the change, there were no checks during reconfigure
to prevent changing these options.  This may lead to Rock cache
corruption and other bugs.  The patch adds necessary checks to Rock
store code.  If user tries to change an option that cannot be updated
dynamically, a warning is reported and the value is left unchanged.

=== modified file 'src/SwapDir.cc'
--- src/SwapDir.cc	2011-05-12 03:58:16 +
+++ src/SwapDir.cc	2011-09-19 22:15:05 +
@@ -293,42 +293,51 @@ SwapDir::optionReadOnlyDump(StoreEntry *
 if (flags.read_only)
 storeAppendPrintf(e, " no-store");
 }
 
 bool
 SwapDir::optionObjectSizeParse(char const *option, const char *value, int isaReconfig)
 {
 int64_t *val;
 if (strcmp(option, "max-size") == 0) {
 val = &max_objsize;
 } else if (strcmp(option, "min-size") == 0) {
 val = &min_objsize;
 } else
 return false;
 
 if (!value)
 self_destruct();
 
 int64_t size = strtoll(value, NULL, 10);
 
-if (isaReconfig && *val != size)
-debugs(3, 1, "Cache dir '" << path << "' object " << option << " now " << size);
+if (isaReconfig && *val != size) {
+if (allowOptionReconfigure(option)) {
+debugs(3, DBG_IMPORTANT, "cache_dir '" << path << "' object " <<
+   option << " now " << size << " Bytes");
+} else {
+debugs(3, DBG_IMPORTANT, "WARNING: cache_dir '" << path << "' "
+   "object " << option << " cannot be changed dynamically, " <<
+   "value left unchanged (" << *val << " Bytes)");
+return true;
+}
+}
 
 *val = size;
 
 return true;
 }
 
 void
 SwapDir::optionObjectSizeDump(StoreEntry * e) const
 {
 if (min_objsize != 0)
 storeAppendPrintf(e, " min-size=%"PRId64, min_objsize);
 
 if (max_objsize != -1)
 storeA

Re: [PATCH] Ignore and warn about attempts to reconfigure static Rock store options.

[Dmitry Kurochkin]

On Tue, 20 Sep 2011 19:41:31 +0400, Dmitry Kurochkin 
 wrote:
> On Tue, 20 Sep 2011 09:08:33 -0600, Alex Rousskov 
>  wrote:
> > On 09/20/2011 04:30 AM, Dmitry Kurochkin wrote:
> > > Hi Alex, Amos.
> > > 
> > > On Mon, 19 Sep 2011 23:07:43 -0600, Alex Rousskov 
> > >  wrote:
> > >> On 09/19/2011 08:10 PM, Amos Jeffries wrote:
> > >>> On Tue, 20 Sep 2011 02:40:13 +0400, Dmitry Kurochkin wrote:
> >  Attached patch does some polishing for Rock store cache_dir
> >  reconfiguration.
> > 
> > 
> >  Some Rock store options cannot be changed dynamically: path, size, and
> >  max-size.  Before the change, there were no checks during reconfigure
> >  to prevent changing these options.  This may lead to Rock cache
> >  corruption and other bugs.  The patch adds necessary checks to Rock
> >  store code.  If user tries to change an option that cannot be updated
> >  dynamically, a warning is reported and the value is left unchanged.
> > >>
> > >>
> > >>
> > >>> Of itself the patch looks okay.
> > >>>
> > >>> BUT... IMO _path_ should never be re-configurable.
> > >>
> > >> Good point! I wonder whether admins expect cache_dir lines to be tied to
> > >> paths (e.g., changing cache_dir order changes nothing) or to position
> > >> (i.e., changing path can be supported). We should document what we
> > >> actually support (but that documentation change, if needed, is outside
> > >> this patch scope).
> > >>
> > >>
> > >>> If rock store can't handle multiple cache_dir of type rock with unique
> > >>> paths there is something deeper wrong.
> > >>
> > >> It can. Nothing special here.
> > >>
> > >>
> > >>> AFAIK path should be the unique key to identify different cache_dir.
> > >>> Changing the path means a completely different dir is now being
> > >>> referenced. If not already loaded the dir needs to be loaded as if on
> > >>> startup. If any existing are no longer referenced the old cache_dir
> > >>> details needs discarding.
> > >>
> > >> I tend to agree: Path-matching is probably better than position-matching.
> > >>
> > >> Dmitry, could you please check whether current code maps cache_dir lines
> > >> to SwapDir objects using cache_dir position in squid.conf or using
> > >> cache_dir paths? If it is the latter, you can remove the path-related
> > >> warning from the patch, right?
> > >>
> > > 
> > > Cache dir mapping uses paths (though it ignores case).  So it works as
> > > you would expect.  I knew that when I was working on the patch and I
> > > still included the path check.  Because it feels to me like we should
> > > check the path if it is passed to SwapDir::reconfigure().  Otherwise we
> > > should not pass path to SwapDir::reconfigure() at all.  (There is also
> > > index parameter which should be checked or removed as well following the
> > > same arguments.)  What do you think about:
> > > 
> > > * remove path check from this patch
> > > 
> > > * prepare another patch that removes index and path parameters from
> > >   SwapDir::reconfigure()
> > 
> > 
> > Sounds like a good plan to me.
> > 
> 
> Attached patch without the path check.
> 

Oops, forgot to remove unneeded changes in src/fs/rock/RockSwapDir.h.

Regards,
  Dmitry

> Regards,
>   Dmitry
> 
> > 
> > Thank you,
> > 
> > Alex.
Ignore and warn about attempts to reconfigure static Rock store options.

Some Rock store options cannot be changed dynamically: path, size, and
max-size.  Before the change, there were no checks during reconfigure
to prevent changing these options.  This may lead to Rock cache
corruption and other bugs.  The patch adds necessary checks to Rock
store code.  If user tries to change an option that cannot be updated
dynamically, a warning is reported and the value is left unchanged.

=== modified file 'src/SwapDir.cc'
--- src/SwapDir.cc	2011-05-12 03:58:16 +
+++ src/SwapDir.cc	2011-09-19 22:15:05 +
@@ -293,42 +293,51 @@ SwapDir::optionReadOnlyDump(StoreEntry *
 if (flags.read_only)
 storeAppendPrintf(e, " no-store");
 }
 
 bool
 SwapDir::optionObjectSizeParse(char const *option, const char *value, int isaReconfig)
 {
 int64_t *val;
 if (strcmp(option, "max-size") == 0) {
 val = &max_objsize;
 } else if (strcmp(option, "min-size") == 0) {
 val = &min_objsize;
 } else
 return false;
 
 if (!value)
 self_destruct();
 
 int64_t size = strtoll(value, NULL, 10);
 
-if (isaReconfig && *val != size)
-debugs(3, 1, "Cache dir '" << path << "' object " << option << " now " << size);
+if (isaReconfig && *val != size) {
+if (allowOptionReconfigure(option)) {
+debugs(3, DBG_IMPORTANT, "cache_dir '" << path << "' object " <<
+   option << " now " << size << " Bytes");
+} else {
+debugs(3, DBG_IMPORTANT, "WARNING: cache_dir '" << path << "' "
+   "object " << option << " cannot be changed dynamically, " <<
+   "value left unchanged (" 

[PATCH] Remove SwapDir::reconfigure() arguments since they are not used.

[Dmitry Kurochkin]

Hi all.

This is a small cleanup patch discussed recently in another thread.
Note it depends on rock-reconfigure-t3.patch.


Before the change, SwapDir::reconfigure() took index and path
arguments.  But none of them was actually used: neither index nor path
can be changed during reconfigure.  And both index and path are
available as SwapDir members.  So there is no reason to have these
arguments.


Regards,
  Dmitry
Remove SwapDir::reconfigure() arguments since they are not used.

Before the change, SwapDir::reconfigure() took index and path
arguments.  But none of them was actually used: neither index nor path
can be changed during reconfigure.  And both index and path are
available as SwapDir members.  So there is no reason to have these
arguments.

=== modified file 'src/SwapDir.h'
--- src/SwapDir.h	2011-09-20 16:17:56 +
+++ src/SwapDir.h	2011-09-20 16:18:13 +
@@ -108,41 +108,41 @@ SQUIDCEXTERN STDIRSELECT *storeDirSelect
 SQUIDCEXTERN int storeVerifySwapDirs(void);
 SQUIDCEXTERN void storeDirCloseSwapLogs(void);
 SQUIDCEXTERN void storeDirCloseTmpSwapLog(int dirn);
 SQUIDCEXTERN void storeDirDiskFull(sdirno);
 SQUIDCEXTERN void storeDirOpenSwapLogs(void);
 SQUIDCEXTERN void storeDirSwapLog(const StoreEntry *, int op);
 SQUIDCEXTERN void storeDirLRUDelete(StoreEntry *);
 SQUIDCEXTERN void storeDirLRUAdd(StoreEntry *);
 SQUIDCEXTERN int storeDirGetBlkSize(const char *path, int *blksize);
 SQUIDCEXTERN int storeDirGetUFSStats(const char *, int *, int *, int *, int *);
 
 /// manages a single cache_dir
 class SwapDir : public Store
 {
 
 public:
 typedef RefCount Pointer;
 
 SwapDir(char const *aType);
 virtual ~SwapDir();
-virtual void reconfigure(int, char *) = 0;
+virtual void reconfigure() = 0;
 char const *type() const;
 
 virtual bool needsDiskStrand() const; ///< needs a dedicated kid process
 virtual bool active() const; ///< may be used in this strand
 /// whether stat should be reported by this SwapDir
 virtual bool doReportStat() const { return active(); }
 
 /* official Store interface functions */
 virtual void diskFull();
 
 virtual StoreEntry * get(const cache_key *);
 
 virtual void get(String const, STOREGETCLIENT, void * cbdata);
 
 virtual uint64_t maxSize() const { return max_size;}
 
 virtual uint64_t minSize() const;
 
 virtual int64_t maxObjectSize() const { return max_objsize; }
 

=== modified file 'src/cache_cf.cc'
--- src/cache_cf.cc	2011-09-20 15:56:52 +
+++ src/cache_cf.cc	2011-09-20 16:20:46 +
@@ -1942,41 +1942,41 @@ parse_cachedir(SquidConfig::_cacheSwap *
 
 for (i = 0; i < swap->n_configured; i++) {
 assert (swap->swapDirs[i].getRaw());
 
 if ((strcasecmp(path_str, dynamic_cast(swap->swapDirs[i].getRaw())->path)) == 0) {
 /* this is specific to on-fs Stores. The right
  * way to handle this is probably to have a mapping
  * from paths to stores, and have on-fs stores
  * register with that, and lookip in that in their
  * own setup logic. RBC 20041225. TODO.
  */
 
 sd = dynamic_cast(swap->swapDirs[i].getRaw());
 
 if (strcmp(sd->type(), StoreFileSystem::FileSystems().items[fs]->type()) != 0) {
 debugs(3, 0, "ERROR: Can't change type of existing cache_dir " <<
sd->type() << " " << sd->path << " to " << type_str << ". Restart required");
 return;
 }
 
-sd->reconfigure (i, path_str);
+sd->reconfigure();
 
 update_maxobjsize();
 
 return;
 }
 }
 
 /* new cache_dir */
 if (swap->n_configured > 63) {
 /* 7 bits, signed */
 debugs(3, DBG_CRITICAL, "WARNING: There is a fixed maximum of 63 cache_dir entries Squid can handle.");
 debugs(3, DBG_CRITICAL, "WARNING: '" << path_str << "' is one to many.");
 self_destruct();
 return;
 }
 
 allocate_new_swapdir(swap);
 
 swap->swapDirs[swap->n_configured] = StoreFileSystem::FileSystems().items[fs]->createSwapDir();
 

=== modified file 'src/fs/coss/CossSwapDir.h'
--- src/fs/coss/CossSwapDir.h	2011-04-28 12:23:55 +
+++ src/fs/coss/CossSwapDir.h	2011-09-20 16:19:02 +
@@ -31,41 +31,41 @@ class CossSwapDir : public SwapDir, publ
 public:
 CossSwapDir();
 virtual void init();
 virtual void create();
 virtual void dump(StoreEntry &)const;
 ~CossSwapDir();
 virtual StoreSearch *search(String const url, HttpRequest *);
 virtual void unlink (StoreEntry &);
 virtual void statfs (StoreEntry &)const;
 virtual bool canStore(const StoreEntry &e, int64_t diskSpaceNeeded, int &load) const;
 virtual int callback();
 virtual void sync();
 virtual StoreIOState::Pointer createStoreIO(StoreEntry &, StoreIOState::STFNCB *, StoreIOState::STIOCB *, void *);
 virtual StoreIOState::Pointer openStoreIO(StoreEntry &, StoreIOSt

Re: [PATCH] Fix session helper "crashing too rapidly"

[Andrew Beverley]

On Tue, 2011-09-20 at 12:09 +1200, Amos Jeffries wrote:
> > Do you mean the -d option to the Squid binary? If so, this doesn't 
> > seem
> > to make any difference; it just prints all the log messages to the
> > display as well as the log file.
> 
>  -d parameter of the helper binary. stderr is piped to cache.log at 
>  level-0. Thus the need for a separate switch to enable lots of debug 
>  from the helper.

Sorry if I'm being stupid, but do you mean the actual session helper
binary or something else? If it's the former, then that obviously just
causes an exit due to an invalid option.

> > Done. Also, the following page should be updated:
> >
> > http://wiki.squid-cache.org/ConfigExamples/Portal/Splash
> >
> > I'm happy to do it myself, if you can give me wiki edit rights?
> 
>  Sure. Just need to know what username you login with.

Great stuff. AndrewBeverley

>  -h is reserved for display of "help" / usage() texts.
> 

Good point.

>  -T would probably be better for an absolute timeout.
> 

Changed as suggested.

>  ext_session_acl.8:
>   * " (unless -h is specified).". Please make a separate sentence. 
>  Something like "Also supports fixed length timeouts for regular splash 
>  page displays."

Done.

>(if you can think of any other useful scenario than regular splash 
>  pages that can be mentioned too.)

Well I've mentioned the possibility of forcing a user to
re-authenticate, but I'm not sure that I'm correct in saying that. Do
you think that is possible?

>   * rather than calling the new mode "Hard timeout". It's a "Periodic" 
>  or "Fixed length" timeout ... something a bit more descriptive like 
>  that.

Done.

>   ** Needs to explain how it interacts with -a mode. ie I would expect 
>  LOGOUT or timeout to terminate the session. Explicit LOGIN required for 
>  a new one to start.

Done.

>  ext_session_acl.cc: (modulo the -h ==> -T change requested)
>   * in getopt() format syntax ':' indicates a value is received for the 
>  option. What you coded is "t:b:a?h?".
>   * I think you should make -t and -T both optional arguments which are 
>  interchangeable.   ("t:T:b:a?")

Done.

Please find updated patch attached.

Andy

This patch adds some additional error messages when concurrency has
not been specified for the session helper.

It also adds a -T timeout option, which gives a fixed-length timeout,
rather than one that times out on user inactivity.

=== modified file 'helpers/external_acl/session/ext_session_acl.8'
--- helpers/external_acl/session/ext_session_acl.8	2010-09-16 13:07:02 +
+++ helpers/external_acl/session/ext_session_acl.8	2011-09-20 21:22:00 +
@@ -1,11 +1,11 @@
-.if !'po4a'hide' .TH ext_session_acl 8 "19 March 2006"
+.if !'po4a'hide' .TH ext_session_acl 8 "19 September 2011"
 .
 .SH NAME
 .if !'po4a'hide' .B ext_session_acl
 .if !'po4a'hide' \-
 Squid session tracking external acl helper.
 .PP
-Version 1.0
+Version 1.1
 .
 .SH SYNOPSIS
 .if !'po4a'hide' .B ext_session_acl
@@ -18,23 +18,39 @@
 .SH DESCRIPTION
 .B ext_session_acl
 maintains a concept of sessions by monitoring requests
-and timing out sessions if no requests have been seen for the idle timeout
-timer.
-.PP
-Intended use is for displaying "terms of use" pages, ad popups etc.
+and timing out sessions. The timeout is based either on idle use (
+.B -t
+) or a fixed period of time (
+.B -T
+). The former is suitable for displaying terms and conditions to a user; the
+latter is suitable for the display of advertisments or other notices (both as a
+splash page - see config examples online). The session helper can also be used
+to force users to re-authenticate.
 .
 .SH OPTIONS
 .if !'po4a'hide' .TP 12
 .if !'po4a'hide' .B "\-t timeout"
-.B Timeout
-for any session. If not specified the default is 3600 seconds.
+Idle timeout for any session. The default if not specified (set to 3600 seconds).
+.
+.if !'po4a'hide' .TP
+.if !'po4a'hide' .B "\-T timeout"
+Fixed timeout for any session. This will end the session after the timeout regardless
+of a user's activity. If used with
+.B active
+mode, this will terminate the user's session after
+.B timeout
+, after which another
+.B LOGIN
+will be required.
+.B LOGOUT
+will reset the session and timeout.
 .
 .if !'po4a'hide' .TP
 .if !'po4a'hide' .B "\-b path"
 .B Path
 to persistent database. If not specified the session details
 will be kept in memory only and all sessions will reset each time
-Squid restarts it's helpers (Squid restart or rotation of logs).
+Squid restarts its helpers (Squid restart or rotation of logs).
 .
 .if !'po4a'hide' .TP
 .if !'po4a'hide' .B \-a
@@ -43,12 +59,16 @@
 .B LOGIN
 , or terminated by the argument
 .B LOGOUT
-.PP
 Without this flag the helper automatically starts the session after
 the first request.
-.
 .SH CONFIGURATION
 .PP
+The
+.B ext_session_acl
+helper is a concurrent helper; therefore, the concurrency= option
+.B must
+be specified in the configuration.
+.PP
 Configuration example using the default automatic mode
 .if !'po4

server-side pconn races no longer retried after r10057.1.8

[Alex Rousskov]

Hello,

I am getting ERR_ZERO_SIZE_OBJECT during v3.2 performance tests. I
believe it is caused by Squid server-side failing when the server closes
a persistent connection before reading our second request. We used to
retry in those cases, as RFC 2616 prescribes.

It looks like we stopped retrying after the following change:

> revno: 10057.1.8
> branch nick: cleanup-comm
> timestamp: Wed 2010-05-19 23:28:21 +1200
> message:
>   Comm restructure part 2 - outbound connections

The relevant change in FwdState::retryOrBail() logic is below. First, we
unconditionally remove the server already tried:

> +paths.shift(); // last one failed. try another.

Then we are retrying only using the next hop, if any:

> +if (paths.size() > 0) {
   ... retry using the next path ...
> +}
> +// else bail. no more paths possible to try.

The old code used to retry using the current server if there were no
other servers to try.

When we are dealing with a pconn race condition, we should not throw the
current server/path/next hop/whatever away. We should retry by opening a
fresh connection the same server/path/next_hop/whatever.

[ I suspect the old code was not exactly doing the right thing when
multiple servers were present either, but it did work correctly in a
typical "single server" case when a pconn race was encountered. ]

A simple change fixing pconn retrials in the new code may go along these
lines:

if (paths.size() > 1) // if we can
paths.shift(); // try another.
// else retry using the same path

but I am not sure whether (a) that would be OK for all other kinds of
retries and (b) would not cause excessive retries since paths will never
be emptied in retryOrBail(). Is there a better way to fix this?


Thank you,

Alex.

Re: [PATCH] Fix session helper "crashing too rapidly"

[Amos Jeffries]

On Tue, 20 Sep 2011 22:26:20 +0100, Andrew Beverley wrote:

On Tue, 2011-09-20 at 12:09 +1200, Amos Jeffries wrote:

> Do you mean the -d option to the Squid binary? If so, this doesn't
> seem
> to make any difference; it just prints all the log messages to the
> display as well as the log file.

 -d parameter of the helper binary. stderr is piped to cache.log at
 level-0. Thus the need for a separate switch to enable lots of 
debug

 from the helper.


Sorry if I'm being stupid, but do you mean the actual session helper
binary or something else? If it's the former, then that obviously 
just

causes an exit due to an invalid option.



Yes I did. Sorry, being unbelievably stupid this week it seems :(.
Completely overlooked the fact the 'd' parameter and debugs do not 
exist there.



> Done. Also, the following page should be updated:
>
> http://wiki.squid-cache.org/ConfigExamples/Portal/Splash
>
> I'm happy to do it myself, if you can give me wiki edit rights?



Enabled.



   (if you can think of any other useful scenario than regular 
splash

 pages that can be mentioned too.)


Well I've mentioned the possibility of forcing a user to
re-authenticate, but I'm not sure that I'm correct in saying that. Do
you think that is possible?


Mostly the helper is just authorization. I know the distinction between 
authorize and authenticate is a bit narrow here, but this helper in 
particular rides that line with great precision. It will need some 
caveats: "to re-authenticate if %LOGIN parameter and active mode (-a) 
are both used.".


Applied the patch with that caveat.

There is quite a bit that could be said about those use-cases. Not sure 
if the wiki or the manual is the best place right now. Probably both in 
the end.


Amos

Re: server-side pconn races no longer retried after r10057.1.8

[Amos Jeffries]

On Tue, 20 Sep 2011 16:35:02 -0600, Alex Rousskov wrote:

Hello,

I am getting ERR_ZERO_SIZE_OBJECT during v3.2 performance tests. 
I
believe it is caused by Squid server-side failing when the server 
closes

a persistent connection before reading our second request. We used to
retry in those cases, as RFC 2616 prescribes.

It looks like we stopped retrying after the following change:


revno: 10057.1.8
branch nick: cleanup-comm
timestamp: Wed 2010-05-19 23:28:21 +1200
message:
  Comm restructure part 2 - outbound connections


The relevant change in FwdState::retryOrBail() logic is below. First, 
we

unconditionally remove the server already tried:


+paths.shift(); // last one failed. try another.


Then we are retrying only using the next hop, if any:


+if (paths.size() > 0) {

   ... retry using the next path ...

+}
+// else bail. no more paths possible to try.


The old code used to retry using the current server if there were no
other servers to try.

When we are dealing with a pconn race condition, we should not throw 
the
current server/path/next hop/whatever away. We should retry by 
opening a

fresh connection the same server/path/next_hop/whatever.

[ I suspect the old code was not exactly doing the right thing when
multiple servers were present either, but it did work correctly in a
typical "single server" case when a pconn race was encountered. ]


The current code was not exactly a change in this area. Retrying the 
server was not actually doing anything previously either. The full 
peer-select cycle was repeating on every retry. Which reset the state 
about what had already been tried and hid a few of these cases.




A simple change fixing pconn retrials in the new code may go along 
these

lines:

if (paths.size() > 1) // if we can
paths.shift(); // try another.
// else retry using the same path

but I am not sure whether (a) that would be OK for all other kinds of
retries and (b) would not cause excessive retries since paths will 
never

be emptied in retryOrBail(). Is there a better way to fix this?


Hmm, only if we have no other choice. In the worst-case this will make 
all other CANNOT_FORWARD cases "hang" until forward_timeout has passed.



Basing it on the error type/code/something would be better.

There are three sets of cases feeding into this function (maybe more if 
other races exist).
 a) server error responses 5xx/403. new path required before 
connectStart().

 b) TCP connection failures. new path required before connectStart().
 c) this pconn race. same path can be passed back to connectStart() 
_once_.


Cycles of repetition trying the same path on any of the (a) and (b) 
cases is a very bad idea for performance.


At this point it should have (local.GetPort() > 0) from a previously 
open connection (c or a) [NP: fd will be -1 already]. So checking for 
the zero-sized error AND a present local port we could unset the local 
port, and call connectStart() without the shift().


Amos

Re: [PATCH] Ignore and warn about attempts to reconfigure static Rock store options.

[Amos Jeffries]

+1. Looks fine now.


Amos

Re: [PATCH] Remove SwapDir::reconfigure() arguments since they are not used.

[Amos Jeffries]

On Tue, 20 Sep 2011 20:32:56 +0400, Dmitry Kurochkin wrote:

Hi all.

This is a small cleanup patch discussed recently in another thread.
Note it depends on rock-reconfigure-t3.patch.


Before the change, SwapDir::reconfigure() took index and path
arguments.  But none of them was actually used: neither index nor 
path

can be changed during reconfigure.  And both index and path are
available as SwapDir members.  So there is no reason to have these
arguments.


Regards,
  Dmitry



+1. Please commit asap.

Amos

i'm having a problem while compiling squid 3.2.0.12 (since 3.2.0.6) on ubuntu server

[Eliezer Croitoru]

using ubuntu server 10.04 latest updates applied.
compiling squid 3.2.0.5 works great but since then tried to compile and 
couldnt make it compile.

i understood that on debian 6 it works great but i didnt check it yet.

the following software was installed in order to make squid 3.1 and 3.2 
compile:
apt-get install build-essential libldap2-dev libpam0g-dev libdb-dev 
dpatch cdbs libsasl2-dev debhelper libcppunit-dev libkrb5-dev comerr-dev 
libcap2-dev libexpat1-dev libxml2-dev libcap2-dev dpkg-dev


config..(worked for 3.2.0.5 and still running for at least since it was 
released a very long time):
./configure --prefix=/opt/squid32012 --includedir=/include 
--mandir=/share/man --infodir=/share/info 
--localstatedir=/opt/squid32012/var --disable-maintainer-mode 
--disable-dependency-tracking --disable-silent-rules --enable-inline 
--enable-async-io=8 --enable-storeio=ufs,aufs,diskd 
--enable-removal-policies=lru,heap --enable-delay-pools 
--enable-cache-digests --enable-underscores --enable-icap-client 
--enable-follow-x-forwarded-for 
--enable-digest-auth-helpers=ldap,password 
--enable-negotiate-auth-helpers=squid_kerb_auth 
--enable-external-acl-helpers=ip_user,ldap_group,session,unix_group,wbinfo_group 
--enable-arp-acl --enable-esi--disable-translation 
--with-logdir=/opt/squid32012/var/log 
--with-pidfile=/var/run/squid32012.pid --with-filedescriptors=65536 
--with-large-files --with-default-user=proxy --enable-linux-netfilter 
--enable-ltdl-convenience


thsnks in advance
Eliezer

the error is:

In file included from ../src/ssl/support.h:38,
 from ssl/ErrorDetailManager.h:4,
 from errorpage.cc:42:
../src/ssl/gadgets.h:39: error: variable or field גX509_free_cppג 
declared void

../src/ssl/gadgets.h:39: error: גX509ג was not declared in this scope
../src/ssl/gadgets.h:39: error: גaג was not declared in this scope
../src/ssl/gadgets.h:40: error: גX509ג was not declared in this scope
../src/ssl/gadgets.h:40: error: גX509_free_cppג was not declared in this 
scope

../src/ssl/gadgets.h:40: error: template argument 1 is invalid
../src/ssl/gadgets.h:40: error: template argument 2 is invalid
../src/ssl/gadgets.h:40: error: invalid type in declaration before ג;ג token
../src/ssl/gadgets.h:42: error: variable or field גEVP_PKEY_free_cppג 
declared void

../src/ssl/gadgets.h:42: error: גEVP_PKEYג was not declared in this scope
../src/ssl/gadgets.h:42: error: גaג was not declared in this scope
../src/ssl/gadgets.h:43: error: גEVP_PKEYג was not declared in this scope
../src/ssl/gadgets.h:43: error: גEVP_PKEY_free_cppג was not declared in 
this scope

../src/ssl/gadgets.h:43: error: template argument 1 is invalid
../src/ssl/gadgets.h:43: error: template argument 2 is invalid
../src/ssl/gadgets.h:43: error: invalid type in declaration before ג;ג token
../src/ssl/gadgets.h:45: error: variable or field גBN_free_cppג declared 
void

../src/ssl/gadgets.h:45: error: גBIGNUMג was not declared in this scope
../src/ssl/gadgets.h:45: error: גaג was not declared in this scope
../src/ssl/gadgets.h:46: error: גBIGNUMג was not declared in this scope
../src/ssl/gadgets.h:46: error: גBN_free_cppג was not declared in this scope
../src/ssl/gadgets.h:46: error: template argument 1 is invalid
../src/ssl/gadgets.h:46: error: template argument 2 is invalid
../src/ssl/gadgets.h:46: error: invalid type in declaration before ג;ג token
../src/ssl/gadgets.h:48: error: variable or field גBIO_free_cppג 
declared void

../src/ssl/gadgets.h:48: error: גBIOג was not declared in this scope
../src/ssl/gadgets.h:48: error: גaג was not declared in this scope
../src/ssl/gadgets.h:49: error: גBIOג was not declared in this scope
../src/ssl/gadgets.h:49: error: גBIO_free_cppג was not declared in this 
scope

../src/ssl/gadgets.h:49: error: template argument 1 is invalid
../src/ssl/gadgets.h:49: error: template argument 2 is invalid
../src/ssl/gadgets.h:49: error: invalid type in declaration before ג;ג token
../src/ssl/gadgets.h:51: error: variable or field 
גASN1_INTEGER_free_cppג declared void
../src/ssl/gadgets.h:51: error: גASN1_INTEGERג was not declared in this 
scope

../src/ssl/gadgets.h:51: error: גaג was not declared in this scope
../src/ssl/gadgets.h:52: error: גASN1_INTEGERג was not declared in this 
scope
../src/ssl/gadgets.h:52: error: גASN1_INTEGER_free_cppג was not declared 
in this scope

../src/ssl/gadgets.h:52: error: template argument 1 is invalid
../src/ssl/gadgets.h:52: error: template argument 2 is invalid
../src/ssl/gadgets.h:52: error: invalid type in declaration before ג;ג token
../src/ssl/gadgets.h:54: error: variable or field גTXT_DB_free_cppג 
declared void

../src/ssl/gadgets.h:54: error: גTXT_DBג was not declared in this scope
../src/ssl/gadgets.h:54: error: גaג was not declared in this scope
../src/ssl/gadgets.h:55: error: גTXT_DBג was not declared in this scope
../src/ssl/gadgets.h:55: error: גTXT_DB_free_cppג was not declared in 
this scope

../src/ssl/gadgets.h:55: error: template argum

Re: server-side pconn races no longer retried after r10057.1.8

[Alex Rousskov]

On 09/20/2011 05:57 PM, Amos Jeffries wrote:
> 
> Basing it on the error type/code/something would be better.
> 
> There are three sets of cases feeding into this function (maybe more if
> other races exist).
>  a) server error responses 5xx/403. new path required before
> connectStart().
>  b) TCP connection failures. new path required before connectStart().
>  c) this pconn race. same path can be passed back to connectStart() _once_.


Agreed, except the "can be" is actually "should or even must be" in (c).
It is wrong to serve an error to the user in this case.


> Cycles of repetition trying the same path on any of the (a) and (b)
> cases is a very bad idea for performance.
> 
> At this point it should have (local.GetPort() > 0) from a previously
> open connection (c or a) [NP: fd will be -1 already]. So checking for
> the zero-sized error AND a present local port we could unset the local
> port, and call connectStart() without the shift().

A positive local.GetPort() check only tells us whether we had used a
connection, not whether we had reused a persistent connection, right?

How about adding an explicit flag.avoidPconn[NextTime] or similar and
setting it as sketched below?

FwdState::connectStart():
...
fwdPconnPool->pop(..., !flag.avoidPconn && checkRetriable());
flag.avoidPconn = we are reusing pconn;

And any time we change destination:

serverDestinations.shift();
flag.avoidPconn = false;

As the above sketch illustrates, the same flag can then be used in
FwdState::connectStart() to _avoid_ using a pconn if we are retrying the
same destination.

I do not like that we would have to remember to reset this flag every
time we shift the serverDestinations array. Perhaps that logic should be
moved into a dedicated FwdState method.


Thank you,

Alex.

Re: i'm having a problem while compiling squid 3.2.0.12 (since 3.2.0.6) on ubuntu server

[Alex Rousskov]

On 09/20/2011 10:35 PM, Eliezer Croitoru wrote:
> using ubuntu server 10.04 latest updates applied.
> compiling squid 3.2.0.5 works great but since then tried to compile and
> couldnt make it compile.
> i understood that on debian 6 it works great but i didnt check it yet.
> 
> the following software was installed in order to make squid 3.1 and 3.2
> compile:
> apt-get install build-essential libldap2-dev libpam0g-dev libdb-dev
> dpatch cdbs libsasl2-dev debhelper libcppunit-dev libkrb5-dev comerr-dev
> libcap2-dev libexpat1-dev libxml2-dev libcap2-dev dpkg-dev
> 
> config..(worked for 3.2.0.5 and still running for at least since it was
> released a very long time):
> ./configure --prefix=/opt/squid32012 --includedir=/include
> --mandir=/share/man --infodir=/share/info
> --localstatedir=/opt/squid32012/var --disable-maintainer-mode
> --disable-dependency-tracking --disable-silent-rules --enable-inline
> --enable-async-io=8 --enable-storeio=ufs,aufs,diskd
> --enable-removal-policies=lru,heap --enable-delay-pools
> --enable-cache-digests --enable-underscores --enable-icap-client
> --enable-follow-x-forwarded-for
> --enable-digest-auth-helpers=ldap,password
> --enable-negotiate-auth-helpers=squid_kerb_auth
> --enable-external-acl-helpers=ip_user,ldap_group,session,unix_group,wbinfo_group
> --enable-arp-acl --enable-esi--disable-translation
> --with-logdir=/opt/squid32012/var/log
> --with-pidfile=/var/run/squid32012.pid --with-filedescriptors=65536
> --with-large-files --with-default-user=proxy --enable-linux-netfilter
> --enable-ltdl-convenience
> 
> thsnks in advance
> Eliezer
> 
> the error is:
> 
> In file included from ../src/ssl/support.h:38,
>  from ssl/ErrorDetailManager.h:4,
>  from errorpage.cc:42:
> ../src/ssl/gadgets.h:39: error: variable or field גX509_free_cppג
> declared void

I believe I have seen this bug as well. Try installing OpenSSL
libraries, including development headers (openssl-devel package or similar).

Please consider filing this bug using Squid bugzilla. Please mention
whether installing OpenSSL helped.

For the record, explicitly disabling SSL support in Squid does not help
in this case. This is a bug.


Thank you,

Alex.




> ../src/ssl/gadgets.h:39: error: גX509ג was not declared in this scope
> ../src/ssl/gadgets.h:39: error: גaג was not declared in this scope
> ../src/ssl/gadgets.h:40: error: גX509ג was not declared in this scope
> ../src/ssl/gadgets.h:40: error: גX509_free_cppג was not declared in this
> scope
> ../src/ssl/gadgets.h:40: error: template argument 1 is invalid
> ../src/ssl/gadgets.h:40: error: template argument 2 is invalid
> ../src/ssl/gadgets.h:40: error: invalid type in declaration before ג;ג
> token
> ../src/ssl/gadgets.h:42: error: variable or field גEVP_PKEY_free_cppג
> declared void
> ../src/ssl/gadgets.h:42: error: גEVP_PKEYג was not declared in this scope
> ../src/ssl/gadgets.h:42: error: גaג was not declared in this scope
> ../src/ssl/gadgets.h:43: error: גEVP_PKEYג was not declared in this scope
> ../src/ssl/gadgets.h:43: error: גEVP_PKEY_free_cppג was not declared in
> this scope
> ../src/ssl/gadgets.h:43: error: template argument 1 is invalid
> ../src/ssl/gadgets.h:43: error: template argument 2 is invalid
> ../src/ssl/gadgets.h:43: error: invalid type in declaration before ג;ג
> token
> ../src/ssl/gadgets.h:45: error: variable or field גBN_free_cppג declared
> void
> ../src/ssl/gadgets.h:45: error: גBIGNUMג was not declared in this scope
> ../src/ssl/gadgets.h:45: error: גaג was not declared in this scope
> ../src/ssl/gadgets.h:46: error: גBIGNUMג was not declared in this scope
> ../src/ssl/gadgets.h:46: error: גBN_free_cppג was not declared in this
> scope
> ../src/ssl/gadgets.h:46: error: template argument 1 is invalid
> ../src/ssl/gadgets.h:46: error: template argument 2 is invalid
> ../src/ssl/gadgets.h:46: error: invalid type in declaration before ג;ג
> token
> ../src/ssl/gadgets.h:48: error: variable or field גBIO_free_cppג
> declared void
> ../src/ssl/gadgets.h:48: error: גBIOג was not declared in this scope
> ../src/ssl/gadgets.h:48: error: גaג was not declared in this scope
> ../src/ssl/gadgets.h:49: error: גBIOג was not declared in this scope
> ../src/ssl/gadgets.h:49: error: גBIO_free_cppג was not declared in this
> scope
> ../src/ssl/gadgets.h:49: error: template argument 1 is invalid
> ../src/ssl/gadgets.h:49: error: template argument 2 is invalid
> ../src/ssl/gadgets.h:49: error: invalid type in declaration before ג;ג
> token
> ../src/ssl/gadgets.h:51: error: variable or field
> גASN1_INTEGER_free_cppג declared void
> ../src/ssl/gadgets.h:51: error: גASN1_INTEGERג was not declared in this
> scope
> ../src/ssl/gadgets.h:51: error: גaג was not declared in this scope
> ../src/ssl/gadgets.h:52: error: גASN1_INTEGERג was not declared in this
> scope
> ../src/ssl/gadgets.h:52: error: גASN1_INTEGER_free_cppג was not declared
> in this scope
> ../src/ssl/gadgets.h:52: error: template argument 1 is invalid
> ..

Re: SSL Bump Certificate Blacklist

[Amos Jeffries]

On 21/09/11 03:05, Alex Rousskov wrote:

On 09/20/2011 05:46 AM, Fabian Hugelshofer wrote:

On 09/16/2011 04:35 PM, Alex Rousskov wrote:

On 09/16/2011 12:55 AM, Fabian Hugelshofer wrote:

What would you use to identify the certificates?
- Serial number?
- Serial number and common name?
- Serial number and issuer?
- Fingerprint (might not be available in each case)?



I hope somebody already answered these important questions in general
CRL context!


CRLs use the serial number in combination with the issuer.


The issuer name and the serial are probably not sufficient as there can
be multiple CAs with the same name (certificates with different
validity, different trust chains, ...).



If we implement a custom black list, we can be flexible and support all
of the above. To start with, I guess we should block all certificates
with DigiNotar in the chain.



What are the next steps, would you like me to open a Squid Bug?


I believe the rule is that new features should be documented on wiki,
but opening a "feature request" bug report would not hurt, I guess.

Can you summarize the minimum number of new ACL types that you think we
should support to make this work? I know we want issuer name and serial
number. Do we want notBefore and notAfter matching? Anything else?

Do we need to be able to test a match against a certificate at Nth
position in the chain? Or can we always test all certificates in the chain?


If this is to be worth using we definitely need to block any chains 
containing a blacklisted certificate at any level. Allowing sites to 
add/remove a link of chain and avoid the blacklist voids the utility of 
this feature.





It would be nice to hear from others regarding this issue to make sure
we are not missing something important.



For my part I'm not versed enough in the details to have a strong 
opinion either way. I thought the CRL infrastructure performed this task 
sufficiently.


Amos
--
Please be using
  Current Stable Squid 2.7.STABLE9 or 3.1.15
  Beta testers wanted for 3.2.0.12

Re: i'm having a problem while compiling squid 3.2.0.12 (since 3.2.0.6) on ubuntu server

[Amos Jeffries]

On 21/09/11 17:02, Alex Rousskov wrote:

On 09/20/2011 10:35 PM, Eliezer Croitoru wrote:

using ubuntu server 10.04 latest updates applied.
compiling squid 3.2.0.5 works great but since then tried to compile and
couldnt make it compile.
i understood that on debian 6 it works great but i didnt check it yet.

the following software was installed in order to make squid 3.1 and 3.2
compile:
apt-get install build-essential libldap2-dev libpam0g-dev libdb-dev
dpatch cdbs libsasl2-dev debhelper libcppunit-dev libkrb5-dev comerr-dev
libcap2-dev libexpat1-dev libxml2-dev libcap2-dev dpkg-dev

config..(worked for 3.2.0.5 and still running for at least since it was
released a very long time):
./configure --prefix=/opt/squid32012 --includedir=/include
--mandir=/share/man --infodir=/share/info
--localstatedir=/opt/squid32012/var --disable-maintainer-mode
--disable-dependency-tracking --disable-silent-rules --enable-inline
--enable-async-io=8 --enable-storeio=ufs,aufs,diskd
--enable-removal-policies=lru,heap --enable-delay-pools
--enable-cache-digests --enable-underscores --enable-icap-client
--enable-follow-x-forwarded-for
--enable-digest-auth-helpers=ldap,password
--enable-negotiate-auth-helpers=squid_kerb_auth
--enable-external-acl-helpers=ip_user,ldap_group,session,unix_group,wbinfo_group
--enable-arp-acl --enable-esi--disable-translation
--with-logdir=/opt/squid32012/var/log
--with-pidfile=/var/run/squid32012.pid --with-filedescriptors=65536
--with-large-files --with-default-user=proxy --enable-linux-netfilter
--enable-ltdl-convenience

thsnks in advance
Eliezer

the error is:

In file included from ../src/ssl/support.h:38,
  from ssl/ErrorDetailManager.h:4,
  from errorpage.cc:42:
../src/ssl/gadgets.h:39: error: variable or field גX509_free_cppג
declared void


I believe I have seen this bug as well. Try installing OpenSSL
libraries, including development headers (openssl-devel package or similar).

Please consider filing this bug using Squid bugzilla. Please mention
whether installing OpenSSL helped.


And which version(s), down to the sub-lettering. There seem to be some 
fun changes going on in the headers for the 1.0.0 releases.




For the record, explicitly disabling SSL support in Squid does not help
in this case. This is a bug.


Thank you,

Alex.



Thank you.

Amos
--
Please be using
  Current Stable Squid 2.7.STABLE9 or 3.1.15
  Beta testers wanted for 3.2.0.12

Re: server-side pconn races no longer retried after r10057.1.8

[Amos Jeffries]

On 21/09/11 16:56, Alex Rousskov wrote:

On 09/20/2011 05:57 PM, Amos Jeffries wrote:


Basing it on the error type/code/something would be better.

There are three sets of cases feeding into this function (maybe more if
other races exist).
  a) server error responses 5xx/403. new path required before
connectStart().
  b) TCP connection failures. new path required before connectStart().
  c) this pconn race. same path can be passed back to connectStart() _once_.


Er, um,   s/ _once_//



Agreed, except the "can be" is actually "should or even must be" in (c).
It is wrong to serve an error to the user in this case.



Yes.




Cycles of repetition trying the same path on any of the (a) and (b)
cases is a very bad idea for performance.

At this point it should have (local.GetPort()>  0) from a previously
open connection (c or a) [NP: fd will be -1 already]. So checking for
the zero-sized error AND a present local port we could unset the local
port, and call connectStart() without the shift().


A positive local.GetPort() check only tells us whether we had used a
connection, not whether we had reused a persistent connection, right?


Yes it tells us the conn was previously successful at opening. So I 
believe it should be the distinguishing difference between (a) and (b) 
cases. The ZERO_SIZED error differentiates between (c) and (a|b).




How about adding an explicit flag.avoidPconn[NextTime] or similar and
setting it as sketched below?

FwdState::connectStart():
 ...
 fwdPconnPool->pop(..., !flag.avoidPconn&&  checkRetriable());
 flag.avoidPconn = we are reusing pconn;


The problem was the last pconn, we may have N perfectly fine ones to use 
on the list.




And any time we change destination:

 serverDestinations.shift();
 flag.avoidPconn = false;

As the above sketch illustrates, the same flag can then be used in
FwdState::connectStart() to _avoid_ using a pconn if we are retrying the
same destination.

I do not like that we would have to remember to reset this flag every
time we shift the serverDestinations array. Perhaps that logic should be
moved into a dedicated FwdState method.


Yes, a flag is possible if we need to go that way. I think we can get 
the same information from local port>0 for free without any state 
maintenance overheads though. And we only need to use it at this one 
point where the cases are clear.


Amos
--
Please be using
  Current Stable Squid 2.7.STABLE9 or 3.1.15
  Beta testers wanted for 3.2.0.12

Re: i'm having a problem while compiling squid 3.2.0.12 (since 3.2.0.6) on ubuntu server

[Eliezer Croitoru]

On 21/09/2011 08:29, Amos Jeffries wrote:

On 21/09/11 17:02, Alex Rousskov wrote:

On 09/20/2011 10:35 PM, Eliezer Croitoru wrote:

using ubuntu server 10.04 latest updates applied.
compiling squid 3.2.0.5 works great but since then tried to compile and
couldnt make it compile.
i understood that on debian 6 it works great but i didnt check it yet.

the following software was installed in order to make squid 3.1 and 3.2
compile:
apt-get install build-essential libldap2-dev libpam0g-dev libdb-dev
dpatch cdbs libsasl2-dev debhelper libcppunit-dev libkrb5-dev comerr-dev
libcap2-dev libexpat1-dev libxml2-dev libcap2-dev dpkg-dev

config..(worked for 3.2.0.5 and still running for at least since it was
released a very long time):
./configure --prefix=/opt/squid32012 --includedir=/include
--mandir=/share/man --infodir=/share/info
--localstatedir=/opt/squid32012/var --disable-maintainer-mode
--disable-dependency-tracking --disable-silent-rules --enable-inline
--enable-async-io=8 --enable-storeio=ufs,aufs,diskd
--enable-removal-policies=lru,heap --enable-delay-pools
--enable-cache-digests --enable-underscores --enable-icap-client
--enable-follow-x-forwarded-for
--enable-digest-auth-helpers=ldap,password
--enable-negotiate-auth-helpers=squid_kerb_auth
--enable-external-acl-helpers=ip_user,ldap_group,session,unix_group,wbinfo_group

--enable-arp-acl --enable-esi--disable-translation
--with-logdir=/opt/squid32012/var/log
--with-pidfile=/var/run/squid32012.pid --with-filedescriptors=65536
--with-large-files --with-default-user=proxy --enable-linux-netfilter
--enable-ltdl-convenience

thsnks in advance
Eliezer

the error is:

In file included from ../src/ssl/support.h:38,
from ssl/ErrorDetailManager.h:4,
from errorpage.cc:42:
../src/ssl/gadgets.h:39: error: variable or field גX509_free_cppג
declared void


I believe I have seen this bug as well. Try installing OpenSSL
libraries, including development headers (openssl-devel package or
similar).

Please consider filing this bug using Squid bugzilla. Please mention
whether installing OpenSSL helped.


And which version(s), down to the sub-lettering. There seem to be some
fun changes going on in the headers for the 1.0.0 releases.



For the record, explicitly disabling SSL support in Squid does not help
in this case. This is a bug.


Thank you,

Alex.



Thank you.

Amos


ok so i installed every possilbe openssl lib that i was thinking of using:
apt-get install libcurl4-openssl-dev libssl-dev libssl0.9.8 libssl0.9.8-dbg

and then i get the following output (some lines where added).:

make[3]: Entering directory `/opt/src/squid-3.2.0.12/src'
g++ -DHAVE_CONFIG_H 
-DDEFAULT_CONFIG_FILE=\"/opt/squid32012/etc/squid.conf\" 
-DDEFAULT_SQUID_DATA_DIR=\"/opt/squid32012/share\" 
-DDEFAULT_SQUID_CONFIG_DIR=\"/opt/squid32012/etc\"  -I.. -I../include 
-I../lib -I../src -I../include   -I../libltdl -I../src -I../libltdl 
-Wall -Wpointer-arith -Wwrite-strings -Wcomments -Werror -pipe 
-D_REENTRANT -m64 -g -O2 -c -o errorpage.o errorpage.cc

In file included from ../src/ssl/support.h:38,
 from ssl/ErrorDetailManager.h:4,
 from errorpage.cc:42:
../src/ssl/gadgets.h:39: error: variable or field גX509_free_cppג 
declared void

../src/ssl/gadgets.h:39: error: גX509ג was not declared in this scope
../src/ssl/gadgets.h:39: error: גaג was not declared in this scope
../src/ssl/gadgets.h:40: error: גX509ג was not declared in this scope
../src/ssl/gadgets.h:40: error: גX509_free_cppג was not declared in this 
scope

../src/ssl/gadgets.h:40: error: template argument 1 is invalid
../src/ssl/gadgets.h:40: error: template argument 2 is invalid
../src/ssl/gadgets.h:40: error: invalid type in declaration before ג;ג token
../src/ssl/gadgets.h:42: error: variable or field גEVP_PKEY_free_cppג 
declared void

../src/ssl/gadgets.h:42: error: גEVP_PKEYג was not declared in this scope
../src/ssl/gadgets.h:42: error: גaג was not declared in this scope
../src/ssl/gadgets.h:43: error: גEVP_PKEYג was not declared in this scope
../src/ssl/gadgets.h:43: error: גEVP_PKEY_free_cppג was not declared in 
this scope

../src/ssl/gadgets.h:43: error: template argument 1 is invalid
../src/ssl/gadgets.h:43: error: template argument 2 is invalid
../src/ssl/gadgets.h:43: error: invalid type in declaration before ג;ג token
../src/ssl/gadgets.h:45: error: variable or field גBN_free_cppג declared 
void

../src/ssl/gadgets.h:45: error: גBIGNUMג was not declared in this scope
../src/ssl/gadgets.h:45: error: גaג was not declared in this scope
../src/ssl/gadgets.h:46: error: גBIGNUMג was not declared in this scope
../src/ssl/gadgets.h:46: error: גBN_free_cppג was not declared in this scope
../src/ssl/gadgets.h:46: error: template argument 1 is invalid
../src/ssl/gadgets.h:46: error: template argument 2 is invalid
../src/ssl/gadgets.h:46: error: invalid type in declaration before ג;ג token
../src/ssl/gadgets.h:48: error: variable or field גBIO_free_cppג 
declared void

../src/ssl/gadgets.h:48: error: גB