White-listing IP-numbers or networks doesn't work

Bug #1455061 reported by Jonas Ringh
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
pam-shield (Debian)
Fix Released
Unknown
pam-shield (Ubuntu)
Confirmed
Undecided
Unassigned

Bug Description

Ubuntu version: 15.04
Package: libpam-shield, version 0.9.6-1.1

Allow statements in the configuration file (/etc/security/shield.conf) aren't correctly matched against the connecting client if an IP-address or network has been entered.

After trying to connect from my workstation at 172.16.0.52 I'm getting this in auth.log.

May 14 12:52:44 VB-k64-1504 PAM-shield[2978]: this is version 0.9.6
May 14 12:52:44 VB-k64-1504 PAM-shield[2978]: reading config file '/etc/security/shield.conf'
May 14 12:52:44 VB-k64-1504 PAM-shield[2978]: logging debug info
May 14 12:52:44 VB-k64-1504 PAM-shield[2978]: allowing from localhost
May 14 12:52:44 VB-k64-1504 PAM-shield[2978]: allowing from 127.0.0.1/255.0.0.0
May 14 12:52:44 VB-k64-1504 PAM-shield[2978]: allowing from 172.16.0.0/255.255.255.0
May 14 12:52:44 VB-k64-1504 PAM-shield[2978]: allowing from 172.16.0.52/255.255.255.255
May 14 12:52:44 VB-k64-1504 PAM-shield[2978]: done reading config file, 0 errors
May 14 12:52:44 VB-k64-1504 PAM-shield[2978]: user test
May 14 12:52:44 VB-k64-1504 PAM-shield[2978]: remotehost 172.16.0.52
May 14 12:52:44 VB-k64-1504 PAM-shield[2978]: missing DNS entry for 172.16.0.52 (allowed)
May 14 12:52:44 VB-k64-1504 PAM-shield[2978]: remoteip 172.16.0.52
May 14 12:52:44 VB-k64-1504 PAM-shield[2978]: 10 times from 172.16.0.52
May 14 12:52:44 VB-k64-1504 PAM-shield[2978]: running command 'add 172.16.0.52'
May 14 12:52:44 VB-k64-1504 shield-trigger[2981]: blocking 172.16.0.52

Connecting from a host that has a name seems to work, like connecting from localhost or if I add the 172.16.0.52 machine to /etc/hosts and use the name instead of IP in the config file.

According to the documentation in the default config file you should be able to use both IP-numbers and network addresses in "allow" statements.

Revision history for this message
jtniehof (jtniehof) wrote :

Hi Jonas--

Thanks for the report. If I recall correctly, "allow_missing_dns no" overrides "allow" entires (despite the "passed through with no checks" in the man page, I probably miswrote there) -- the DNS check comes first. I'll take a look and at very least update the documentation, but I do think it makes sense to let "allow" skip the DNS checks if it's numeric.

In the meantime, try "allow_missing_dns yes" and "allow_missing_reverse yes" if you feel comfortable with the security/attribution implications.

Note that the chance of upstream changes showing up in Ubuntu is quite low; I finally orphaned the package in Debian since it would take months and, sometimes, years to get a sponsor for the upload.

Revision history for this message
Jonas Ringh (jringh) wrote :

I have "allow_missing_dns" and "allow_missing_reverse" set to "yes" already. I also have "block all-users" set.

After digging through the source code a bit, I tracked it down to the function "match_ipv4_list" in "pam_shield_lib.c" and the for loop that loops over the octets in the ip and mask. GCC seems to do some somewhat dodgy optimization of that loop, if you comment out the "break" statement on line 124 you will get the following warning...

pam_shield_lib.c:126:23: warning: iteration 1u invokes undefined behavior [-Waggressive-loop-optimizations]
       if (( ip->ip.any[i] & ip->mask.any[i]) != (saddr[i] & ip->mask.any[i])) {
                       ^
pam_shield_lib.c:125:5: note: containing loop
     for(i = 0; i < sizeof(ip->ip.in.s_addr); i++) {
     ^

I found two ways to make the loop behave. Either you declare i volatile or turn off optimization on that section of the code.

And don't forget about "match_ipv6_list", you probably have the same problem there too.

Revision history for this message
jtniehof (jtniehof) wrote :

Thanks Jonas! I've entered this in my github and I should have a chance to address it this weekend. Let me know if you'd like to be credited as other than your full name here.

Revision history for this message
Randy Goldenberg (rsgoldenberg) wrote :

Bug still exists in Ubuntu 20.04.

Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in pam-shield (Ubuntu):
status: New → Confirmed
Changed in pam-shield (Debian):
status: Unknown → Confirmed
Changed in pam-shield (Debian):
status: Confirmed → Fix Released
Revision history for this message
jtniehof (jtniehof) wrote :

Before anybody gets excited, the fix in Debian is removing the package from the repository. Current upstream is at https://github.com/h0tw1r3/pam_shield

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.