Re: UPDATE: less-458
On Thu, Apr 17, 2014 at 6:37 AM, Alexandr Shadchin alexandr.shadc...@gmail.com wrote: This diff updates less to the latest release 458. Tested on amd64 and i386. Comments ? OK ? Seems good to me. ok guenther@
Re: support for Realtek RTS5227 Card Reader
On Thu, Apr 17, 2014 at 11:58:03PM +0200, Mark Kettenis wrote: Date: Thu, 17 Apr 2014 23:42:26 +0200 From: Claudio Jeker clau...@openbsd.org Found this in my X240, the following diff makes it work. rtsx0 at pci1 dev 0 function 0 Realtek RTS5227 Card Reader rev 0x01: msi sdmmc0 at rtsx0 scsibus4 at sdmmc0: 2 targets, initiator 0 sd1 at scsibus4 targ 1 lun 0: SD/MMC, Drive #01, SCSI2 0/direct fixed sd1: 15296MB, 512 bytes/sector, 31326208 sectors A bit surprised since it seems Linux has two different drivers for RTS52x9 and RTS5227. You might want to sort the IDs such that they are in numerical order. Anyway, if this works, ok kettenis@, there is no downside. The linux driver (mfd/rts5227.c, compared to mfd/rts5229.c) seems to enable some optional PCI express features (LTR, OBFF), and also tweaks some settings for cases that might not apply because we currently run SD cards in the lowest clock mode. So, yes, if this makes read/write for SD cards work for you, please commit. Could you also adjust the man page? We can still try some of the tweaks the Linux driver applies later. E.g. the rts5227_force_power_down() code might be interesting, or the check for a reverse socket in rts5229_fetch_vendor_settings(). Index: dev/pci/rtsx_pci.c === RCS file: /cvs/src/sys/dev/pci/rtsx_pci.c,v retrieving revision 1.4 diff -u -p -r1.4 rtsx_pci.c --- dev/pci/rtsx_pci.c 6 Nov 2013 13:51:02 - 1.4 +++ dev/pci/rtsx_pci.c 4 Apr 2014 20:13:41 - @@ -58,7 +58,8 @@ rtsx_pci_match(struct device *parent, vo return 0; if (PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_REALTEK_RTS5209 || - PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_REALTEK_RTS5229) + PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_REALTEK_RTS5229 || + PCI_PRODUCT(pa-pa_id) == PCI_PRODUCT_REALTEK_RTS5227) return 1; return 0;
Re: sudo -u environment help
FYI tech@, there was a thread on misc@ about sudo -iu not setting some environment variables: http://thread.gmane.org/gmane.os.openbsd.misc/211823/ On 2014-04-08 Tue 09:26 AM |, Craig R. Skinner wrote: To clarify, there are no ~/. shell dot files. $PATH umask are set in /etc/login.conf $MAIL is the default set by login(1) /etc/profile sources /etc/ksh.kshrc, which just sets $PS1, window decor some aliases, nothing major. This arrangement works fine when logging in directly, or via sudo su -l user From my reading of sudo(8), I thought the same environment could be gained with something like sudo -H -i -u username. Am I missing sudo flags or settings in /etc/sudoers? On 2014-04-04 Fri 11:30 AM |, Craig R. Skinner wrote: Hi, When sudo'ing to another user, how can I obtain all of their environment settings as they receive when logging in themselves? When I use sudo in this manner, settings such as $PATH, $MAIL umask aren't being honoured: $ echo $LOGNAME; echo $PATH; echo $MAIL; umask craig /usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/local/sbin:/usr/site/bin:/usr/site/sbin:/home/craig/bin /var/mail/craig 027 Here, $PATH, $MAIL umask are unchanged: $ sudo -H -i -u david $ echo $LOGNAME; echo $PATH; echo $MAIL; umask david /usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/local/sbin:/usr/site/bin:/usr/site/sbin:/home/craig/bin /var/mail/craig 027 Compare the difference when logging in as that user: $ login david ... $ echo $LOGNAME; echo $PATH; echo $MAIL; umask david /usr/bin:/bin:/usr/local/bin:/usr/site/bin:/home/david/bin /var/mail/david 022 /etc/login.conf: default:\ :passwordcheck=/usr/local/bin/pwqcheck -1:\ :passwordtries=0:\ :path=/usr/bin /bin /usr/local/bin /usr/site/bin ~/bin:\ :umask=022:\ :datasize-cur= staff:\ :path=/usr/bin /bin /usr/sbin /sbin /usr/local/bin /usr/local/sbin /usr/site/bin /usr/site/sbin ~/bin:\ :umask=027:\ :datasize-cur= $ egrep 'env_|Defaults' /etc/sudoers | grep -v ^# Defaults env_keep +=DESTDIR DISTDIR EDITOR FETCH_CMD FLAVOR FTPMODE GROUP MAKE Defaults env_keep +=MAKECONF MULTI_PACKAGES NOMAN OKAY_FILES OWNER PKG_CACHE Defaults env_keep +=PKG_DBDIR PKG_DESTDIR PKG_PATH PKG_TMPDIR PORTSDIR Defaults env_keep +=RELEASEDIR SHARED_ONLY SSH_AUTH_SOCK SUBPACKAGE VISUAL Defaults env_keep +=WRKOBJDIR Defaults always_set_home, ignore_dot, use_loginclass login(1): login enters information into the environment (see environ(7)) specifying the user's home directory (HOME), command interpreter (SHELL), search path (PATH), terminal type (TERM), and user name (both LOGNAME and USER). ENVIRONMENT login sets the following environment variables: HOME MAIL sudo(8): Command Environment .. On BSD systems, if the use_loginclass option is enabled, the environment is initialized based on the path and setenv settings in /etc/login.conf. The new environment contains the TERM, PATH, HOME, MAIL, SHELL, LOGNAME, USER, USERNAME and SUDO_* variables in addition to variables from the invoking process permitted by the env_check and env_keep options. This is effectively a whitelist for environment variables. How can I become another user - without knowing their password, and gain their 'natural' environment? e.g. from wheel group to a users group member. 'su -l username' 'login username' require their password. I thought 'sudo -H -i -u username' would do it. Any suggestions on what else I need to configure?
check if chmod was successful
Hi guys,
this little diff checks if the chmod call was successful or not.
Regards,
Fritjof
Index: rand/randfile.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/rand/randfile.c,v
retrieving revision 1.33
diff -u -p -r1.33 randfile.c
--- rand/randfile.c 18 Apr 2014 11:31:16 - 1.33
+++ rand/randfile.c 18 Apr 2014 14:00:05 -
@@ -120,7 +120,9 @@ int RAND_write_file(const char *file)
out = fopen(file,wb);
if (out == NULL) goto err;
- chmod(file,0600);
+ if (chmod(file, 0600) == -1)
+ goto err;
+
n=RAND_DATA;
for (;;)
{
Re: check if chmod was successful
On Fri, Apr 18, 2014 at 04:00:28PM +0200, Fritjof Bornebusch wrote: Hi guys, this little diff checks if the chmod call was successful or not. Regards, Fritjof Index: rand/randfile.c === RCS file: /cvs/src/lib/libssl/src/crypto/rand/randfile.c,v retrieving revision 1.33 diff -u -p -r1.33 randfile.c --- rand/randfile.c 18 Apr 2014 11:31:16 - 1.33 +++ rand/randfile.c 18 Apr 2014 14:00:05 - @@ -120,7 +120,9 @@ int RAND_write_file(const char *file) out = fopen(file,wb); if (out == NULL) goto err; - chmod(file,0600); + if (chmod(file, 0600) == -1) + goto err; + You would leak out. Reyk
openssl's *strlcy
Small demonstration of the kinds of things we'll have to mop up for weeks more. From OpenSSL CHANGES file: *) Introduce safe string copy and catenation functions (BUF_strlcpy() and BUF_strlcat()). [Ben Laurie (CHATS) and Richard Levitte] That's from back in 2002. These functions work just like ours in OpenBSD. The return values indicate overflow. We've been advising people to check for overflow from the start, which is why we designed these like snprintf. Good move, but then let's see how they used them: ./apps/ca.c:BUF_strlcpy(tofree, s, len); ./apps/ca.c:BUF_strlcat(tofree, /, len); ./apps/ca.c:BUF_strlcat(tofree, CONFIG_FILE, len); ./apps/ca.c:BUF_strlcat(buf[2], /, sizeof(buf[2])); ./apps/ca.c:BUF_strlcpy(row[DB_file], unknown, 8); ./apps/ca.c:BUF_strlcpy(row[DB_file], unknown, 8); ./apps/ca.c:BUF_strlcpy(str, (char *) revtm-data, i); ./apps/ca.c:BUF_strlcat(str, ,, i); ./apps/ca.c:BUF_strlcat(str, reason, i); ./apps/ca.c:BUF_strlcat(str, ,, i); ./apps/ca.c:BUF_strlcat(str, other, i); ./apps/apps.c: BUF_strlcpy(out, p, size); ./apps/apps.c: BUF_strlcpy(buf[0], serialfile, BSIZE); ./apps/engine.c:BUF_strlcat(*buf, , , *size); ./apps/engine.c:BUF_strlcat(*buf, s, *size); ./apps/pkcs12.c:BUF_strlcpy(macpass, pass, sizeof macpass); ./apps/pkcs12.c:BUF_strlcpy(macpass, pass, sizeof macpass); ./apps/req.c: BUF_strlcpy(buf, value, sizeof buf); ./apps/req.c: BUF_strlcat(buf, \n, sizeof buf); ./apps/req.c: BUF_strlcpy(buf, def, sizeof buf); ./apps/req.c: BUF_strlcat(buf, \n, sizeof buf); ./apps/req.c: BUF_strlcpy(buf, value, sizeof buf); ./apps/req.c: BUF_strlcat(buf, \n, sizeof buf); ./apps/req.c: BUF_strlcpy(buf, def, sizeof buf); ./apps/req.c: BUF_strlcat(buf, \n, sizeof buf); ./apps/s_socket.c: BUF_strlcpy(*host, h1-h_name, strlen(h1-h_name) + 1); ./apps/srp.c: BUF_strlcpy(tofree, s, len); ./apps/srp.c: BUF_strlcat(tofree, /, len); ./apps/srp.c: BUF_strlcat(tofree, CONFIG_FILE, len); ./apps/x509.c: BUF_strlcpy(buf, CAfile, len); ./apps/x509.c: BUF_strlcat(buf, POSTFIX, len); ./apps/x509.c: BUF_strlcpy(buf, serialfile, len); ./crypto/asn1/a_time.c: if (t-data[0] = '5') BUF_strlcpy(str, 19, newlen); ./crypto/asn1/a_time.c: else BUF_strlcpy(str, 20, newlen); ./crypto/asn1/a_time.c: BUF_strlcat(str, (char *)t-data, newlen); ./crypto/bio/b_dump.c: BUF_strlcpy(buf, str, sizeof buf); ./crypto/bio/b_dump.c: BUF_strlcat(buf, tmp, sizeof buf); ./crypto/bio/b_dump.c: BUF_strlcat(buf,, sizeof buf); ./crypto/bio/b_dump.c: BUF_strlcat(buf, tmp, sizeof buf); ./crypto/bio/b_dump.c: BUF_strlcat(buf, , sizeof buf); ./crypto/bio/b_dump.c: BUF_strlcat(buf, tmp, sizeof buf); ./crypto/bio/b_dump.c: BUF_strlcat(buf, \n, sizeof buf); ./crypto/bio/bss_file.c:BUF_strlcpy(p, a+, sizeof p); ./crypto/bio/bss_file.c:elseBUF_strlcpy(p, a, sizeof p); ./crypto/bio/bss_file.c:BUF_strlcpy(p, r+, sizeof p); ./crypto/bio/bss_file.c:BUF_strlcpy(p, w, sizeof p); ./crypto/bio/bss_file.c:BUF_strlcpy(p, r, sizeof p); ./crypto/buffer/buf_str.c:BUF_strlcpy(char *dst, const char *src, size_t size) ./crypto/buffer/buf_str.c:BUF_strlcat(char *dst, const char *src, size_t size) ./crypto/buffer/buffer.h:size_t BUF_strlcpy(char *dst, const char *src, size_t siz); ./crypto/buffer/buffer.h:size_t BUF_strlcat(char *dst, const char *src, size_t siz); ./crypto/conf/conf_def.c: BUF_strlcpy(section,default,10); ./crypto/conf/conf_def.c: BUF_strlcpy(v-name,pname,strlen(pname)+1); ./crypto/dso/dso_lib.c: BUF_strlcpy(copied, filename, strlen(filename) + 1); ./crypto/dso/dso_lib.c: BUF_strlcpy(result, filename, strlen(filename) + 1); ./crypto/err/err.c: BUF_strlcat(str,a,(size_t)s+1); ./crypto/evp/evp_pbe.c: if (!pbe_obj) BUF_strlcpy (obj_tmp, NULL, sizeof obj_tmp); ./crypto/objects/obj_dat.c: BUF_strlcpy(buf,s,buf_len); ./crypto/objects/obj_dat.c: BUF_strlcpy(buf,bndec,buf_len); ./crypto/objects/obj_dat.c: BUF_strlcpy(buf,tbuf,buf_len); ./crypto/pem/pem_lib.c: BUF_strlcat(buf,Proc-Type: 4,,PEM_BUFSIZE); ./crypto/pem/pem_lib.c: BUF_strlcat(buf,str,PEM_BUFSIZE); ./crypto/pem/pem_lib.c: BUF_strlcat(buf,\n,PEM_BUFSIZE); ./crypto/pem/pem_lib.c: BUF_strlcat(buf,DEK-Info: ,PEM_BUFSIZE); ./crypto/pem/pem_lib.c: BUF_strlcat(buf,type,PEM_BUFSIZE); ./crypto/pem/pem_lib.c:
fix for ifa RB tree corruption
Bad stuff happens when the ifa lookup tree gets corrupted. In my case local traffic was suddenly no longer local and was forwarded to lo0 ad infinitum. This was caused by the usage of rdomains and destroing pseudo interfaces. The sadl address was still in rdomain 0, was therefor not found in the tree and so you end up with a bad node and a use after free. The following diff solves this by removing and readding the sadl to the RB tree when switching rdomain. OK? -- :wq Claudio Index: if.c === RCS file: /cvs/src/sys/net/if.c,v retrieving revision 1.283 diff -u -p -r1.283 if.c --- if.c10 Apr 2014 13:47:21 - 1.283 +++ if.c18 Apr 2014 16:03:11 - @@ -1515,6 +1580,8 @@ ifioctl(struct socket *so, u_long cmd, c #ifdef INET in_ifdetach(ifp); #endif + /* remove sadl from ifa RB tree */ + ifa_del(ifp, ifp-if_lladdr); splx(s); } @@ -1525,6 +1592,9 @@ ifioctl(struct socket *so, u_long cmd, c /* Add interface to the specified rdomain */ ifp-if_rdomain = ifr-ifr_rdomainid; + + /* re-add sadl to the ifa RB tree in new rdomain */ + ifa_add(ifp, ifp-if_lladdr); break; case SIOCAIFGROUP:
Re: help needed from someone with an sk(4)
so, what are we doing with this now?
I still want to hide in_cksum_phdr() and kill in_cksum_addword() so that
nobody ever uses that sh*t again.
yes, sk loses is half-baked cksum offload support with this, as
discussed before.
as naddy pointed out there are (at least) two private copies of
in_cksum_phdr in other drivers, that is a seperate issue in my book if
it is an issue at all, that's not an exposed API (well, for some value
of exposed, we're talking in-kernel only).
oks?
Index: dev/pci/if_sk.c
===
RCS file: /cvs/src/sys/dev/pci/if_sk.c,v
retrieving revision 1.167
diff -u -p -r1.167 if_sk.c
--- dev/pci/if_sk.c 28 Dec 2013 03:36:25 - 1.167
+++ dev/pci/if_sk.c 24 Jan 2014 09:16:46 -
@@ -180,7 +180,6 @@ void sk_iff_yukon(struct sk_if_softc *);
void sk_tick(void *);
void sk_yukon_tick(void *);
-void sk_rxcsum(struct ifnet *, struct mbuf *, const u_int16_t, const
u_int16_t);
#ifdef SK_DEBUG
#define DPRINTF(x) if (skdebug) printf x
@@ -550,9 +549,6 @@ sk_init_rx_ring(struct sk_if_softc *sc_i
nexti = i + 1;
cd-sk_rx_chain[i].sk_next = cd-sk_rx_chain[nexti];
rd-sk_rx_ring[i].sk_next = htole32(SK_RX_RING_ADDR(sc_if,
nexti));
- rd-sk_rx_ring[i].sk_csum1_start = htole16(ETHER_HDR_LEN);
- rd-sk_rx_ring[i].sk_csum2_start = htole16(ETHER_HDR_LEN +
- sizeof(struct ip));
}
for (i = 0; i SK_RX_RING_CNT; i++) {
@@ -1732,7 +1728,6 @@ sk_rxeof(struct sk_if_softc *sc_if)
int i, cur, total_len = 0;
u_int32_t rxstat, sk_ctl;
bus_dmamap_tdmamap;
- u_int16_t csum1, csum2;
DPRINTFN(2, (sk_rxeof\n));
@@ -1765,9 +1760,6 @@ sk_rxeof(struct sk_if_softc *sc_if)
cur_rx-sk_mbuf = NULL;
total_len = SK_RXBYTES(letoh32(cur_desc-sk_ctl));
- csum1 = letoh16(sc_if-sk_rdata-sk_rx_ring[i].sk_csum1);
- csum2 = letoh16(sc_if-sk_rdata-sk_rx_ring[i].sk_csum2);
-
SK_INC(i, SK_RX_RING_CNT);
if ((sk_ctl (SK_RXCTL_STATUS_VALID | SK_RXCTL_FIRSTFRAG |
@@ -1805,8 +1797,6 @@ sk_rxeof(struct sk_if_softc *sc_if)
ifp-if_ipackets++;
- sk_rxcsum(ifp, m, csum1, csum2);
-
#if NBPFILTER 0
if (ifp-if_bpf)
bpf_mtap(ifp-if_bpf, m, BPF_DIRECTION_IN);
@@ -1818,94 +1808,6 @@ sk_rxeof(struct sk_if_softc *sc_if)
}
void
-sk_rxcsum(struct ifnet *ifp, struct mbuf *m, const u_int16_t csum1, const
u_int16_t csum2)
-{
- struct ether_header *eh;
- struct ip *ip;
- u_int8_t *pp;
- int hlen, len, plen;
- u_int16_t iph_csum, ipo_csum, ipd_csum, csum;
-
- pp = mtod(m, u_int8_t *);
- plen = m-m_pkthdr.len;
- if (plen sizeof(*eh))
- return;
- eh = (struct ether_header *)pp;
- iph_csum = in_cksum_addword(csum1, (~csum2 0x));
-
- if (eh-ether_type == htons(ETHERTYPE_VLAN)) {
- u_int16_t *xp = (u_int16_t *)pp;
-
- xp = (u_int16_t *)pp;
- if (xp[1] != htons(ETHERTYPE_IP))
- return;
- iph_csum = in_cksum_addword(iph_csum, (~xp[0] 0x));
- iph_csum = in_cksum_addword(iph_csum, (~xp[1] 0x));
- xp = (u_int16_t *)(pp + sizeof(struct ip));
- iph_csum = in_cksum_addword(iph_csum, xp[0]);
- iph_csum = in_cksum_addword(iph_csum, xp[1]);
- pp += EVL_ENCAPLEN;
- } else if (eh-ether_type != htons(ETHERTYPE_IP))
- return;
-
- pp += sizeof(*eh);
- plen -= sizeof(*eh);
-
- ip = (struct ip *)pp;
-
- if (ip-ip_v != IPVERSION)
- return;
-
- hlen = ip-ip_hl 2;
- if (hlen sizeof(struct ip))
- return;
- if (hlen ntohs(ip-ip_len))
- return;
-
- /* Don't deal with truncated or padded packets. */
- if (plen != ntohs(ip-ip_len))
- return;
-
- len = hlen - sizeof(struct ip);
- if (len 0) {
- u_int16_t *p;
-
- p = (u_int16_t *)(ip + 1);
- ipo_csum = 0;
- for (ipo_csum = 0; len 0; len -= sizeof(*p), p++)
- ipo_csum = in_cksum_addword(ipo_csum, *p);
- iph_csum = in_cksum_addword(iph_csum, ipo_csum);
- ipd_csum = in_cksum_addword(csum2, (~ipo_csum 0x));
- } else
- ipd_csum = csum2;
-
- if (iph_csum != 0x)
- return;
- m-m_pkthdr.csum_flags |= M_IPV4_CSUM_IN_OK;
-
- if (ip-ip_off htons(IP_MF | IP_OFFMASK))
- return; /* ip frag, we're done for now */
-
- pp += hlen;
-
- /* Only know checksum protocol for udp/tcp */
- if (ip-ip_p ==
Re: tighten /etc/rc's pf ruleset slightly further
this one is still open as well. oks?
* Henning Brauer lists-openbsdt...@bsws.de [2014-01-21 03:24]:
absolutely prevent forwarding carp or NFS/rpc using the shiny new
received-on any.
can only minimally test that here. need at least one carp and one
diskless test.
Index: rc
===
RCS file: /cvs/src/etc/rc,v
retrieving revision 1.420
diff -u -p -r1.420 rc
--- rc19 Jan 2014 09:39:04 - 1.420
+++ rc21 Jan 2014 01:53:59 -
@@ -335,13 +335,14 @@ if [ X${pf} != XNO ]; then
RULES=$RULES\npass out inet6 proto udp from any port
dhcpv6-client to any port dhcpv6-server
RULES=$RULES\npass in inet6 proto udp from any port
dhcpv6-server to any port dhcpv6-client
fi
- RULES=$RULES\npass proto carp keep state (no-sync)
+ RULES=$RULES\npass in proto carp keep state (no-sync)
+ RULES=$RULES\npass out proto carp !received-on any keep state
(no-sync)
case `sysctl vfs.mounts.nfs 2/dev/null` in
*[1-9]*)
# don't kill NFS
RULES=set reassemble yes no-df\n$RULES
RULES=$RULES\npass in proto { tcp, udp } from any port { 111,
2049 } to any
- RULES=$RULES\npass out proto { tcp, udp } from any to any port
{ 111, 2049 }
+ RULES=$RULES\npass out proto { tcp, udp } from any to any port
{ 111, 2049 } !received-on any
;;
esac
echo $RULES | pfctl -f -
--
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/
Re: openssl's *strlcy
On 2014/04/18 09:50, dera...@cvs.openbsd.org wrote: Small demonstration of the kinds of things we'll have to mop up for weeks more. From OpenSSL CHANGES file: *) Introduce safe string copy and catenation functions (BUF_strlcpy() and BUF_strlcat()). [Ben Laurie (CHATS) and Richard Levitte] That's from back in 2002. These functions work just like ours in OpenBSD. The return values indicate overflow. We've been advising people to check for overflow from the start, which is why we designed these like snprintf. Good move, but then let's see how they used them: PostgreSQL did a strlcpy conversion recently. Use strlcpy() and related functions to provide a clear guarantee that fixed-size buffers are not overrun. Guess what, this is exactly whow they used them too.
Re: openssl's *strlcy
Seems it is ok to use strlcat/strlcpy that way in some cases:
$ cat src/usr.sbin/smtpd/*.c | egrep -c ' strlc(at|py)\('
249
Re: openssl's *strlcy
On Fri, Apr 18, 2014 at 05:19:15PM -0700, Claus Assmann wrote:
Seems it is ok to use strlcat/strlcpy that way in some cases:
$ cat src/usr.sbin/smtpd/*.c | egrep -c ' strlc(at|py)\('
249
If your only goal is ensuring you don't have a non-nul terminated
string, sure, that's great. and the way we like to indicate
that you thought about that in code is to explicitly put (void)
in front of it.
so i.e.
(void) strlcat(foo, bar, size)
should mean you dont care if the value is truncated. and that's
perfectly ok, because lots of times you dont.
The problem is what if bad things happen if the string is truncated?
I started looking just a short time ago, and will probably call it a
night, why? well. my wife is asking me to put the computer down, and
frankly, I need to rewrite the *second* function I got to. For your
viewing pleaseure, please check out line 1124 of apps/ca.c There you
see two strlcpys.. no problem right, but wait, we're building a path
we're adding stuff on the end, if that / doesn't get there there are
problems. oh look, then there's pointer arithmatic and some pretty
crufty goo.
This sort of tells me they never heard of asprintf.
The point is in the *second usage* I go to look at it, I can't just
look at that code and say oh it's fine.
Of necessity, some crypto code will be a bit scary to look at.
anything that has to do regular jobs, like building a pathname, in a
security library, should be a thing of beauty, not something that
makes you want to throw up in your mouth. (and don't say it's just
an app. I've seen what people do with this app and what
they feed it data from..)
sure, s/strlcpy/strncpy/ is 'better' - but this library is
security code, and it should be clean, and modern. that's the goal
here. not halfassed fixes with find and sed. If this is our fork,
it will not be halfassed.
-Bob
Re: openssl's *strlcy
I'm guessing that openssl was incorporated into OpenBSD base without
prior sufficient audit by the OBSD devs because it was presumed to have
better auditing / quality control upstream given its security critical
nature and function. (A number of devs have commented in the past about
the [lack of] code style, but I get the impression no-one expected the
degree of *sloppiness* now being uncovered.) So here's a question, are
there any other chunks of code that have been imported en-mass from an
upstream source that could/should use an audit? especially, something
that some of us non-developers might be able to assist with?
On 18 Apr 2014 at 19:06, Bob Beck wrote:
On Fri, Apr 18, 2014 at 05:19:15PM -0700, Claus Assmann wrote:
Seems it is ok to use strlcat/strlcpy that way in some cases:
$ cat src/usr.sbin/smtpd/*.c | egrep -c ' strlc(at|py)\('
249
If your only goal is ensuring you don't have a non-nul terminated
string, sure, that's great. and the way we like to indicate
that you thought about that in code is to explicitly put (void)
in front of it.
so i.e.
(void) strlcat(foo, bar, size)
should mean you dont care if the value is truncated. and that's
perfectly ok, because lots of times you dont.
The problem is what if bad things happen if the string is truncated?
I started looking just a short time ago, and will probably call it a
night, why? well. my wife is asking me to put the computer down, and
frankly, I need to rewrite the *second* function I got to. For your
viewing pleaseure, please check out line 1124 of apps/ca.c There you
see two strlcpys.. no problem right, but wait, we're building a path
we're adding stuff on the end, if that / doesn't get there there are
problems. oh look, then there's pointer arithmatic and some pretty
crufty goo.
This sort of tells me they never heard of asprintf.
The point is in the *second usage* I go to look at it, I can't just
look at that code and say oh it's fine.
Of necessity, some crypto code will be a bit scary to look at.
anything that has to do regular jobs, like building a pathname, in a
security library, should be a thing of beauty, not something that
makes you want to throw up in your mouth. (and don't say it's just
an app. I've seen what people do with this app and what
they feed it data from..)
sure, s/strlcpy/strncpy/ is 'better' - but this library is
security code, and it should be clean, and modern. that's the goal
here. not halfassed fixes with find and sed. If this is our fork,
it will not be halfassed.
-Bob
