Thread: Patch to add Heimdal kerberos support
Attached please find a patch to make Postgres compile with Heimdal krb5 support. This patch adds a new option, --with-heimdal. "--with-krb5" now implies MIT krb5 support. Also added are two new defines, KRB5_MIT and KRB5_HEIMDAL, which indicate which flavor of KRB5 support is present (there are still some API differences). Enjoy! Take care, Bill
Attachment
Bill Studenmund <wrstuden@netbsd.org> writes:
> Attached please find a patch to make Postgres compile with Heimdal krb5
> support. This patch adds a new option, --with-heimdal. "--with-krb5" now
> implies MIT krb5 support.
Couldn't we do this in a way that doesn't require a user configure switch?
--- src/backend/libpq/auth.c 2001/10/28 06:25:44 1.71
+++ src/backend/libpq/auth.c 2001/11/12 22:32:00
@@ -229,7 +229,7 @@
" Kerberos error %d\n", retval);
com_err("postgres", retval,
"while getting server principal for service %s",
- pg_krb_server_keyfile);
+ PG_KRB_SRVNAM);
krb5_kt_close(pg_krb5_context, pg_krb5_keytab);
This change seems like a step backwards.
krb5_free_context(pg_krb5_context);
return STATUS_ERROR;
@@ -283,8 +283,13 @@
*
* I have no idea why this is considered necessary.
*/
+#ifdef KRB5_MIT
retval = krb5_unparse_name(pg_krb5_context,
ticket->enc_part2->client, &kusername);
+#else
+ retval = krb5_unparse_name(pg_krb5_context,
+ ticket->client, &kusername);
+#endif
If this is the only code change needed, couldn't we dispense with it
somehow? I notice that the previous authors of this code had grave
doubts about comparing the username at all. I don't know much about
Kerberos' security model --- is the fact that we got a ticket sufficient
authentication, and if not why not?
regards, tom lane
On Mon, 12 Nov 2001, Tom Lane wrote:
> Bill Studenmund <wrstuden@netbsd.org> writes:
> > Attached please find a patch to make Postgres compile with Heimdal krb5
> > support. This patch adds a new option, --with-heimdal. "--with-krb5" now
> > implies MIT krb5 support.
>
> Couldn't we do this in a way that doesn't require a user configure switch?
Not that I know of. There are slight differences in the APIs, and Heimdal
needs a different set of libraries compiled in.
I'll ask around.
> --- src/backend/libpq/auth.c 2001/10/28 06:25:44 1.71
> +++ src/backend/libpq/auth.c 2001/11/12 22:32:00
> @@ -229,7 +229,7 @@
> " Kerberos error %d\n", retval);
> com_err("postgres", retval,
> "while getting server principal for service %s",
> - pg_krb_server_keyfile);
> + PG_KRB_SRVNAM);
> krb5_kt_close(pg_krb5_context, pg_krb5_keytab);
>
> This change seems like a step backwards.
This patch really isn't Heimdal-related. If you look at the code
preceeding this, we are looking for the principal for service
PG_KRB_SRVNAM in the file pg_krb_server_keyfile. Unpatched, this error
message says,
"...error.. while getting server principal for service
/usr/local/psql/data/krb5.srvtab"
which makes no sense. It was really "... while getting server principal
for service postgres" for instance; PG_KRB_SRVNAM is the service name we
looked up, so it is the one we should mention.
>
> krb5_free_context(pg_krb5_context);
> return STATUS_ERROR;
> @@ -283,8 +283,13 @@
> *
> * I have no idea why this is considered necessary.
> */
> +#ifdef KRB5_MIT
> retval = krb5_unparse_name(pg_krb5_context,
> ticket->enc_part2->client, &kusername);
> +#else
> + retval = krb5_unparse_name(pg_krb5_context,
> + ticket->client, &kusername);
> +#endif
>
> If this is the only code change needed, couldn't we dispense with it
There is one more a little later on in the patch, in the front-end code.
> somehow? I notice that the previous authors of this code had grave
> doubts about comparing the username at all. I don't know much about
> Kerberos' security model --- is the fact that we got a ticket sufficient
> authentication, and if not why not?
I'll be honest that I'm still learning kerberos and all of the twists and
turns of its code.
I think the point is this test and the code after it makes sure that your
kerberos and your postgres usernames match. I think that's VERY important.
Otherwise I could log into kerberos as wrstuden and access postgres as
user tgl. That seems BAD to me. :-)
Take care,
Bill
Bill Studenmund <wrstuden@netbsd.org> writes:
> I think the point is this test and the code after it makes sure that your
> kerberos and your postgres usernames match. I think that's VERY important.
> Otherwise I could log into kerberos as wrstuden and access postgres as
> user tgl. That seems BAD to me. :-)
Well, it's not clear to me. Where did the ticket come from? Perhaps
we've already determined that you are who you say you are just by being
able to acquire the ticket. Even more to the point are the comments in
front of the pg_an_to_ln subroutine: sure, we may be comparing against
*something* extracted from the ticket, but it's not at all clear that
it's a username. Seems like there's lots of potential for BADness in
that.
regards, tom lane
I said:
> Well, it's not clear to me. Where did the ticket come from? Perhaps
> we've already determined that you are who you say you are just by being
> able to acquire the ticket.
After doing a little light reading (RFC 1510...) it seems that what the
Kerberos auth exchange actually gives you is confidence that the other
party is the Kerberos principal named in the ticket you got from him.
We don't use the claimed Postgres username in the process of acquiring
the ticket, so there's no automatic cross-check as I was hoping. We
must look at the principal name.
The problem from our point of view is that a Kerberos principal name
is a multi-part entity, and it's not entirely clear how to map that into
a Postgres username. I'm moderately unhappy with the solution used in
the code now: take the first name component, ignore all else, including
the realm. This appears to mean that a server living in Kerberos realm
k1 will happily accept people from different realms, eg joe@k2, without
noticing that this is not the same person as joe@k1. Bad dog.
It also seems that our present code may be using some long-deprecated
Kerberos APIs --- for example, the krb5_recvauth man pages I can find on
the net describe several more parameters than our code is expecting to
pass. I suspect that that direct access to a field of the ticket is
a hangover we should be able to get rid of.
regards, tom lane
On Mon, 12 Nov 2001, Tom Lane wrote: > I said: > > Well, it's not clear to me. Where did the ticket come from? Perhaps > > we've already determined that you are who you say you are just by being > > able to acquire the ticket. > > After doing a little light reading (RFC 1510...) it seems that what the > Kerberos auth exchange actually gives you is confidence that the other > party is the Kerberos principal named in the ticket you got from him. > We don't use the claimed Postgres username in the process of acquiring > the ticket, so there's no automatic cross-check as I was hoping. We > must look at the principal name. Exactly. > The problem from our point of view is that a Kerberos principal name > is a multi-part entity, and it's not entirely clear how to map that into > a Postgres username. I'm moderately unhappy with the solution used in > the code now: take the first name component, ignore all else, including > the realm. This appears to mean that a server living in Kerberos realm > k1 will happily accept people from different realms, eg joe@k2, without > noticing that this is not the same person as joe@k1. Bad dog. Hmmm... That is bad. You do have to have cross-realm authentication set up to need to worry, but yes, if you do, you have a problem. For now the right thing probably is to pull out the realm and make sure it is the same. Eventually it would be nice to do something like list principals (name@realm) which can log in as postgres user foo. Like what .klogin does for telnet. But that is going a bit beyond making pg work with Heimdal, that's making the krb5 code work better. :-) Still checking on a good auto-detect. > It also seems that our present code may be using some long-deprecated > Kerberos APIs --- for example, the krb5_recvauth man pages I can find on > the net describe several more parameters than our code is expecting to > pass. I suspect that that direct access to a field of the ticket is > a hangover we should be able to get rid of. For krb5_recvauth()? We are using the right function signature... The general problem of directly accessing the ticket is one which porting to Heimdal revealed. The MIT code was routinely accessing library-private bits of info. There's an effort afoot to clean this up, but I don't think it's far-enough along yet. Plus a number of OSs have shipped with the non-unified version of Heimdal. :-| Take care, Bill
Bill Studenmund <wrstuden@netbsd.org> writes: > Still checking on a good auto-detect. ISTM that autoconf should be capable of figuring out which version of kerberos libraries we have --- that sort of discrepancy is exactly what it's designed to handle. >> Kerberos APIs --- for example, the krb5_recvauth man pages I can find on >> the net describe several more parameters than our code is expecting to >> pass. > For krb5_recvauth()? We are using the right function signature... According to whom? I found http://archive.ncsa.uiuc.edu/General/CC/kerberos/krb5api/krb5api4.html in which krb5_recvauth doesn't agree with our code. (I miscounted yesterday, there's only one more parameter described than we pass, but it's definitely not the same.) regards, tom lane
On Tue, 13 Nov 2001, Tom Lane wrote: > Bill Studenmund <wrstuden@netbsd.org> writes: > > Still checking on a good auto-detect. > > ISTM that autoconf should be capable of figuring out which version of > kerberos libraries we have --- that sort of discrepancy is exactly > what it's designed to handle. Yes, but I don't know of a good thing to go looking for to tell. :-) I'm not aware of something which says "Heimdal" or "MIT". Yes, I can personally look at the libraries, and if I see libroken and libasn1, then chances are it's heimdal, and if I see libk5crypto (I think that's the one), chances are it's MIT. But I'd like the configure script to be more robust - how do we tell if we have a broken install of either type? I'm not sure, and so I don't want to make that call. > >> Kerberos APIs --- for example, the krb5_recvauth man pages I can find on > >> the net describe several more parameters than our code is expecting to > >> pass. > > > For krb5_recvauth()? We are using the right function signature... > > According to whom? I found According to the krb5_recvauth() source code in my source tree, which is the Heimdal tree NetBSD 1.5 shipped with. > http://archive.ncsa.uiuc.edu/General/CC/kerberos/krb5api/krb5api4.html > in which krb5_recvauth doesn't agree with our code. (I miscounted > yesterday, there's only one more parameter described than we pass, > but it's definitely not the same.) Weird. Because of this question, I pulled down a copy of MIT kerberos, and its prototype for krb5_recvauth() matches our usage and also Heimdal's. I suspect that documentation is out of date, though I can't definitly tell. I have no idea what the rc_type (the one the docs show that we don't use) parameter would have done. No, I looked at the docs in the MIT kerberos I pulled down, and they list an rc_type parameter for krb5_recvauth. The code, though, doesn't have one! Also, nothing else in the doc refers to rc_type.... Take care, Bill
Bill Studenmund <wrstuden@netbsd.org> writes:
> I'm not aware of something which says "Heimdal" or "MIT". Yes, I can
> personally look at the libraries, and if I see libroken and libasn1, then
> chances are it's heimdal, and if I see libk5crypto (I think that's the
> one), chances are it's MIT. But I'd like the configure script to be more
> robust - how do we tell if we have a broken install of either type?
Peter Eisentraut is our local autoconf guru, and perhaps will be willing
to help you out with this. But my guess would be that we link with
whichever libraries we see out there. It's not our business to
determine whether the Kerberos install is "broken".
> No, I looked at the docs in the MIT kerberos I pulled down, and they list
> an rc_type parameter for krb5_recvauth. The code, though, doesn't have
> one! Also, nothing else in the doc refers to rc_type....
Hmph. Okay, it must just be an error in the MIT docs. Nevermind that
then.
I still wonder whether there isn't some documented API (common to both
MIT and Heimdal) for extracting the client principal from a ticket.
I mean, that's almost the entire reason for getting the ticket in the
first place; you can hardly argue that this is not core functionality.
I find it hard to believe that Heimdal hasn't duplicated the standard
way of getting the principal from a ticket. I can believe that we
weren't *using* the standard way, however...
regards, tom lane
On Tue, 13 Nov 2001, Tom Lane wrote: > Bill Studenmund <wrstuden@netbsd.org> writes: > > I'm not aware of something which says "Heimdal" or "MIT". Yes, I can > > personally look at the libraries, and if I see libroken and libasn1, then > > chances are it's heimdal, and if I see libk5crypto (I think that's the > > one), chances are it's MIT. But I'd like the configure script to be more > > robust - how do we tell if we have a broken install of either type? > > Peter Eisentraut is our local autoconf guru, and perhaps will be willing > to help you out with this. But my guess would be that we link with > whichever libraries we see out there. It's not our business to > determine whether the Kerberos install is "broken". Actually the current configure script makes it its business to tell if the install is broken. :-) It tests that we can successfully link in a number of symbols. For instance, "checking for com_err in -lcom_err", "checking for krb5_encrypt in -lcrypto", "checking for krb5_encrypt in -lk5crypto", and "checking for krb5_sendauth in -lkrb5". These tests need to change depending on the flavor of kerberos. For instance, under heimdal, krb5_encrypt is in libkrb5 and different libs have to be linked in to make tests work (-lasn1 -lroken -lcrypto). Looking at things, we might be able to look for the presence of the symbol __heimdal_version to see if it's heimdal krb5: > strings /usr/lib/libkrb5.so | grep -i heimdal __heimdal_version __heimdal_long_version @(#)$Version: heimdal-0.3e (NetBSD) $ heimdal-0.3e So that would work/could be made to work. > I still wonder whether there isn't some documented API (common to both > MIT and Heimdal) for extracting the client principal from a ticket. > I mean, that's almost the entire reason for getting the ticket in the > first place; you can hardly argue that this is not core functionality. > I find it hard to believe that Heimdal hasn't duplicated the standard > way of getting the principal from a ticket. I can believe that we > weren't *using* the standard way, however... I'm checking, but I remember comments from Jason Thrope, a NetBSD developer who did some of the initial porting to heimdal after it was brought into NetBSD. I think he was working on getting gssapi to work with heimdal. There were many complaints about how the MIT code was taking liberties with reaching into library internals, and the principal name (what we're getting at here) was one of them. Remember how Heimdal developed. A lot of its work was done during the encryption==munitions==noexport era. So heimdal was written to the RFCs and other published specs. What we're seeing here is one example of where the specs didn't cover everything. No one bothered to make a way (routines) to get the name, as you could just reach in the packet and get it. Just the standards didn't describe the names of the fields in the ticket structure, MIT and KTH chose different names, and no one noticed until code had been written for both. :-| From talking w/ Ken Hornstein (keeper of the kerberos FAQ), the APIs are different at the moment. They are working to merge them, but they are still different. And from PostgreSQL's point of view, divergent versions have been shipped. Take care, Bill
On Tue, 13 Nov 2001, Tom Lane wrote: > I still wonder whether there isn't some documented API (common to both > MIT and Heimdal) for extracting the client principal from a ticket. > I mean, that's almost the entire reason for getting the ticket in the > first place; you can hardly argue that this is not core functionality. > I find it hard to believe that Heimdal hasn't duplicated the standard > way of getting the principal from a ticket. I can believe that we > weren't *using* the standard way, however... I've been digging into this, and I think the problem is there is no standard way to do what we're doing. It is a flaw in the design of the specs and the krb5_unparse_name() routine. There are however other ways to do what we want. First off, I think the comment about pg_an_to_ln is wrong; if you have some sort of goofy multi-part name (like the example out of X.400 hell, "ORGANIZATION=U. C. Berkeley/NAME=Paul M. Aoki@CS.BERKELEY.EDU" *AND* your kerberos lib isn't set up to deal with it (krb5_aname_to_localname() punts), why should PostgreSQL let you in? Oh, the comment is also very old. It's from version 1.1 of the file, checked in in 1996. Everything else changed, so I'd expect that if you are using X.400 names like the horror above, well, you are 1) using MIT, and 2) you can set things up right (or at least there is documentation to show you how). (Heimdal supports single-component names, or two-component names where the second name is root. In the latter case, the name is taken as root.) Dang. I've been fussing with code for a bit, and realised that while there are ways we can get rid of the use of krb5_aname_to_localname, we still have to reach in the received ticket and pull out the principal in a non-standard way. Take care, Bill
Tom Lane writes: > Couldn't we do this in a way that doesn't require a user configure switch? We've seen a Heimdal patch before that had the same source changes but solved the configury in some other ugly way. It's not trivial to write a macro that checks for struct members. However, that other patch didn't need to link in all these extra libraries as this patch tries; it only used -lkrb5. There is only one Heimdal I trust? -- Peter Eisentraut peter_e@gmx.net
Tom Lane writes: > It also seems that our present code may be using some long-deprecated > Kerberos APIs --- for example, the krb5_recvauth man pages I can find on > the net describe several more parameters than our code is expecting to > pass. No idea about the deprecatedness, but our Kerberos code does compile with the Kerberos V shipped in Red Hat 7.0 (MIT, I think), and the Kerberos IV code compiled successfully with KTH Kerberos last time I checked. However, most of the three people that ever mentioned anything detailed about the Kerberos support in PostgreSQL had the sound of "fundementally flawed", "totally insecure", etc. -- Peter Eisentraut peter_e@gmx.net
On Wed, 14 Nov 2001, Peter Eisentraut wrote: > Tom Lane writes: > > > Couldn't we do this in a way that doesn't require a user configure switch? > > We've seen a Heimdal patch before that had the same source changes but > solved the configury in some other ugly way. It's not trivial to write a There is another test we can do. There is a variable which contains the Heimdal version number, '__heimdal_version'. If that symbol is present, then we have Heimdal, and if it's not, we have MIT. So something like 'nm /usr/lib/libkrb5.a | grep __heimdal_version' is a good test. > macro that checks for struct members. However, that other patch didn't > need to link in all these extra libraries as this patch tries; it only > used -lkrb5. There is only one Heimdal I trust? Yes, there is only one Heimdal. However some linkers are better with inter-library dependencies than others. NetBSD 1.5, the OS I've used for developing this, doesn't handle them automagically. So you have to explicitly include them. I think -current and 1.5.2 have this fixed, I'm just stuck in the stone age. ;-) Something like --with-extralibs would suffice to cover this. But the extra libs would need to be used for compiling the kerberos test cases. :-) I would appreciate help with this. I think for now the difference in API needs to be supported, but everything else about the config setup can be changed as desired. :-) Take care, Bill
On Wed, 14 Nov 2001, Peter Eisentraut wrote: > No idea about the deprecatedness, but our Kerberos code does compile with > the Kerberos V shipped in Red Hat 7.0 (MIT, I think), and the Kerberos IV > code compiled successfully with KTH Kerberos last time I checked. > > However, most of the three people that ever mentioned anything detailed > about the Kerberos support in PostgreSQL had the sound of "fundementally > flawed", "totally insecure", etc. I'll admit our kerberos support was NOT what I expected it to be when I got it working; I was quite surprised to see all of the queries going by in the clear. It depends on what you want. If you want to not have clear-text passwords go by and to have a very good idea who the person on the other side of the session is, then this kerberos support does that. No one will be able to sniff a password off of this. It's like kpop or telnet -a If you expected an encrypted session, well, you don't get it. This isn't telnet -ax. :-( Is there interest in supporting encrypted sessions? I can think of two ways to do it: 1) in addition to kerberos as an authentication, we also add kerbers-priv which is the current kerberos but we switch to encryption once we indicate successful authentication. 2) we add a start-encrypting command to the protocol. I dislike 2) as the best encryption key to use is the one we got with the authentication step, which we'd have to hang onto for a while in case we decided to start encrypting. Take care, Bill
Bill Studenmund <wrstuden@netbsd.org> writes:
> Is there interest in supporting encrypted sessions?
We already have SSL support; it's not clear to me that it's worth our
trouble to support a second mechanism. Especially one that only works
with one flavor of authentication.
I might be more interested if Kerberos were more popular ... but you're
the first person who's done any work on the Kerberos code in the whole
time I've been around the project, so I suspect it ain't of wide
interest.
regards, tom lane
On Wed, 14 Nov 2001, Tom Lane wrote: > Bill Studenmund <wrstuden@netbsd.org> writes: > > Is there interest in supporting encrypted sessions? > > We already have SSL support; it's not clear to me that it's worth our > trouble to support a second mechanism. Especially one that only works > with one flavor of authentication. I was unaware of the SSL support when I asked the question, and had seen comments in the code about how we don't encrypt as it would break the protocol if we did... Evidently we have figured out how to do one sort of encryption. :-) > I might be more interested if Kerberos were more popular ... but you're > the first person who's done any work on the Kerberos code in the whole > time I've been around the project, so I suspect it ain't of wide > interest. Two things: 1) at least two other people have worked on the kerberos code. backend/libpq/auth.c revision 1.45 added the current krb5 support; that came from someone. :-) And Peter said there has been one other Heimdal patch, which also came from someone. :-) Second, I agree with Peter that the current support is sub-optimal. *I* wouldn't use it, as I really want session encryption. So if what we have isn't what people want, is it any wonder they don't use it? :-) I'll admit I don't expect folks to rush out and totally switch to kerberos. But if we don't have the feature, no one will. Also, I expect Kerberos to get more popular now that Windows is supporting it. From looking at the code, the ssl changes show exactly what has to happen for data exchange to support other encryptions. Instead of recv() or SSL_read(), we do a kerberos read and decrypt. The thing I'm not sure of is how to have the backend tell the client it needs to do encryption. A new "authmethod" would be an easy one, but there might be better ways. Take care, Bill
Bill Studenmund <wrstuden@netbsd.org> writes:
> Second, I agree with Peter that the current support is sub-optimal. *I*
> wouldn't use it, as I really want session encryption.
If you have the energy to fix it, I won't stand in your way.
regards, tom lane
On Wed, 14 Nov 2001, Tom Lane wrote: > Bill Studenmund <wrstuden@netbsd.org> writes: > > Second, I agree with Peter that the current support is sub-optimal. *I* > > wouldn't use it, as I really want session encryption. > > If you have the energy to fix it, I won't stand in your way. That sounds fair. :-) Take care, Bill
Bill Studenmund writes: > There is another test we can do. There is a variable which contains the > Heimdal version number, '__heimdal_version'. If that symbol is present, > then we have Heimdal, and if it's not, we have MIT. We're not interested in whether it's Heimdal or MIT, we're only interested in whether that certain struct member is present and the set of libraries needed to get at the functions we want. The latter can be done with AC_SEARCH_LIBS, and if you look into krb5 first you might get all those other libraries for free depending on the platform. Checking for struct members can be done with a compilation test in the style of config/c-library.m4. (Basically, you just check if the code you want to use compiles. That's always a good start for a configure check, lacking other ideas.) I don't know if you want to sell this as a bug fix for 7.2. If you are then I can work out a "good" patch for this in the next two days for you to test. -- Peter Eisentraut peter_e@gmx.net
On Thu, 15 Nov 2001, Peter Eisentraut wrote: > Bill Studenmund writes: > > > There is another test we can do. There is a variable which contains the > > Heimdal version number, '__heimdal_version'. If that symbol is present, > > then we have Heimdal, and if it's not, we have MIT. > > We're not interested in whether it's Heimdal or MIT, we're only interested > in whether that certain struct member is present and the set of libraries > needed to get at the functions we want. The latter can be done with > AC_SEARCH_LIBS, and if you look into krb5 first you might get all those > other libraries for free depending on the platform. Checking for struct > members can be done with a compilation test in the style of > config/c-library.m4. (Basically, you just check if the code you want to > use compiles. That's always a good start for a configure check, lacking > other ideas.) The problem I had with AC_SEARCH_LIBS was that if I don't add all of the libraries (-lasn1 -lroken etc.) the test program won't compile, even though the .o compiles fine. Can we make the test only depend on generating a .o? > I don't know if you want to sell this as a bug fix for 7.2. If you are > then I can work out a "good" patch for this in the next two days for you > to test. I'd like to make it a bug fix for 7.2 if we can, so I'd appreciate the help. I can test any new versions of the patch. :-) Oh, I'll be out of town from Nov 19 through the 24th, and I have intermittent weekend EMail, so please understand if I am less than responsive in those time windows. :-) Take care, Bill
Bill Studenmund writes: > The problem I had with AC_SEARCH_LIBS was that if I don't add all of the > libraries (-lasn1 -lroken etc.) the test program won't compile, even > though the .o compiles fine. Can we make the test only depend on > generating a .o? That wouldn't tell you a whole lot about the existence of a function. You do need to link to be able to verify that. > I'd like to make it a bug fix for 7.2 if we can, so I'd appreciate the > help. I can test any new versions of the patch. :-) The attached patch should handle the different struct members (at least it doesn't break them for me) and it doesn't gratuitously fail on libraries that are not really needed. I haven't done anything about -lasn1 -lroken -lcrypto because I'd need to know what functions you need from there. You should be able to get it to work when you configure thus: $ LIBS='-lasn1 -lroken -lcrypto' ./configure --with-krb5 ...other options... If you can confirm that this patch works I can check it in so it appears in whatever beta3+1 is going to be. However, be aware that we're approaching the first release candidate, and since this issue doesn't represent a regression from 7.1 it's not going to hold up the release. -- Peter Eisentraut peter_e@gmx.net
Attachment
On Sat, 17 Nov 2001, Peter Eisentraut wrote: > Bill Studenmund writes: > > > The problem I had with AC_SEARCH_LIBS was that if I don't add all of the > > libraries (-lasn1 -lroken etc.) the test program won't compile, even > > though the .o compiles fine. Can we make the test only depend on > > generating a .o? > > That wouldn't tell you a whole lot about the existence of a function. You > do need to link to be able to verify that. The thought was that the existance of the symbol (not function) __heimdal_version is an easy way to tell which version is in use. > > I'd like to make it a bug fix for 7.2 if we can, so I'd appreciate the > > help. I can test any new versions of the patch. :-) > > The attached patch should handle the different struct members (at least it > doesn't break them for me) and it doesn't gratuitously fail on libraries > that are not really needed. I haven't done anything about -lasn1 -lroken > -lcrypto because I'd need to know what functions you need from there. You > should be able to get it to work when you configure thus: Well, the real problem is that libkrb5 needs these libraries. More importantly, the tests to see if libkrb5 is ok need them. :-) > $ LIBS='-lasn1 -lroken -lcrypto' ./configure --with-krb5 ...other options... Unfortunatly that won't work as libasn1 needs libcom_err. So the above line complains that gcc is broken. :-( So what needs to happen is that libasn1, libroken, and libcrypto need to be looked for part way through the kerberos tests, if we have heimdal. If we have MIT, we shouldn't look for them. Well, more to the point, not having working libasn1 and libroken is only a problem for heimdal; it's an ok state of affairs for MIT kerberos. Thoughts? > If you can confirm that this patch works I can check it in so it appears > in whatever beta3+1 is going to be. However, be aware that we're > approaching the first release candidate, and since this issue doesn't > represent a regression from 7.1 it's not going to hold up the release. I understand. Take care, Bill
On Sat, 17 Nov 2001, Peter Eisentraut wrote: > Bill Studenmund writes: > > > The problem I had with AC_SEARCH_LIBS was that if I don't add all of the > > libraries (-lasn1 -lroken etc.) the test program won't compile, even > > though the .o compiles fine. Can we make the test only depend on > > generating a .o? > > That wouldn't tell you a whole lot about the existence of a function. You > do need to link to be able to verify that. Oh. Looking back on this, I see a step I forgot to mention. I was jumping from AC_SEARCH_LIBS, which does need linking to see a function is there, to making a test which would make just a .o, to see if __heimdal_version is in krb5.h. That way we could see if we're heimdal or MIT directly. Sorry. > > I'd like to make it a bug fix for 7.2 if we can, so I'd appreciate the > > help. I can test any new versions of the patch. :-) > > The attached patch should handle the different struct members (at least it > doesn't break them for me) and it doesn't gratuitously fail on libraries > that are not really needed. I haven't done anything about -lasn1 -lroken > -lcrypto because I'd need to know what functions you need from there. You > should be able to get it to work when you configure thus: Ok. The patch didn't work. I'm attaching my revised version. I did a couple of things, and can break it into different patches if desired. 1) I moved the test for the struct members to before the check for libraries. I don't like this as it means looking in a .h out of order with looking for the other .h's, but, well, you have to have the .h for the lib search test to really work. 2) I changed the krb5 lib test to look for libasn1, libroken, and libcrypto as needed. The thing I don't like about this is I'm basing the test on the presence of some of the struct fields. I know there is active work on consolidating the APIs, so this might not be a good long-term test. It also is a hack, though perhaps good enough for now. 3) I pulled in a few of the changes I made in response to our earlier discussion. pg_an_to_ln() is not used, we use a routine out of libkrb5 instead. I don't understand the comment that, "we can't punt," if we can't find a name as if we can't find a name, how can we say anything about the security of the authentication? 4) I also added changes to make the PAM authenitcation method behave like the kerberos ones; the PAM authentication method is always part of the protocol, just if it's not compiled in and you try to use it, you get an error message about it. This change, besides being consistent with the other auth methods, makes it possible to add another auth method. Adding encrypted sessions will probably need it (but that's a seperate thread). I realize it's probably too late for 7.2, but I'd like to submit these (at least 1, 2, and 3) for consideration for 7.2.1 whenever it happens. :-) Peter, what comments on this do you have? Take care, Bill
Would help to attach the patch. Oh, the include/krb5 test for includes is so that the heimdal code can find /usr/include/krb5/krb5.h (--with-krb5=/usr). Take care, Bill
Attachment
Peter, I assume this is to be applied to 7.3, right? --------------------------------------------------------------------------- Peter Eisentraut wrote: > Bill Studenmund writes: > > > The problem I had with AC_SEARCH_LIBS was that if I don't add all of the > > libraries (-lasn1 -lroken etc.) the test program won't compile, even > > though the .o compiles fine. Can we make the test only depend on > > generating a .o? > > That wouldn't tell you a whole lot about the existence of a function. You > do need to link to be able to verify that. > > > I'd like to make it a bug fix for 7.2 if we can, so I'd appreciate the > > help. I can test any new versions of the patch. :-) > > The attached patch should handle the different struct members (at least it > doesn't break them for me) and it doesn't gratuitously fail on libraries > that are not really needed. I haven't done anything about -lasn1 -lroken > -lcrypto because I'd need to know what functions you need from there. You > should be able to get it to work when you configure thus: > > $ LIBS='-lasn1 -lroken -lcrypto' ./configure --with-krb5 ...other options... > > If you can confirm that this patch works I can check it in so it appears > in whatever beta3+1 is going to be. However, be aware that we're > approaching the first release candidate, and since this issue doesn't > represent a regression from 7.1 it's not going to hold up the release. > > -- > Peter Eisentraut peter_e@gmx.net Content-Description: Patch [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 2: you can get off all lists at once with the unregister command > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
OK, I believe this patch includes Peter's suggested changes.
Your patch has been added to the PostgreSQL unapplied patches list at:
http://candle.pha.pa.us/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.
---------------------------------------------------------------------------
Bill Studenmund wrote:
> Would help to attach the patch.
>
> Oh, the include/krb5 test for includes is so that the heimdal code can
> find /usr/include/krb5/krb5.h (--with-krb5=/usr).
>
> Take care,
>
> Bill
Content-Description: Revised patch to add heimdal support
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Bruce Momjian writes: > OK, I believe this patch includes Peter's suggested changes. Hard to tell without seeing the patch, but the previous one looked a lot more familiar to me. -- Peter Eisentraut peter_e@gmx.net
Peter Eisentraut wrote:
> Bruce Momjian writes:
>
> > OK, I believe this patch includes Peter's suggested changes.
>
> Hard to tell without seeing the patch, but the previous one looked a lot
> more familiar to me.
Yes, I understand. They are all at:
http://candle.pha.pa.us/cgi-bin/pgpatches
I will put out a request to review everything in there before they are
applied.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Bruce Momjian writes: > Peter, I assume this is to be applied to 7.3, right? I've checked in Heimdal support. -- Peter Eisentraut peter_e@gmx.net
Peter has added the needed Kerberos fixes to CVS for 7.3. --------------------------------------------------------------------------- > > > > Peter, I assume this is to be applied to 7.3, right? > > > > --------------------------------------------------------------------------- > > > > Peter Eisentraut wrote: > > > Bill Studenmund writes: > > > > > > > The problem I had with AC_SEARCH_LIBS was that if I don't add all of the > > > > libraries (-lasn1 -lroken etc.) the test program won't compile, even > > > > though the .o compiles fine. Can we make the test only depend on > > > > generating a .o? > > > > > > That wouldn't tell you a whole lot about the existence of a function. You > > > do need to link to be able to verify that. > > > > > > > I'd like to make it a bug fix for 7.2 if we can, so I'd appreciate the > > > > help. I can test any new versions of the patch. :-) > > > > > > The attached patch should handle the different struct members (at least it > > > doesn't break them for me) and it doesn't gratuitously fail on libraries > > > that are not really needed. I haven't done anything about -lasn1 -lroken > > > -lcrypto because I'd need to know what functions you need from there. You > > > should be able to get it to work when you configure thus: > > > > > > $ LIBS='-lasn1 -lroken -lcrypto' ./configure --with-krb5 ...other options... > > > > > > If you can confirm that this patch works I can check it in so it appears > > > in whatever beta3+1 is going to be. However, be aware that we're > > > approaching the first release candidate, and since this issue doesn't > > > represent a regression from 7.1 it's not going to hold up the release. > > > > > > -- > > > Peter Eisentraut peter_e@gmx.net > > > > Content-Description: Patch > > > > [ Attachment, skipping... ] > > > > > > > > ---------------------------(end of broadcast)--------------------------- > > > TIP 2: you can get off all lists at once with the unregister command > > > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) > > > > -- > > Bruce Momjian | http://candle.pha.pa.us > > pgman@candle.pha.pa.us | (610) 853-3000 > > + If your life is a hard drive, | 830 Blythe Avenue > > + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 > > -- > Bruce Momjian | http://candle.pha.pa.us > root@candle.pha.pa.us | (610) 853-3000 > + If your life is a hard drive, | 830 Blythe Avenue > + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Peter has added the needed Kerberos fixes to CVS for 7.3. --------------------------------------------------------------------------- pgman wrote: > > Peter, I assume this is to be applied to 7.3, right? > > --------------------------------------------------------------------------- > > Peter Eisentraut wrote: > > Bill Studenmund writes: > > > > > The problem I had with AC_SEARCH_LIBS was that if I don't add all of the > > > libraries (-lasn1 -lroken etc.) the test program won't compile, even > > > though the .o compiles fine. Can we make the test only depend on > > > generating a .o? > > > > That wouldn't tell you a whole lot about the existence of a function. You > > do need to link to be able to verify that. > > > > > I'd like to make it a bug fix for 7.2 if we can, so I'd appreciate the > > > help. I can test any new versions of the patch. :-) > > > > The attached patch should handle the different struct members (at least it > > doesn't break them for me) and it doesn't gratuitously fail on libraries > > that are not really needed. I haven't done anything about -lasn1 -lroken > > -lcrypto because I'd need to know what functions you need from there. You > > should be able to get it to work when you configure thus: > > > > $ LIBS='-lasn1 -lroken -lcrypto' ./configure --with-krb5 ...other options... > > > > If you can confirm that this patch works I can check it in so it appears > > in whatever beta3+1 is going to be. However, be aware that we're > > approaching the first release candidate, and since this issue doesn't > > represent a regression from 7.1 it's not going to hold up the release. > > > > -- > > Peter Eisentraut peter_e@gmx.net > > Content-Description: Patch > > [ Attachment, skipping... ] > > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 2: you can get off all lists at once with the unregister command > > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) > > -- > Bruce Momjian | http://candle.pha.pa.us > pgman@candle.pha.pa.us | (610) 853-3000 > + If your life is a hard drive, | 830 Blythe Avenue > + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 -- Bruce Momjian | http://candle.pha.pa.us root@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026