Fixed DNS query leaks and increased defense against DNS cache poisoning. We were leaking (i.e. forgetting about) DNS queries under several conditions. The most realistic leak case would go like this: - We send UDP query1. No response. - We send UDP query2. The response for query1 comes, with TC bit. - We try to connect over TCP, sending TCP query3. The response for query2 comes, with TC bit, matching TCP query3 ID. Since we are waiting a response over TCP, we drop the UDP response, and delete the query from the queue. We leak. This change avoids forgetting the query under the above scenario. Moreover, the above steps are hiding another problem: we are accepting responses to timed out queries, making DNS cache poisoning easier. This change avoids that by using unique query ID for each sent query. We have also added an instance ID so that we still can track/identify a single "transaction" from Squid point of view, even when that transaction involves many DNS query messages. When we forget about a DNS query, the caller may get stuck, holding a cbdata lock. This is typical for ACLs that require domain name resolution, for example. On a busy server with a long ACL list, the lock counter keeps growing due to forgotten requests and may overflow, causing a "c->locks < 65535" assertion. This change fixes the assertion unless there are more DNS leaks or different lock leaks present. === modified file 'src/base/InstanceId.h' --- src/base/InstanceId.h 2010-10-13 00:14:42 +0000 +++ src/base/InstanceId.h 2010-10-28 14:22:17 +0000 @@ -6,47 +6,48 @@ /** Identifier for class instances * - unique IDs for a large number of concurrent instances, but may wrap; * - useful for debugging and insecure request/response matching; * - sequential IDs within a class except when wrapping; * - always positive IDs. * \todo: add storage type parameter to support configurable Value types? * \todo: add creation/destruction debugging? */ template class InstanceId { public: typedef unsigned int Value; ///< id storage type; \todo: parameterize? InstanceId(): value(++Last ? Last : ++Last) {} operator Value() const { return value; } bool operator ==(const InstanceId &o) const { return value == o.value; } bool operator !=(const InstanceId &o) const { return !(*this == o); } + void change() {value = ++Last ? Last : ++Last;} /// prints Prefix followed by ID value; \todo: use HEX for value printing? std::ostream &print(std::ostream &os) const; public: static const char *Prefix; ///< Class shorthand string for debugging - const Value value; ///< instance identifier + Value value; ///< instance identifier private: InstanceId(const InstanceId& right); ///< not implemented; IDs are unique InstanceId& operator=(const InstanceId &right); ///< not implemented private: static Value Last; ///< the last used ID value }; /// convenience macro to instantiate Class-specific stuff in .cc files #define InstanceIdDefinitions(Class, prefix) \ template<> std::ostream & \ InstanceId::print(std::ostream &os) const { \ return os << Prefix << value; \ } \ template<> const char *InstanceId::Prefix = prefix; \ template<> InstanceId::Value InstanceId::Last = 0 /// print the id === modified file 'src/dns_internal.cc' --- src/dns_internal.cc 2010-10-20 01:32:49 +0000 +++ src/dns_internal.cc 2010-10-28 14:20:40 +0000 @@ -28,40 +28,41 @@ * GNU General Public License for more details. * * You should have received a copy of the GNU General Public License * along with this program; if not, write to the Free Software * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111, USA. * */ #include "config.h" #include "squid.h" #include "event.h" #include "CacheManager.h" #include "SquidTime.h" #include "Store.h" #include "comm.h" #include "fde.h" #include "ip/tools.h" #include "MemBuf.h" #include "util.h" #include "wordlist.h" +#include "base/InstanceId.h" #if HAVE_ARPA_NAMESER_H #include #endif #if HAVE_RESOLV_H #include #endif /* MS Visual Studio Projects are monolithic, so we need the following #ifndef to exclude the internal DNS code from compile process when using external DNS process. */ #if !USE_DNSSERVERS #ifdef _SQUID_WIN32_ #include "squid_windows.h" #define REG_TCPIP_PARA_INTERFACES "SYSTEM\\CurrentControlSet\\Services\\Tcpip\\Parameters\\Interfaces" #define REG_TCPIP_PARA "SYSTEM\\CurrentControlSet\\Services\\Tcpip\\Parameters" #define REG_VXD_MSTCP "SYSTEM\\CurrentControlSet\\Services\\VxD\\MSTCP" #endif #ifndef _PATH_RESCONF @@ -107,61 +108,64 @@ "","","","","", /* RFC 2671 */ "Bad OPT Version or TSIG Signature Failure" }; typedef struct _idns_query idns_query; typedef struct _ns ns; typedef struct _sp sp; typedef struct _nsvc nsvc; struct _idns_query { hash_link hash; rfc1035_query query; char buf[RESOLV_BUFSZ]; char name[NS_MAXDNAME + 1]; char orig[NS_MAXDNAME + 1]; size_t sz; - unsigned short id; + unsigned short msg_id; /// random query ID sent to server; changes with every query sent + InstanceId xact_id; /// identifies our "transaction", stays constant when query is retried + int nsends; int need_vc; struct timeval start_t; struct timeval sent_t; struct timeval queue_t; dlink_node lru; IDNSCB *callback; void *callback_data; int attempt; int rcode; idns_query *queue; unsigned short domain; unsigned short do_searchpath; bool need_A; struct { int count; rfc1035_rr *answers; } initial_AAAA; }; +InstanceIdDefinitions(idns_query, "dns"); struct _nsvc { int ns; int fd; unsigned short msglen; int read_msglen; MemBuf *msg; MemBuf *queue; bool busy; }; struct _ns { Ip::Address S; int nqueries; int nreplies; #if WHEN_EDNS_RESPONSES_ARE_PARSED int last_seen_edns; #endif nsvc *vc; }; @@ -221,40 +225,41 @@ static void idnsParseNameservers(void); #ifndef _SQUID_MSWIN_ static void idnsParseResolvConf(void); #endif #ifdef _SQUID_WIN32_ static void idnsParseWIN32Registry(void); static void idnsParseWIN32SearchList(const char *); #endif static void idnsCacheQuery(idns_query * q); static void idnsSendQuery(idns_query * q); static IOCB idnsReadVCHeader; static void idnsDoSendQueryVC(nsvc *vc); static int idnsFromKnownNameserver(Ip::Address const &from); static idns_query *idnsFindQuery(unsigned short id); static void idnsGrokReply(const char *buf, size_t sz, int from_ns); static PF idnsRead; static EVH idnsCheckQueue; static void idnsTickleQueue(void); static void idnsRcodeCount(int, int); +static unsigned short idnsQueryID(void); static void idnsAddNameserver(const char *buf) { Ip::Address A; if (!(A = buf)) { debugs(78, 0, "WARNING: rejecting '" << buf << "' as a name server, because it is not a numeric IP address"); return; } if (A.IsAnyAddr()) { debugs(78, 0, "WARNING: Squid does not accept " << A << " in DNS server specifications."); A.SetLocalhost(); debugs(78, 0, "Will be using " << A << " instead, assuming you meant that DNS is running on the same machine"); } if (!Ip::EnableIpv6 && !A.SetIPv4()) { debugs(78, DBG_IMPORTANT, "WARNING: IPv6 is disabled. Discarding " << A << " in DNS server specifications."); return; @@ -643,41 +648,41 @@ #endif static void idnsStats(StoreEntry * sentry) { dlink_node *n; idns_query *q; int i; int j; char buf[MAX_IPSTRLEN]; storeAppendPrintf(sentry, "Internal DNS Statistics:\n"); storeAppendPrintf(sentry, "\nThe Queue:\n"); storeAppendPrintf(sentry, " DELAY SINCE\n"); storeAppendPrintf(sentry, " ID SIZE SENDS FIRST SEND LAST SEND\n"); storeAppendPrintf(sentry, "------ ---- ----- ---------- ---------\n"); for (n = lru_list.head; n; n = n->next) { q = (idns_query *)n->data; storeAppendPrintf(sentry, "%#06x %4d %5d %10.3f %9.3f\n", - (int) q->id, (int) q->sz, q->nsends, + (int) q->msg_id, (int) q->sz, q->nsends, tvSubDsec(q->start_t, current_time), tvSubDsec(q->sent_t, current_time)); } if (Config.dns.packet_max > 0) storeAppendPrintf(sentry, "DNS jumbo-grams: %Zd Bytes\n", Config.dns.packet_max); else storeAppendPrintf(sentry, "DNS jumbo-grams: not working\n"); storeAppendPrintf(sentry, "\nNameservers:\n"); storeAppendPrintf(sentry, "IP ADDRESS # QUERIES # REPLIES\n"); storeAppendPrintf(sentry, "---------------------------------------------- --------- ---------\n"); for (i = 0; i < nns; i++) { storeAppendPrintf(sentry, "%-45s %9d %9d\n", /* Let's take the maximum: (15 IPv4/45 IPv6) */ nameservers[i].S.NtoA(buf,MAX_IPSTRLEN), nameservers[i].nqueries, nameservers[i].nreplies); } @@ -870,40 +875,44 @@ static void idnsSendQuery(idns_query * q) { if (DnsSocketA < 0 && DnsSocketB < 0) { debugs(78, 1, "WARNING: idnsSendQuery: Can't send query, no DNS socket!"); return; } if (nns <= 0) { debugs(78, 1, "WARNING: idnsSendQuery: Can't send query, no DNS nameservers known!"); return; } assert(q->lru.next == NULL); assert(q->lru.prev == NULL); int x = -1, y = -1; int ns; + q->start_t = current_time; + q->msg_id = idnsQueryID(); + rfc1035SetQueryID(q->buf, q->msg_id); + do { ns = q->nsends % nns; if (q->need_vc) { idnsSendQueryVC(q, ns); x = y = 0; } else { if (DnsSocketB >= 0 && nameservers[ns].S.IsIPv6()) y = comm_udp_sendto(DnsSocketB, nameservers[ns].S, q->buf, q->sz); else if (DnsSocketA) x = comm_udp_sendto(DnsSocketA, nameservers[ns].S, q->buf, q->sz); } q->nsends++; q->queue_t = q->sent_t = current_time; if (y < 0 && nameservers[ns].S.IsIPv6()) debugs(50, 1, "idnsSendQuery: FD " << DnsSocketB << ": sendto: " << xstrerror()); if (x < 0 && nameservers[ns].S.IsIPv4()) @@ -934,41 +943,41 @@ continue; if (nameservers[i].S.GetPort() != from.GetPort()) continue; return i; } return -1; } static idns_query * idnsFindQuery(unsigned short id) { dlink_node *n; idns_query *q; for (n = lru_list.tail; n; n = n->prev) { q = (idns_query*)n->data; - if (q->id == id) + if (q->msg_id == id) return q; } return NULL; } static unsigned short idnsQueryID(void) { unsigned short id = squid_random() & 0xFFFF; unsigned short first_id = id; while (idnsFindQuery(id)) { id++; if (id == first_id) { debugs(78, 1, "idnsQueryID: Warning, too many pending DNS requests"); break; } } @@ -1013,41 +1022,41 @@ if (q->hash.key) { hash_remove_link(idns_lookup_hash, &q->hash); q->hash.key = NULL; } } static void idnsGrokReply(const char *buf, size_t sz, int from_ns) { int n; rfc1035_message *message = NULL; idns_query *q; n = rfc1035MessageUnpack(buf, sz, &message); if (message == NULL) { debugs(78, 1, "idnsGrokReply: Malformed DNS response"); return; } - debugs(78, 3, "idnsGrokReply: ID 0x" << std::hex << message->id << ", " << std::dec << n << " answers"); + debugs(78, 3, "idnsGrokReply: QID 0x" << std::hex << message->id << ", " << std::dec << n << " answers"); q = idnsFindQuery(message->id); if (q == NULL) { debugs(78, 3, "idnsGrokReply: Late response"); rfc1035MessageDestroy(&message); return; } if (rfc1035QueryCompare(&q->query, message->query) != 0) { debugs(78, 3, "idnsGrokReply: Query mismatch (" << q->query.name << " != " << message->query->name << ")"); rfc1035MessageDestroy(&message); return; } #if WHEN_EDNS_RESPONSES_ARE_PARSED // TODO: actually gr the message right here. // pull out the DNS meta data we need (A records, AAAA records and EDNS OPT) and store in q // this is overall better than force-feeding A response with AAAA an section later anyway. // AND allows us to merge AN+AR sections from both responses (one day) @@ -1062,133 +1071,130 @@ max_shared_edns = min(max_shared_edns, nameservers[i].last_seen_edns); } else { nameservers[from_ns].last_seen_edns = q->edns_seen; // maybe reduce the global limit downwards to accomodate this NS max_shared_edns = min(max_shared_edns, q->edns_seen); } if (max_shared_edns < RFC1035_DEFAULT_PACKET_SZ) max_shared_edns = -1; } #endif if (message->tc) { debugs(78, 3, HERE << "Resolver requested TC (" << q->query.name << ")"); dlinkDelete(&q->lru, &lru_list); rfc1035MessageDestroy(&message); if (!q->need_vc) { q->need_vc = 1; q->nsends--; idnsSendQuery(q); - } + } else { + // Strange: A TCP DNS response with the truncation bit (TC) set. + // Return an error and cleanup; no point in trying TCP again. + debugs(78, 3, HERE << "TCP DNS response"); + idnsCallback(q, NULL, 0, "Truncated TCP DNS response"); + cbdataFree(q); + } return; } dlinkDelete(&q->lru, &lru_list); idnsRcodeCount(n, q->attempt); if (n < 0) { q->rcode = -n; debugs(78, 3, "idnsGrokReply: error " << rfc1035ErrorMessage(n) << " (" << q->rcode << ")"); if (q->rcode == 2 && ++q->attempt < MAX_ATTEMPT) { /* * RCODE 2 is "Server failure - The name server was * unable to process this query due to a problem with * the name server." */ debugs(78, 3, "idnsGrokReply: Query result: SERV_FAIL"); rfc1035MessageDestroy(&message); - q->start_t = current_time; - q->id = idnsQueryID(); - rfc1035SetQueryID(q->buf, q->id); idnsSendQuery(q); return; } if (q->rcode == 3 && q->do_searchpath && q->attempt < MAX_ATTEMPT) { assert(NULL == message->answer); strcpy(q->name, q->orig); debugs(78, 3, "idnsGrokReply: Query result: NXDOMAIN - " << q->name ); if (q->domain < npc) { strcat(q->name, "."); strcat(q->name, searchpath[q->domain].domain); debugs(78, 3, "idnsGrokReply: searchpath used for " << q->name); q->domain++; } else { q->attempt++; } idnsDropMessage(message, q); - q->start_t = current_time; - q->id = idnsQueryID(); - rfc1035SetQueryID(q->buf, q->id); if (Ip::EnableIpv6 && q->query.qtype == RFC1035_TYPE_AAAA) { debugs(78, 3, "idnsGrokReply: Trying AAAA Query for " << q->name); - q->sz = rfc3596BuildAAAAQuery(q->name, q->buf, sizeof(q->buf), q->id, &q->query, Config.dns.packet_max); + q->sz = rfc3596BuildAAAAQuery(q->name, q->buf, sizeof(q->buf), 0, &q->query, Config.dns.packet_max); } else { debugs(78, 3, "idnsGrokReply: Trying A Query for " << q->name); // see EDNS notes at top of file why this sends 0 - q->sz = rfc3596BuildAQuery(q->name, q->buf, sizeof(q->buf), q->id, &q->query, 0); + q->sz = rfc3596BuildAQuery(q->name, q->buf, sizeof(q->buf), 0, &q->query, 0); } idnsCacheQuery(q); idnsSendQuery(q); return; } } if (q->need_A && (Config.onoff.dns_require_A == 1 || n <= 0 ) ) { /* ERROR or NO AAAA exist. Failover to A records. */ /* Apparently its also a good idea to lookup and store the A records * just in case the AAAA are not available when we need them. * This could occur due to number of network failings beyond our control * thus the || above allowing the user to request always both. */ if (n == 0) debugs(78, 3, "idnsGrokReply: " << q->name << " has no AAAA records. Looking up A record instead."); else if (q->need_A && n <= 0) debugs(78, 3, "idnsGrokReply: " << q->name << " AAAA query failed. Trying A now instead."); else // admin requested this. debugs(78, 3, "idnsGrokReply: " << q->name << " AAAA query done. Configured to retrieve A now also."); // move the initial message results into the failover query for merging later. if (n > 0) { q->initial_AAAA.count = message->ancount; q->initial_AAAA.answers = message->answer; message->answer = NULL; } // remove the hashed query info idnsDropMessage(message, q); // reset the query as an A query q->nsends = 0; - q->start_t = current_time; - q->id = idnsQueryID(); - rfc1035SetQueryID(q->buf, q->id); // see EDNS notes at top of file why this sends 0 - q->sz = rfc3596BuildAQuery(q->name, q->buf, sizeof(q->buf), q->id, &q->query, 0); + q->sz = rfc3596BuildAQuery(q->name, q->buf, sizeof(q->buf), 0, &q->query, 0); q->need_A = false; idnsCacheQuery(q); idnsSendQuery(q); return; } /** If there are two result sets from preceeding AAAA and A lookups merge them with a preference for AAAA */ if (q->initial_AAAA.count > 0 && n > 0) { /* two sets of RR need merging */ rfc1035_rr *result = (rfc1035_rr*) xmalloc( sizeof(rfc1035_rr)*(n + q->initial_AAAA.count) ); rfc1035_rr *tmp = result; debugs(78, 6, HERE << "Merging DNS results " << q->name << " AAAA has " << q->initial_AAAA.count << " RR, A has " << n << " RR"); memcpy(tmp, q->initial_AAAA.answers, (sizeof(rfc1035_rr)*(q->initial_AAAA.count)) ); tmp += q->initial_AAAA.count; /* free the RR object without freeing its child strings (they are now taken by the copy above) */ safe_free(q->initial_AAAA.answers); memcpy( tmp, message->answer, (sizeof(rfc1035_rr)*n) ); @@ -1311,49 +1317,52 @@ /* name servers went away; reconfiguring or shutting down */ return; for (n = lru_list.tail; n; n = p) { p = n->prev; q = static_cast(n->data); /* Anything to process in the queue? */ if (tvSubDsec(q->queue_t, current_time) < Config.Timeout.idns_retransmit ) break; /* Query timer expired? */ if (tvSubDsec(q->sent_t, current_time) < Config.Timeout.idns_retransmit * 1 << ((q->nsends - 1) / nns)) { dlinkDelete(&q->lru, &lru_list); q->queue_t = current_time; dlinkAdd(q, &q->lru, &lru_list); continue; } - debugs(78, 3, "idnsCheckQueue: ID 0x" << std::hex << std::setfill('0') << std::setw(4) << q->id << "timeout" ); + debugs(78, 3, "idnsCheckQueue: ID " << q->xact_id << + " QID 0x" << std::hex << std::setfill('0') << + std::setw(4) << q->msg_id << ": timeout" ); dlinkDelete(&q->lru, &lru_list); if (tvSubDsec(q->start_t, current_time) < Config.Timeout.idns_query) { idnsSendQuery(q); } else { - debugs(78, 2, "idnsCheckQueue: ID " << std::hex << q->id << - ": giving up after " << std::dec << q->nsends << " tries and " << + debugs(78, 2, "idnsCheckQueue: ID " << q->xact_id << + " QID 0x" << std::hex << q->msg_id << + " : giving up after " << std::dec << q->nsends << " tries and " << std::setw(5)<< std::setprecision(2) << tvSubDsec(q->start_t, current_time) << " seconds"); if (q->rcode != 0) idnsCallback(q, NULL, -q->rcode, rfc1035ErrorMessage(q->rcode)); else idnsCallback(q, NULL, -16, "Timeout"); cbdataFree(q); } } idnsTickleQueue(); } static void idnsReadVC(int fd, char *buf, size_t len, comm_err_t flag, int xerrno, void *data) { nsvc * vc = (nsvc *)data; if (flag == COMM_ERR_CLOSING) @@ -1561,171 +1570,170 @@ comm_close(vc->fd); } } // XXX: vcs are not closed/freed yet and may try to access nameservers[] idnsFreeNameservers(); idnsFreeSearchpath(); } static int idnsCachedLookup(const char *key, IDNSCB * callback, void *data) { idns_query *q; idns_query *old = (idns_query *) hash_lookup(idns_lookup_hash, key); if (!old) return 0; q = cbdataAlloc(idns_query); + // idns_query is POD so no constructors are called after allocation + q->xact_id.change(); q->callback = callback; q->callback_data = cbdataReference(data); q->queue = old->queue; old->queue = q; return 1; } static void idnsCacheQuery(idns_query *q) { q->hash.key = q->query.name; hash_join(idns_lookup_hash, &q->hash); } void idnsALookup(const char *name, IDNSCB * callback, void *data) { unsigned int i; int nd = 0; idns_query *q; if (idnsCachedLookup(name, callback, data)) return; q = cbdataAlloc(idns_query); - - q->id = idnsQueryID(); + // idns_query is POD so no constructors are called after allocation + q->xact_id.change(); for (i = 0; i < strlen(name); i++) if (name[i] == '.') nd++; if (Config.onoff.res_defnames && npc > 0 && name[strlen(name)-1] != '.') { q->do_searchpath = 1; } else { q->do_searchpath = 0; } strcpy(q->orig, name); strcpy(q->name, q->orig); if (q->do_searchpath && nd < ndots) { q->domain = 0; strcat(q->name, "."); strcat(q->name, searchpath[q->domain].domain); debugs(78, 3, "idnsALookup: searchpath used for " << q->name); } if (Ip::EnableIpv6) { - q->sz = rfc3596BuildAAAAQuery(q->name, q->buf, sizeof(q->buf), q->id, &q->query, Config.dns.packet_max); + q->sz = rfc3596BuildAAAAQuery(q->name, q->buf, sizeof(q->buf), 0, &q->query, Config.dns.packet_max); q->need_A = true; } else { // see EDNS notes at top of file why this sends 0 - q->sz = rfc3596BuildAQuery(q->name, q->buf, sizeof(q->buf), q->id, &q->query, 0); + q->sz = rfc3596BuildAQuery(q->name, q->buf, sizeof(q->buf), 0, &q->query, 0); q->need_A = false; } if (q->sz < 0) { /* problem with query data -- query not sent */ callback(data, NULL, 0, "Internal error"); cbdataFree(q); return; } debugs(78, 3, "idnsALookup: buf is " << q->sz << " bytes for " << q->name << - ", id = 0x" << std::hex << q->id); + ", id = 0x" << std::hex << q->msg_id); q->callback = callback; q->callback_data = cbdataReference(data); - q->start_t = current_time; - idnsCacheQuery(q); idnsSendQuery(q); } void idnsPTRLookup(const Ip::Address &addr, IDNSCB * callback, void *data) { idns_query *q; char ip[MAX_IPSTRLEN]; addr.NtoA(ip,MAX_IPSTRLEN); q = cbdataAlloc(idns_query); - q->id = idnsQueryID(); + // idns_query is POD so no constructors are called after allocation + q->xact_id.change(); if (Ip::EnableIpv6 && addr.IsIPv6()) { struct in6_addr addr6; addr.GetInAddr(addr6); - q->sz = rfc3596BuildPTRQuery6(addr6, q->buf, sizeof(q->buf), q->id, &q->query, Config.dns.packet_max); + q->sz = rfc3596BuildPTRQuery6(addr6, q->buf, sizeof(q->buf), 0, &q->query, Config.dns.packet_max); } else { struct in_addr addr4; addr.GetInAddr(addr4); // see EDNS notes at top of file why this sends 0 - q->sz = rfc3596BuildPTRQuery4(addr4, q->buf, sizeof(q->buf), q->id, &q->query, 0); + q->sz = rfc3596BuildPTRQuery4(addr4, q->buf, sizeof(q->buf), 0, &q->query, 0); } /* PTR does not do inbound A/AAAA */ q->need_A = false; if (q->sz < 0) { /* problem with query data -- query not sent */ callback(data, NULL, 0, "Internal error"); cbdataFree(q); return; } if (idnsCachedLookup(q->query.name, callback, data)) { cbdataFree(q); return; } debugs(78, 3, "idnsPTRLookup: buf is " << q->sz << " bytes for " << ip << - ", id = 0x" << std::hex << q->id); + ", id = 0x" << std::hex << q->msg_id); q->callback = callback; q->callback_data = cbdataReference(data); - q->start_t = current_time; - idnsCacheQuery(q); idnsSendQuery(q); } #if SQUID_SNMP /* * The function to return the DNS via SNMP */ variable_list * snmp_netIdnsFn(variable_list * Var, snint * ErrP) { int i, n = 0; variable_list *Answer = NULL; MemBuf tmp; debugs(49, 5, "snmp_netDnsFn: Processing request: " << snmpDebugOid(Var->name, Var->name_length, tmp)); *ErrP = SNMP_ERR_NOERROR; switch (Var->name[LEN_SQ_NET + 1]) {