Changeset View
Changeset View
Standalone View
Standalone View
sys/netgraph/ng_nat.c
| Show First 20 Lines • Show All 812 Lines • ▼ Show 20 Lines | ng_nat_rcvdata(hook_p hook, item_p item ) | |||||||||
| if (m->m_pkthdr.len < ipofs + sizeof(struct ip)) | if (m->m_pkthdr.len < ipofs + sizeof(struct ip)) | |||||||||
| goto send; /* packet too short to hold IP */ | goto send; /* packet too short to hold IP */ | |||||||||
| c = (char *)mtodo(m, ipofs); | c = (char *)mtodo(m, ipofs); | |||||||||
| ip = (struct ip *)mtodo(m, ipofs); | ip = (struct ip *)mtodo(m, ipofs); | |||||||||
| if (ip->ip_v != IPVERSION) | if (ip->ip_v != IPVERSION) | |||||||||
| goto send; /* other IP version, let it pass */ | goto send; /* other IP version, let it pass */ | |||||||||
| if (m->m_pkthdr.len < ipofs + ntohs(ip->ip_len)) | uint16_t ip_len = ntohs(ip->ip_len); | |||||||||
| if (m->m_pkthdr.len < (ipofs + ip_len)) | ||||||||||
markj: Since `ip_len` isn't used elsewhere, just check `m->m_pkthdr.len < ipofs + ntohs(ip->ip_len)`? | ||||||||||
Done Inline ActionsWell, arguably it's used below. I just wanted to draw attention of a human reader/writer that it's the same value we reading pretty much, but code assumes that it can be different before and after. Compilers nowadays don't care what and when you declare anyways. sobomax: Well, arguably it's used below. I just wanted to draw attention of a human reader/writer that… | ||||||||||
| goto send; /* packet too short (i.e. fragmented or broken) */ | goto send; /* packet too short (i.e. fragmented or broken) */ | |||||||||
| /* | /* | |||||||||
| * We drop packet when: | * We drop packet when: | |||||||||
| * 1. libalias returns PKT_ALIAS_ERROR; | * 1. libalias returns PKT_ALIAS_ERROR; | |||||||||
| * 2. For incoming packets: | * 2. For incoming packets: | |||||||||
| * a) for unresolved fragments; | * a) for unresolved fragments; | |||||||||
| * b) libalias returns PKT_ALIAS_IGNORED and | * b) libalias returns PKT_ALIAS_IGNORED and | |||||||||
| Show All 17 Lines | if (rval == PKT_ALIAS_ERROR) { | |||||||||
| NG_FREE_ITEM(item); | NG_FREE_ITEM(item); | |||||||||
| return (EINVAL); | return (EINVAL); | |||||||||
| } | } | |||||||||
| } else | } else | |||||||||
| panic("ng_nat: unknown hook!\n"); | panic("ng_nat: unknown hook!\n"); | |||||||||
| if (rval == PKT_ALIAS_RESPOND) | if (rval == PKT_ALIAS_RESPOND) | |||||||||
| m->m_flags |= M_SKIP_FIREWALL; | m->m_flags |= M_SKIP_FIREWALL; | |||||||||
| m->m_pkthdr.len = m->m_len = ntohs(ip->ip_len) + ipofs; | ||||||||||
| if ((ip->ip_off & htons(IP_OFFMASK)) == 0 && | /* Re-read just in case it has been updated */ | |||||||||
| ip->ip_p == IPPROTO_TCP) { | ip_len = ntohs(ip->ip_len); | |||||||||
| struct tcphdr *th = (struct tcphdr *)((caddr_t)ip + | int new_m_len = ip_len + ipofs; | |||||||||
Not Done Inline Actions
markj: | ||||||||||
Done Inline ActionsWhen does libalias change the packet length? I see that ipfw updates the mbuf length as well, but there's no explanation for why. markj: When does libalias change the packet length? I see that ipfw updates the mbuf length as well… | ||||||||||
Done Inline ActionsI suppose it's for generality, at least. I suspect as part of the NATing of complex application-level protocols that might embed IP address in its string representation (say FTP), there might be cases when processed frame needs to be slightly larger/smaller in terms of number of bytes, so that the IP frame length might need to be adjusted to match. I was spelunking various NAT protocol handlers while investigating this issue and I don't think I spotted any clear cases of that happening. But that's the only logical conclusion I found as to why do we read mbuf length back from the frame. sobomax: I suppose it's for generality, at least. I suspect as part of the NATing of complex application… | ||||||||||
| (ip->ip_hl << 2)); | ||||||||||
| if (new_m_len > (m->m_len + M_TRAILINGSPACE(m))) { | ||||||||||
| /* | /* | |||||||||
| * This is just a safety railguard to make sure LibAlias has not | ||||||||||
| * screwed the IP packet up somehow, should probably be KASSERT() | ||||||||||
| * at some point. Calling in_delayed_cksum() will parse IP packet | ||||||||||
| * again and reliably panic if there is less data than the IP | ||||||||||
| * header declares, there might be some other places too. | ||||||||||
| */ | ||||||||||
| printf("ng_nat_rcvdata: outgoing packet corrupted, " | ||||||||||
| "not enough data: expected %d, available (%d - %d)\n", | ||||||||||
| ip_len, m->m_len, ipofs); | ||||||||||
| NG_FREE_ITEM(item); | ||||||||||
| return (ENXIO); | ||||||||||
| } | ||||||||||
| m->m_pkthdr.len = m->m_len = new_m_len; | ||||||||||
| if ((ip->ip_off & htons(IP_OFFMASK)) != 0 || ip->ip_p != IPPROTO_TCP) | ||||||||||
| goto send; | ||||||||||
| uint16_t pl_offset = ip->ip_hl << 2; | ||||||||||
| struct tcphdr *th = (struct tcphdr *)((caddr_t)ip + pl_offset); | ||||||||||
Not Done Inline ActionsVariables should be declared at the beginning of a scope. markj: Variables should be declared at the beginning of a scope. | ||||||||||
| /* | ||||||||||
| * Here is our terrible HACK. | * Here is our terrible HACK. | |||||||||
| * | * | |||||||||
| * Sometimes LibAlias edits contents of TCP packet. | * Sometimes LibAlias edits contents of TCP packet. | |||||||||
| * In this case it needs to recompute full TCP | * In this case it needs to recompute full TCP | |||||||||
| * checksum. However, the problem is that LibAlias | * checksum. However, the problem is that LibAlias | |||||||||
| * doesn't have any idea about checksum offloading | * doesn't have any idea about checksum offloading | |||||||||
| * in kernel. To workaround this, we do not do | * in kernel. To workaround this, we do not do | |||||||||
| * checksumming in LibAlias, but only mark the | * checksumming in LibAlias, but only mark the | |||||||||
| * packets with TH_RES1 in the th_x2 field. If we | * packets in th_x2 field. If we receive a marked | |||||||||
| * receive a marked packet, we calculate correct | * packet, we calculate correct checksum for it | |||||||||
| * checksum for it aware of offloading. | * aware of offloading. | |||||||||
| * | * | |||||||||
| * Why do I do such a terrible hack instead of | * Why do I do such a terrible hack instead of | |||||||||
| * recalculating checksum for each packet? | * recalculating checksum for each packet? | |||||||||
| * Because the previous checksum was not checked! | * Because the previous checksum was not checked! | |||||||||
| * Recalculating checksums for EVERY packet will | * Recalculating checksums for EVERY packet will | |||||||||
| * hide ALL transmission errors. Yes, marked packets | * hide ALL transmission errors. Yes, marked packets | |||||||||
| * still suffer from this problem. But, sigh, natd(8) | * still suffer from this problem. But, sigh, natd(8) | |||||||||
| * has this problem, too. | * has this problem, too. | |||||||||
| */ | */ | |||||||||
| if (tcp_get_flags(th) & TH_RES1) { | if (!th->th_x2) | |||||||||
| uint16_t ip_len = ntohs(ip->ip_len); | goto send; | |||||||||
| tcp_set_flags(th, tcp_get_flags(th) & ~TH_RES1); | th->th_x2 = 0; | |||||||||
| th->th_sum = in_pseudo(ip->ip_src.s_addr, | th->th_sum = in_pseudo(ip->ip_src.s_addr, ip->ip_dst.s_addr, | |||||||||
| ip->ip_dst.s_addr, htons(IPPROTO_TCP + | htons(IPPROTO_TCP + ip_len - pl_offset)); | |||||||||
| ip_len - (ip->ip_hl << 2))); | ||||||||||
| if ((m->m_pkthdr.csum_flags & CSUM_TCP) == 0) { | if ((m->m_pkthdr.csum_flags & CSUM_TCP) != 0) | |||||||||
| m->m_pkthdr.csum_data = offsetof(struct tcphdr, | goto send; | |||||||||
| th_sum); | ||||||||||
| in_delayed_cksum(m); | m->m_pkthdr.csum_data = offsetof(struct tcphdr, th_sum); | |||||||||
| } | in_delayed_cksum_o(m, ipofs); | |||||||||
| } | ||||||||||
| } | ||||||||||
| send: | send: | |||||||||
| if (hook == priv->in) | if (hook == priv->in) | |||||||||
| NG_FWD_ITEM_HOOK(error, item, priv->out); | NG_FWD_ITEM_HOOK(error, item, priv->out); | |||||||||
| else | else | |||||||||
| NG_FWD_ITEM_HOOK(error, item, priv->in); | NG_FWD_ITEM_HOOK(error, item, priv->in); | |||||||||
| return (error); | return (error); | |||||||||
| ▲ Show 20 Lines • Show All 68 Lines • Show Last 20 Lines | ||||||||||
Since ip_len isn't used elsewhere, just check m->m_pkthdr.len < ipofs + ntohs(ip->ip_len)?